Skip to content

Handle legacy mappings with placeholder fields #85059

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

Merged
merged 32 commits into from
Apr 26, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
a70bf43
Placeholder fields
ywelsch Mar 1, 2022
20fd0c1
Merge remote-tracking branch 'elastic/master' into placeholder-fields
ywelsch Mar 22, 2022
d66e928
o
ywelsch Mar 22, 2022
1523ca1
add tests
ywelsch Mar 22, 2022
2b0395b
add tests
ywelsch Mar 22, 2022
42e3911
more foxes
ywelsch Mar 23, 2022
5e1ea96
tests
ywelsch Mar 24, 2022
114dd7f
Merge remote-tracking branch 'elastic/master' into placeholder-fields
ywelsch Mar 24, 2022
2a33e8e
Merge remote-tracking branch 'elastic/master' into placeholder-fields
ywelsch Mar 28, 2022
3e579ec
fix tests
ywelsch Mar 28, 2022
fbd4c4f
second mapping only supported on 5x
ywelsch Mar 28, 2022
b761fd1
Merge remote-tracking branch 'elastic/master' into placeholder-fields
ywelsch Apr 19, 2022
cdf33dd
move legacy mapping handling to factory
ywelsch Apr 20, 2022
d49e1bb
Merge remote-tracking branch 'elastic/master' into placeholder-fields
ywelsch Apr 20, 2022
89e8bfd
fix compilation
ywelsch Apr 20, 2022
5f01d4a
wrong class
ywelsch Apr 20, 2022
6fd1366
rename TypeParser.supportsLegacyField to TypeParser.supportsVersion(V…
ywelsch Apr 20, 2022
713dbc2
move unknownParams downwards
ywelsch Apr 20, 2022
880fbe6
Merge remote-tracking branch 'elastic/master' into placeholder-fields
ywelsch Apr 21, 2022
f75f5d2
Merge remote-tracking branch 'elastic/master' into placeholder-fields
ywelsch Apr 25, 2022
ad99173
place holder tests
ywelsch Apr 25, 2022
07234b1
new default constructor
ywelsch Apr 25, 2022
68408c8
feedback
ywelsch Apr 25, 2022
cb213b1
fix test
ywelsch Apr 25, 2022
2482c26
disalbing warnings is hard
ywelsch Apr 25, 2022
c677101
Merge remote-tracking branch 'elastic/master' into placeholder-fields
ywelsch Apr 25, 2022
b25369e
separate TypeParser interface for Metadata mappers
ywelsch Apr 26, 2022
4c86d3c
Remove getMapperParsers method
ywelsch Apr 26, 2022
f163412
javadocs / rename
ywelsch Apr 26, 2022
0b1157b
spotless
ywelsch Apr 26, 2022
07a2602
rename checkMappingsCompatibility
ywelsch Apr 26, 2022
5b85b55
Merge remote-tracking branch 'elastic/master' into placeholder-fields
ywelsch Apr 26, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,15 @@
import org.elasticsearch.common.settings.IndexScopedSettings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
import org.elasticsearch.core.Nullable;
import org.elasticsearch.index.IndexSettings;
import org.elasticsearch.index.analysis.AnalyzerScope;
import org.elasticsearch.index.analysis.IndexAnalyzers;
import org.elasticsearch.index.analysis.NamedAnalyzer;
import org.elasticsearch.index.mapper.DocumentMapper;
import org.elasticsearch.index.mapper.MapperRegistry;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.index.mapper.Mapping;
import org.elasticsearch.index.similarity.SimilarityService;
import org.elasticsearch.script.ScriptCompiler;
import org.elasticsearch.script.ScriptService;
Expand Down Expand Up @@ -89,7 +92,7 @@ public IndexMetadata verifyIndexMetadata(IndexMetadata indexMetadata, Version mi
// Next we have to run this otherwise if we try to create IndexSettings
// with broken settings it would fail in checkMappingsCompatibility
newMetadata = archiveBrokenIndexSettings(newMetadata);
checkMappingsCompatibility(newMetadata);
createAndValidateMapping(newMetadata);
return newMetadata;
}

Expand Down Expand Up @@ -126,8 +129,10 @@ private static void checkSupportedVersion(IndexMetadata indexMetadata, Version m
* Note that we don't expect users to encounter mapping incompatibilities, since our index compatibility
* policy guarantees we can read mappings from previous compatible index versions. A failure here would
* indicate a compatibility bug (which are unfortunately not that uncommon).
* @return the mapping
*/
private void checkMappingsCompatibility(IndexMetadata indexMetadata) {
@Nullable
public Mapping createAndValidateMapping(IndexMetadata indexMetadata) {
try {

// We cannot instantiate real analysis server or similarity service at this point because the node
Expand Down Expand Up @@ -194,6 +199,8 @@ public Set<Entry<String, NamedAnalyzer>> entrySet() {
scriptService
);
mapperService.merge(indexMetadata, MapperService.MergeReason.MAPPING_RECOVERY);
DocumentMapper documentMapper = mapperService.documentMapper();
return documentMapper == null ? null : documentMapper.mapping();
}
} catch (Exception ex) {
// Wrap the inner exception so we have the index name in the exception message
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,12 @@ private FieldValues<Boolean> scriptValues() {
}
}

public static final TypeParser PARSER = new TypeParser((n, c) -> new Builder(n, c.scriptCompiler(), c.indexVersionCreated()));
private static final Version MINIMUM_COMPATIBILITY_VERSION = Version.fromString("5.0.0");

public static final TypeParser PARSER = new TypeParser(
(n, c) -> new Builder(n, c.scriptCompiler(), c.indexVersionCreated()),
MINIMUM_COMPATIBILITY_VERSION
);

public static final class BooleanFieldType extends TermBasedFieldType {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@

package org.elasticsearch.index.mapper;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.message.ParameterizedMessage;
import org.apache.lucene.document.LongPoint;
import org.apache.lucene.document.SortedNumericDocValuesField;
import org.apache.lucene.document.StoredField;
Expand Down Expand Up @@ -72,6 +75,7 @@
public final class DateFieldMapper extends FieldMapper {

private static final DeprecationLogger DEPRECATION_LOGGER = DeprecationLogger.getLogger(DateFieldMapper.class);
private static final Logger logger = LogManager.getLogger(DateFieldMapper.class);

public static final String CONTENT_TYPE = "date";
public static final String DATE_NANOS_CONTENT_TYPE = "date_nanos";
Expand Down Expand Up @@ -266,7 +270,12 @@ public Builder(
DateFormatter defaultFormat = resolution == Resolution.MILLISECONDS
? DEFAULT_DATE_TIME_FORMATTER
: DEFAULT_DATE_TIME_NANOS_FORMATTER;
this.format = Parameter.stringParam("format", false, m -> toType(m).format, defaultFormat.pattern());
this.format = Parameter.stringParam(
"format",
indexCreatedVersion.isLegacyIndexVersion(),
m -> toType(m).format,
defaultFormat.pattern()
);
if (dateFormatter != null) {
this.format.setValue(dateFormatter.pattern());
this.locale.setValue(dateFormatter.locale());
Expand All @@ -277,7 +286,15 @@ private DateFormatter buildFormatter() {
try {
return DateFormatter.forPattern(format.getValue()).withLocale(locale.getValue());
} catch (IllegalArgumentException e) {
throw new IllegalArgumentException("Error parsing [format] on field [" + name() + "]: " + e.getMessage(), e);
if (indexCreatedVersion.isLegacyIndexVersion()) {
logger.warn(
new ParameterizedMessage("Error parsing format [{}] of legacy index, falling back to default", format.getValue()),
e
);
return DateFormatter.forPattern(format.getDefaultValue()).withLocale(locale.getValue());
} else {
throw new IllegalArgumentException("Error parsing [format] on field [" + name() + "]: " + e.getMessage(), e);
}
}
}

Expand Down Expand Up @@ -341,6 +358,8 @@ public DateFieldMapper build(MapperBuilderContext context) {
}
}

private static final Version MINIMUM_COMPATIBILITY_VERSION = Version.fromString("5.0.0");

public static final TypeParser MILLIS_PARSER = new TypeParser((n, c) -> {
boolean ignoreMalformedByDefault = IGNORE_MALFORMED_SETTING.get(c.getSettings());
return new Builder(
Expand All @@ -351,7 +370,7 @@ public DateFieldMapper build(MapperBuilderContext context) {
ignoreMalformedByDefault,
c.indexVersionCreated()
);
});
}, MINIMUM_COMPATIBILITY_VERSION);

public static final TypeParser NANOS_PARSER = new TypeParser((n, c) -> {
boolean ignoreMalformedByDefault = IGNORE_MALFORMED_SETTING.get(c.getSettings());
Expand All @@ -363,7 +382,7 @@ public DateFieldMapper build(MapperBuilderContext context) {
ignoreMalformedByDefault,
c.indexVersionCreated()
);
});
}, MINIMUM_COMPATIBILITY_VERSION);

public static final class DateFieldType extends MappedFieldType {
protected final DateFormatter dateTimeFormatter;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

package org.elasticsearch.index.mapper;

import org.elasticsearch.Version;
import org.elasticsearch.common.xcontent.support.XContentMapValues;
import org.elasticsearch.xcontent.XContentBuilder;

Expand Down Expand Up @@ -124,6 +125,11 @@ public Mapper.Builder parse(String name, Map<String, Object> node, MappingParser
}
return builder.path(path);
}

@Override
public boolean supportsVersion(Version indexCreatedVersion) {
return true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this I find a bit surprising especially given that field aliases were added with 6.4 . I wonder if this is never called, in which case maybe we should throw exception instead, or if it could rely on the default impl instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even though field aliases were only added in ES 6.4, they could be used with older indices as well (i.e. you had a 5.x index for example that you kept using in 6.4 and now added the field alias to the mapping of that 5.x index). As field aliases are more of a runtime property (e.g. just like runtime fields, which you could also use on indices created before the release of runtime fields) and have no connection to the actual data, I see no reason to limit their use.

}
}

public static class Builder extends Mapper.Builder {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1191,7 +1191,7 @@ public Builder init(FieldMapper initializer) {
return this;
}

private void merge(FieldMapper in, Conflicts conflicts) {
protected void merge(FieldMapper in, Conflicts conflicts) {
for (Parameter<?> param : getParameters()) {
param.merge(in, conflicts);
}
Expand Down Expand Up @@ -1238,7 +1238,7 @@ protected void addScriptValidation(
* Writes the current builder parameter values as XContent
*/
@Override
public final XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
boolean includeDefaults = params.paramAsBoolean("include_defaults", false);
for (Parameter<?> parameter : getParameters()) {
parameter.toXContent(builder, includeDefaults);
Expand Down Expand Up @@ -1304,6 +1304,12 @@ public final void parse(String name, MappingParserContext parserContext, Map<Str
parameter = paramsMap.get(propName);
}
if (parameter == null) {
if (parserContext.indexVersionCreated().isLegacyIndexVersion()) {
// ignore unknown parameters on legacy indices
handleUnknownParamOnLegacyIndex(propName, propNode);
iterator.remove();
continue;
}
if (isDeprecatedParameter(propName, parserContext.indexVersionCreated())) {
deprecationLogger.warn(
DeprecationCategory.API,
Expand Down Expand Up @@ -1352,6 +1358,10 @@ public final void parse(String name, MappingParserContext parserContext, Map<Str
validate();
}

protected void handleUnknownParamOnLegacyIndex(String propName, Object propNode) {
// ignore
}

protected static ContentPath parentPath(String name) {
int endPos = name.lastIndexOf(".");
if (endPos == -1) {
Expand Down Expand Up @@ -1388,21 +1398,39 @@ public static final class TypeParser implements Mapper.TypeParser {

private final BiFunction<String, MappingParserContext, Builder> builderFunction;
private final BiConsumer<String, MappingParserContext> contextValidator;
private final Version minimumCompatibilityVersion; // see Mapper.TypeParser#supportsVersion()

/**
* Creates a new 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) -> {});
this(builderFunction, (n, c) -> {}, Version.CURRENT.minimumIndexCompatibilityVersion());
}

/**
* Variant of {@link #TypeParser(BiFunction)} that allows to defining a minimumCompatibilityVersion to
* allow parsing mapping definitions of legacy indices (see {@link Mapper.TypeParser#supportsVersion(Version)}).
*/
public TypeParser(BiFunction<String, MappingParserContext, Builder> builderFunction, Version minimumCompatibilityVersion) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it help to add javadocs here to clarify when to use which constructor? as far as I understand, the constructor that takes the version is the one called by all the mappers that are supported on legacy indices, all the rest remains the same?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

addressed in f163412

this(builderFunction, (n, c) -> {}, minimumCompatibilityVersion);
}

public TypeParser(
BiFunction<String, MappingParserContext, Builder> builderFunction,
BiConsumer<String, MappingParserContext> contextValidator
) {
this(builderFunction, contextValidator, Version.CURRENT.minimumIndexCompatibilityVersion());
}

private TypeParser(
BiFunction<String, MappingParserContext, Builder> builderFunction,
BiConsumer<String, MappingParserContext> contextValidator,
Version minimumCompatibilityVersion
) {
this.builderFunction = builderFunction;
this.contextValidator = contextValidator;
this.minimumCompatibilityVersion = minimumCompatibilityVersion;
}

@Override
Expand All @@ -1412,6 +1440,11 @@ public Builder parse(String name, Map<String, Object> node, MappingParserContext
builder.parse(name, parserContext, node);
return builder;
}

@Override
public boolean supportsVersion(Version indexCreatedVersion) {
return indexCreatedVersion.onOrAfter(minimumCompatibilityVersion);
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -162,8 +162,11 @@ public FieldMapper build(MapperBuilderContext context) {

}

private static final Version MINIMUM_COMPATIBILITY_VERSION = Version.fromString("5.0.0");

public static TypeParser PARSER = new TypeParser(
(n, c) -> new Builder(n, c.scriptCompiler(), IGNORE_MALFORMED_SETTING.get(c.getSettings()), c.indexVersionCreated())
(n, c) -> new Builder(n, c.scriptCompiler(), IGNORE_MALFORMED_SETTING.get(c.getSettings()), c.indexVersionCreated()),
MINIMUM_COMPATIBILITY_VERSION
);

private final Builder builder;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,10 +175,12 @@ public IpFieldMapper build(MapperBuilderContext context) {

}

private static final Version MINIMUM_COMPATIBILITY_VERSION = Version.fromString("5.0.0");

public static final TypeParser PARSER = new TypeParser((n, c) -> {
boolean ignoreMalformedByDefault = IGNORE_MALFORMED_SETTING.get(c.getSettings());
return new Builder(n, c.scriptCompiler(), ignoreMalformedByDefault, c.indexVersionCreated());
});
}, MINIMUM_COMPATIBILITY_VERSION);

public static final class IpFieldType extends SimpleMappedFieldType {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@

package org.elasticsearch.index.mapper;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.message.ParameterizedMessage;
import org.apache.lucene.analysis.TokenStream;
import org.apache.lucene.analysis.tokenattributes.CharTermAttribute;
import org.apache.lucene.document.Field;
Expand Down Expand Up @@ -78,6 +81,8 @@
*/
public final class KeywordFieldMapper extends FieldMapper {

private static final Logger logger = LogManager.getLogger(KeywordFieldMapper.class);

public static final String CONTENT_TYPE = "keyword";

public static class Defaults {
Expand Down Expand Up @@ -131,8 +136,7 @@ public static class Builder extends FieldMapper.Builder {
private final Parameter<Boolean> hasNorms = TextParams.norms(false, m -> toType(m).fieldType.omitNorms() == false);
private final Parameter<SimilarityProvider> similarity = TextParams.similarity(m -> toType(m).similarity);

private final Parameter<String> normalizer = Parameter.stringParam("normalizer", false, m -> toType(m).normalizerName, null)
.acceptsNull();
private final Parameter<String> normalizer;

private final Parameter<Boolean> splitQueriesOnWhitespace = Parameter.boolParam(
"split_queries_on_whitespace",
Expand All @@ -156,6 +160,12 @@ public Builder(String name, IndexAnalyzers indexAnalyzers, ScriptCompiler script
this.indexAnalyzers = indexAnalyzers;
this.scriptCompiler = Objects.requireNonNull(scriptCompiler);
this.indexCreatedVersion = Objects.requireNonNull(indexCreatedVersion);
this.normalizer = Parameter.stringParam(
"normalizer",
indexCreatedVersion.isLegacyIndexVersion(),
m -> toType(m).normalizerName,
null
).acceptsNull();
this.script.precludesParameters(nullValue);
addScriptValidation(script, indexed, hasDocValues);

Expand Down Expand Up @@ -245,7 +255,17 @@ private KeywordFieldType buildFieldType(MapperBuilderContext context, FieldType
assert indexAnalyzers != null;
normalizer = indexAnalyzers.getNormalizer(normalizerName);
if (normalizer == null) {
throw new MapperParsingException("normalizer [" + normalizerName + "] not found for field [" + name + "]");
if (indexCreatedVersion.isLegacyIndexVersion()) {
logger.warn(
new ParameterizedMessage(
"Could not find normalizer [{}] of legacy index, falling back to default",
normalizerName
)
);
normalizer = Lucene.KEYWORD_ANALYZER;
} else {
throw new MapperParsingException("normalizer [" + normalizerName + "] not found for field [" + name + "]");
}
}
searchAnalyzer = quoteAnalyzer = normalizer;
if (splitQueriesOnWhitespace.getValue()) {
Expand Down Expand Up @@ -274,8 +294,11 @@ public KeywordFieldMapper build(MapperBuilderContext context) {
}
}

private static final Version MINIMUM_COMPATIBILITY_VERSION = Version.fromString("5.0.0");

public static final TypeParser PARSER = new TypeParser(
(n, c) -> new Builder(n, c.getIndexAnalyzers(), c.scriptCompiler(), c.indexVersionCreated())
(n, c) -> new Builder(n, c.getIndexAnalyzers(), c.scriptCompiler(), c.indexVersionCreated()),
MINIMUM_COMPATIBILITY_VERSION
);

public static final class KeywordFieldType extends StringFieldType {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

package org.elasticsearch.index.mapper;

import org.elasticsearch.Version;
import org.elasticsearch.common.Strings;
import org.elasticsearch.xcontent.ToXContentFragment;

Expand All @@ -34,6 +35,13 @@ public String name() {

public interface TypeParser {
Mapper.Builder parse(String name, Map<String, Object> node, MappingParserContext parserContext) throws MapperParsingException;

/**
* Whether we can parse this type on indices with the given index created version.
*/
default boolean supportsVersion(Version indexCreatedVersion) {
return indexCreatedVersion.onOrAfter(Version.CURRENT.minimumIndexCompatibilityVersion());
}
}

private final String simpleName;
Expand Down
Loading