-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Mapping update for “date_range” field type is not idempotent #2094
Conversation
Can one of the admins verify this patch? |
❌ Gradle Check failure 8205142772544c6a8667384cf88242a22bad7f42 |
❌ Gradle Check failure a9cdbdee3d2e1f79d44b2b2be2ab60693888476f |
Unrelated failure:
|
start gradle check |
❌ Gradle Check failure a9cdbdee3d2e1f79d44b2b2be2ab60693888476f |
@@ -195,7 +195,7 @@ static BinaryFieldMapper createQueryBuilderFieldBuilder(BuilderContext context) | |||
} | |||
|
|||
static RangeFieldMapper createExtractedRangeFieldBuilder(String name, RangeType rangeType, BuilderContext context) { | |||
RangeFieldMapper.Builder builder = new RangeFieldMapper.Builder(name, rangeType, true); | |||
RangeFieldMapper.Builder builder = new RangeFieldMapper.Builder(name, rangeType, true, context.indexCreatedVersion()); | |||
// For now no doc values, because in processQuery(...) only the Lucene range fields get added: | |||
builder.docValues(false); | |||
return builder.build(context); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm confused about how this change works. This builder.build(context)
is where the failure was coming from previously, right? (this build()
method would call the setupFieldType()
method which would call context.indexCreatedVersion()
which would trigger the "[index.version.created] is not present in the index settings..." exception). Won't this change trigger the same exception on line 198 when it calls context.indexCreatedVersion()
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is correct but the important part is what context is used when method is called. The exception was raised because BuilderContext
was created out of empty settings, but I will double check where this method is called to make sure no exceptions are going to be raised. Thanks for spotting it!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@andrross made a change to fetch index created only when it is available, should be safer, thanks a lot!
|
❌ Gradle Check failure 14d440d2229515cf97d13b221d358092b5f8ea19 |
❌ Gradle Check failure 5c0e50f0a6d496987daa1f368d3f7e338a5caed2 |
Signed-off-by: Andriy Redko <[email protected]>
Signed-off-by: Andriy Redko <[email protected]> (cherry picked from commit 6b6f033)
…#2106) Signed-off-by: Andriy Redko <[email protected]> (cherry picked from commit 6b6f033) Co-authored-by: Andriy Redko <[email protected]>
Signed-off-by: Andriy Redko [email protected]
Description
Update the same field using the same mapping with the same settings causes
Issues Resolved
Closes #1921
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.