-
Notifications
You must be signed in to change notification settings - Fork 24.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
Lower default mappings parsing compatibility to MINIMUM_READONLY_COMPATIBLE #119017
Changes from all commits
26fe1a7
b9e3c0a
56b392d
c040f6c
665fcff
f943a25
a906eff
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1618,6 +1618,12 @@ public static BiConsumer<String, MappingParserContext> notFromDynamicTemplates(S | |
}; | ||
} | ||
|
||
private static final IndexVersion MINIMUM_LEGACY_COMPATIBILITY_VERSION = IndexVersion.fromId(5000099); | ||
|
||
public static TypeParser createTypeParserWithLegacySupport(BiFunction<String, MappingParserContext, Builder> builderFunction) { | ||
return new TypeParser(builderFunction, MINIMUM_LEGACY_COMPATIBILITY_VERSION); | ||
} | ||
|
||
/** | ||
* TypeParser implementation that automatically handles parsing | ||
*/ | ||
|
@@ -1632,29 +1638,29 @@ public static final class TypeParser implements Mapper.TypeParser { | |
* @param builderFunction a function that produces a Builder from a name and parsercontext | ||
*/ | ||
public TypeParser(BiFunction<String, MappingParserContext, Builder> builderFunction) { | ||
this(builderFunction, (n, c) -> {}, IndexVersions.MINIMUM_COMPATIBLE); | ||
this(builderFunction, (n, c) -> {}, IndexVersions.MINIMUM_READONLY_COMPATIBLE); | ||
} | ||
|
||
/** | ||
* Variant of {@link #TypeParser(BiFunction)} that allows to defining a minimumCompatibilityVersion to | ||
* Variant of {@link #TypeParser(BiFunction)} that allows to define a minimumCompatibilityVersion to | ||
* allow parsing mapping definitions of legacy indices (see {@link Mapper.TypeParser#supportsVersion(IndexVersion)}). | ||
*/ | ||
public TypeParser(BiFunction<String, MappingParserContext, Builder> builderFunction, IndexVersion minimumCompatibilityVersion) { | ||
private TypeParser(BiFunction<String, MappingParserContext, Builder> builderFunction, IndexVersion minimumCompatibilityVersion) { | ||
this(builderFunction, (n, c) -> {}, minimumCompatibilityVersion); | ||
} | ||
|
||
public TypeParser( | ||
BiFunction<String, MappingParserContext, Builder> builderFunction, | ||
BiConsumer<String, MappingParserContext> contextValidator | ||
) { | ||
this(builderFunction, contextValidator, IndexVersions.MINIMUM_COMPATIBLE); | ||
this(builderFunction, contextValidator, IndexVersions.MINIMUM_READONLY_COMPATIBLE); | ||
} | ||
|
||
public TypeParser( | ||
BiFunction<String, MappingParserContext, Builder> builderFunction, | ||
List<BiConsumer<String, MappingParserContext>> contextValidator | ||
) { | ||
this(builderFunction, (n, c) -> contextValidator.forEach(v -> v.accept(n, c)), IndexVersions.MINIMUM_COMPATIBLE); | ||
this(builderFunction, (n, c) -> contextValidator.forEach(v -> v.accept(n, c)), IndexVersions.MINIMUM_READONLY_COMPATIBLE); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Effectively, lowering the minimum compatible doesn't take any effect, because we were checking the lower bound only for legacy indices which provide their specific lower bound. The default minimum compatible version could have been null for what it mattered. |
||
} | ||
|
||
private TypeParser( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -57,9 +57,13 @@ public MapperRegistry( | |
*/ | ||
public Mapper.TypeParser getMapperParser(String type, IndexVersion indexVersionCreated) { | ||
Mapper.TypeParser parser = mapperParsers.get(type); | ||
if (indexVersionCreated.isLegacyIndexVersion() && (parser == null || parser.supportsVersion(indexVersionCreated) == false)) { | ||
return PlaceHolderFieldMapper.PARSER.apply(type); | ||
if (indexVersionCreated.isLegacyIndexVersion()) { | ||
if (parser == null || parser.supportsVersion(indexVersionCreated) == false) { | ||
return PlaceHolderFieldMapper.PARSER.apply(type); | ||
} | ||
return parser; | ||
} else { | ||
assert parser == null || parser.supportsVersion(indexVersionCreated); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With this assertion, we are now checking the min compatible version for all indices, and making sure that things are consistent. That said, only legacy indices will ever check the version compatibility in prod code. |
||
return parser; | ||
} | ||
} | ||
|
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.
This makes the code a little easier to follow: there is one lower bound legacy compatibility constant, rather than one per mapper. After all the lower bound is always v5. It's also easier to follow what mappers provide legacy support, as they must now all call this method.