From 4e3c5488fe73d0159534afa7653e981fe9368549 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Thu, 23 Mar 2023 13:37:58 +0000 Subject: [PATCH 1/4] WIP: TextFieldMapper failures --- .../index/mapper/TextFieldMapper.java | 65 +++--- .../index/mapper/TextParams.java | 185 +++++++++++------- 2 files changed, 152 insertions(+), 98 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/TextFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/TextFieldMapper.java index d4ebba385940f..70ed682b22117 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/TextFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/TextFieldMapper.java @@ -274,7 +274,8 @@ public static class Builder extends FieldMapper.Builder { private final Parameter> meta = Parameter.metaParam(); - final TextParams.Analyzers analyzers; + final TextParams.AnalyzerParameters analyzers; + final IndexAnalyzers indexAnalyzers; public Builder(String name, IndexAnalyzers indexAnalyzers) { this(name, Version.CURRENT, indexAnalyzers); @@ -283,10 +284,9 @@ public Builder(String name, IndexAnalyzers indexAnalyzers) { public Builder(String name, Version indexCreatedVersion, IndexAnalyzers indexAnalyzers) { super(name); this.indexCreatedVersion = indexCreatedVersion; - this.analyzers = new TextParams.Analyzers( - indexAnalyzers, - m -> ((TextFieldMapper) m).indexAnalyzer, - m -> (((TextFieldMapper) m).positionIncrementGap), + this.indexAnalyzers = indexAnalyzers; + this.analyzers = new TextParams.AnalyzerParameters( + m -> ((TextFieldMapper) m).analyzerConfiguration, indexCreatedVersion ); } @@ -341,17 +341,11 @@ private TextFieldType buildFieldType( FieldType fieldType, MultiFields multiFields, MapperBuilderContext context, - Version indexCreatedVersion + Version indexCreatedVersion, + TextParams.Analyzers analyzers ) { - NamedAnalyzer searchAnalyzer = analyzers.getSearchAnalyzer(); - NamedAnalyzer searchQuoteAnalyzer = analyzers.getSearchQuoteAnalyzer(); - if (analyzers.positionIncrementGap.isConfigured()) { - if (fieldType.indexOptions().compareTo(IndexOptions.DOCS_AND_FREQS_AND_POSITIONS) < 0) { - throw new IllegalArgumentException( - "Cannot set position_increment_gap on field [" + name + "] without positions enabled" - ); - } - } + NamedAnalyzer searchAnalyzer = analyzers.searchAnalyzer(); + NamedAnalyzer searchQuoteAnalyzer = analyzers.searchQuoteAnalyzer(); TextSearchInfo tsi = new TextSearchInfo(fieldType, similarity.getValue(), searchAnalyzer, searchQuoteAnalyzer); TextFieldType ft; if (indexCreatedVersion.isLegacyIndexVersion()) { @@ -390,7 +384,7 @@ private KeywordFieldMapper.KeywordFieldType syntheticSourceDelegate(FieldType fi return null; } - private SubFieldInfo buildPrefixInfo(MapperBuilderContext context, FieldType fieldType, TextFieldType tft) { + private SubFieldInfo buildPrefixInfo(MapperBuilderContext context, TextParams.Analyzers analyzers, FieldType fieldType, TextFieldType tft) { if (indexPrefixes.get() == null) { return null; } @@ -423,15 +417,15 @@ private SubFieldInfo buildPrefixInfo(MapperBuilderContext context, FieldType fie fullName + "._index_prefix", pft, new PrefixWrappedAnalyzer( - analyzers.getIndexAnalyzer().analyzer(), - analyzers.positionIncrementGap.get(), + analyzers.indexAnalyzer().analyzer(), + analyzers.configuration().posIncrementGap(), indexPrefixes.get().minChars, indexPrefixes.get().maxChars ) ); } - private SubFieldInfo buildPhraseInfo(FieldType fieldType, TextFieldType parent) { + private SubFieldInfo buildPhraseInfo(TextParams.Analyzers analyzers, FieldType fieldType, TextFieldType parent) { if (indexPhrases.get() == false) { return null; } @@ -444,15 +438,14 @@ private SubFieldInfo buildPhraseInfo(FieldType fieldType, TextFieldType parent) FieldType phraseFieldType = new FieldType(fieldType); parent.setIndexPhrases(); PhraseWrappedAnalyzer a = new PhraseWrappedAnalyzer( - analyzers.getIndexAnalyzer().analyzer(), - analyzers.positionIncrementGap.get() + analyzers.indexAnalyzer().analyzer(), + analyzers.configuration().posIncrementGap() ); return new SubFieldInfo(parent.name() + FAST_PHRASE_SUFFIX, phraseFieldType, a); } - public Map indexAnalyzers(String name, SubFieldInfo phraseFieldInfo, SubFieldInfo prefixFieldInfo) { + private Map indexAnalyzers(String name, NamedAnalyzer main, SubFieldInfo phraseFieldInfo, SubFieldInfo prefixFieldInfo) { Map analyzers = new HashMap<>(); - NamedAnalyzer main = this.analyzers.getIndexAnalyzer(); analyzers.put(name, main); if (phraseFieldInfo != null) { analyzers.put( @@ -471,6 +464,7 @@ public Map indexAnalyzers(String name, SubFieldInfo phras @Override public TextFieldMapper build(MapperBuilderContext context) { + TextParams.Analyzers analyzers = this.analyzers.buildAnalyzers(indexAnalyzers); MultiFields multiFields = multiFieldsBuilder.build(this, context); FieldType fieldType = TextParams.buildFieldType( index, @@ -480,9 +474,16 @@ public TextFieldMapper build(MapperBuilderContext context) { indexCreatedVersion.isLegacyIndexVersion() ? () -> false : norms, termVectors ); - TextFieldType tft = buildFieldType(fieldType, multiFields, context, indexCreatedVersion); - SubFieldInfo phraseFieldInfo = buildPhraseInfo(fieldType, tft); - SubFieldInfo prefixFieldInfo = buildPrefixInfo(context, fieldType, tft); + if (this.analyzers.positionIncrementGap.isConfigured()) { + if (fieldType.indexOptions().compareTo(IndexOptions.DOCS_AND_FREQS_AND_POSITIONS) < 0) { + throw new IllegalArgumentException( + "Cannot set position_increment_gap on field [" + name + "] without positions enabled" + ); + } + } + TextFieldType tft = buildFieldType(fieldType, multiFields, context, indexCreatedVersion, analyzers); + SubFieldInfo phraseFieldInfo = buildPhraseInfo(analyzers, fieldType, tft); + SubFieldInfo prefixFieldInfo = buildPrefixInfo(context, analyzers, fieldType, tft); for (Mapper mapper : multiFields) { if (mapper.name().endsWith(FAST_PHRASE_SUFFIX) || mapper.name().endsWith(FAST_PREFIX_SUFFIX)) { throw new MapperParsingException("Cannot use reserved field name [" + mapper.name() + "]"); @@ -492,9 +493,10 @@ public TextFieldMapper build(MapperBuilderContext context) { name, fieldType, tft, - indexAnalyzers(tft.name(), phraseFieldInfo, prefixFieldInfo), + indexAnalyzers(tft.name(), analyzers.indexAnalyzer(), phraseFieldInfo, prefixFieldInfo), prefixFieldInfo, phraseFieldInfo, + analyzers, multiFields, copyTo.build(), this @@ -1150,16 +1152,18 @@ public Query existsQuery(SearchExecutionContext context) { private final FieldType fieldType; private final SubFieldInfo prefixFieldInfo; private final SubFieldInfo phraseFieldInfo; + private final TextParams.AnalyzerConfiguration analyzerConfiguration; private final Map indexAnalyzerMap; - protected TextFieldMapper( + private TextFieldMapper( String simpleName, FieldType fieldType, TextFieldType mappedFieldType, Map indexAnalyzers, SubFieldInfo prefixFieldInfo, SubFieldInfo phraseFieldInfo, + TextParams.Analyzers analyzers, MultiFields multiFields, CopyTo copyTo, Builder builder @@ -1174,8 +1178,8 @@ protected TextFieldMapper( this.prefixFieldInfo = prefixFieldInfo; this.phraseFieldInfo = phraseFieldInfo; this.indexCreatedVersion = builder.indexCreatedVersion; - this.indexAnalyzer = builder.analyzers.getIndexAnalyzer(); - this.indexAnalyzers = builder.analyzers.indexAnalyzers; + this.indexAnalyzer = analyzers.indexAnalyzer(); + this.indexAnalyzers = builder.indexAnalyzers; this.positionIncrementGap = builder.analyzers.positionIncrementGap.getValue(); this.index = builder.index.getValue(); this.store = builder.store.getValue(); @@ -1189,6 +1193,7 @@ protected TextFieldMapper( this.fieldData = builder.fieldData.get(); this.indexPhrases = builder.indexPhrases.getValue(); this.indexAnalyzerMap = Map.copyOf(indexAnalyzers); + this.analyzerConfiguration = analyzers.configuration(); } @Override diff --git a/server/src/main/java/org/elasticsearch/index/mapper/TextParams.java b/server/src/main/java/org/elasticsearch/index/mapper/TextParams.java index 8859f97624457..a2b14b9e73e89 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/TextParams.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/TextParams.java @@ -12,7 +12,6 @@ import org.apache.lucene.index.IndexOptions; import org.elasticsearch.Version; import org.elasticsearch.index.analysis.AnalysisMode; -import org.elasticsearch.index.analysis.AnalysisRegistry; import org.elasticsearch.index.analysis.IndexAnalyzers; import org.elasticsearch.index.analysis.NamedAnalyzer; import org.elasticsearch.index.mapper.FieldMapper.Parameter; @@ -29,96 +28,146 @@ public final class TextParams { private TextParams() {} - public static final class Analyzers { - public final Parameter indexAnalyzer; - public final Parameter searchAnalyzer; - public final Parameter searchQuoteAnalyzer; + /* + * i configured s configured sq configured return + * i y i + * i n default index analyzer + * s * y s + * s y n i + * s n n default search analyzer + * sq * * y sq + * sq * y n s + * sq y n n i + * sq n n n default search quote analyzer + * + * + * + * */ + + public record AnalyzerConfiguration(String indexAnalyzer, String searchAnalyzer, String searchQuoteAnalyzer, int posIncrementGap) { + + public Analyzers buildAnalyzers(IndexAnalyzers indexAnalyzers) { + return new Analyzers( + wrap(buildIndexAnalyzer(indexAnalyzers), AnalysisMode.INDEX_TIME), + wrap(buildSearchAnalyzer(indexAnalyzers), AnalysisMode.SEARCH_TIME), + wrap(buildSearchQuoteAnalyzer(indexAnalyzers), AnalysisMode.SEARCH_TIME), + this + ); + } + + private NamedAnalyzer buildIndexAnalyzer(IndexAnalyzers indexAnalyzers) { + if (this.indexAnalyzer == null) { + return indexAnalyzers.getDefaultIndexAnalyzer(); + } + NamedAnalyzer a = indexAnalyzers.get(this.indexAnalyzer); + if (a == null) { + throw new IllegalArgumentException("Unknown analyzer [" + this.indexAnalyzer + "]"); + } + return a; + } + + private NamedAnalyzer buildSearchAnalyzer(IndexAnalyzers indexAnalyzers) { + if (this.searchAnalyzer == null) { + if (this.indexAnalyzer == null) { + return indexAnalyzers.getDefaultSearchAnalyzer(); + } + return indexAnalyzers.get(this.indexAnalyzer); // null check will already have happened in buildIndexAnalyzer + } + NamedAnalyzer a = indexAnalyzers.get(this.searchAnalyzer); + if (a == null) { + throw new IllegalArgumentException("Unknown analyzer [" + this.searchAnalyzer + "]"); + } + return a; + } + + private NamedAnalyzer buildSearchQuoteAnalyzer(IndexAnalyzers indexAnalyzers) { + if (this.searchQuoteAnalyzer == null) { + if (this.searchAnalyzer == null) { + if (this.indexAnalyzer == null) { + return indexAnalyzers.getDefaultSearchQuoteAnalyzer(); + } + return indexAnalyzers.get(this.indexAnalyzer); // null checked already in buildIndexAnalyzer + } + return indexAnalyzers.get(this.searchAnalyzer); // null checked already in buildSearchAnalyzer + } + NamedAnalyzer a = indexAnalyzers.get(this.searchQuoteAnalyzer); + if (a == null) { + throw new IllegalArgumentException("Unknown analyzer [" + this.searchQuoteAnalyzer + "]"); + } + return a; + } + + private NamedAnalyzer wrap(NamedAnalyzer in, AnalysisMode analysisMode) { + in.checkAllowedInMode(analysisMode); + if (in.getPositionIncrementGap("") != posIncrementGap) { + return new NamedAnalyzer(in, posIncrementGap); + } + return in; + } + + } + + public record Analyzers( + NamedAnalyzer indexAnalyzer, + NamedAnalyzer searchAnalyzer, + NamedAnalyzer searchQuoteAnalyzer, + AnalyzerConfiguration configuration + ) {} + + public static final class AnalyzerParameters { + public final Parameter indexAnalyzer; + public final Parameter searchAnalyzer; + public final Parameter searchQuoteAnalyzer; public final Parameter positionIncrementGap; - public final IndexAnalyzers indexAnalyzers; - public Analyzers( - IndexAnalyzers indexAnalyzers, - Function analyzerInitFunction, - Function positionGapInitFunction, + public AnalyzerParameters( + Function analyzerInitFunction, Version indexCreatedVersion ) { - this.indexAnalyzer = Parameter.analyzerParam( + this.indexAnalyzer = Parameter.stringParam( "analyzer", indexCreatedVersion.isLegacyIndexVersion(), - analyzerInitFunction, - indexAnalyzers::getDefaultIndexAnalyzer, - indexCreatedVersion + mapper -> analyzerInitFunction.apply(mapper).indexAnalyzer, + null ) - .setSerializerCheck( - (id, ic, a) -> id - || ic - || Objects.equals(a, getSearchAnalyzer()) == false - || Objects.equals(a, getSearchQuoteAnalyzer()) == false - ) - .addValidator(a -> a.checkAllowedInMode(AnalysisMode.INDEX_TIME)); - this.searchAnalyzer = Parameter.analyzerParam( + .setSerializerCheck((includeDefaults, isConfigured, value) -> value != null) + .setMergeValidator( + // special case - we allow 'default' to be merged in to an unconfigured analyzer + (previous, toMerge, conflicts) -> Objects.equals(previous, toMerge) || (previous == null && "default".equals(toMerge)) + ); + this.searchAnalyzer = Parameter.stringParam( "search_analyzer", true, - m -> m.fieldType().getTextSearchInfo().searchAnalyzer(), - () -> { - if (indexAnalyzer.isConfigured() == false) { - NamedAnalyzer defaultAnalyzer = indexAnalyzers.get(AnalysisRegistry.DEFAULT_SEARCH_ANALYZER_NAME); - if (defaultAnalyzer != null) { - return defaultAnalyzer; - } - } - return indexAnalyzer.get(); - }, - indexCreatedVersion - ) - .setSerializerCheck((id, ic, a) -> id || ic || Objects.equals(a, getSearchQuoteAnalyzer()) == false) - .addValidator(a -> a.checkAllowedInMode(AnalysisMode.SEARCH_TIME)); - this.searchQuoteAnalyzer = Parameter.analyzerParam( + mapper -> analyzerInitFunction.apply(mapper).searchAnalyzer, + null + ).setSerializerCheck((includeDefaults, isConfigured, value) -> value != null); + this.searchQuoteAnalyzer = Parameter.stringParam( "search_quote_analyzer", true, - m -> m.fieldType().getTextSearchInfo().searchQuoteAnalyzer(), - () -> { - if (searchAnalyzer.isConfigured() == false && indexAnalyzer.isConfigured() == false) { - NamedAnalyzer defaultAnalyzer = indexAnalyzers.get(AnalysisRegistry.DEFAULT_SEARCH_QUOTED_ANALYZER_NAME); - if (defaultAnalyzer != null) { - return defaultAnalyzer; - } - } - return searchAnalyzer.get(); - }, - indexCreatedVersion - ).addValidator(a -> a.checkAllowedInMode(AnalysisMode.SEARCH_TIME)); + mapper -> analyzerInitFunction.apply(mapper).searchQuoteAnalyzer, + null + ).setSerializerCheck((includeDefaults, isConfigured, value) -> value != null); this.positionIncrementGap = Parameter.intParam( "position_increment_gap", false, - positionGapInitFunction, + mapper -> analyzerInitFunction.apply(mapper).posIncrementGap, TextFieldMapper.Defaults.POSITION_INCREMENT_GAP ).addValidator(v -> { if (v < 0) { throw new MapperParsingException("[position_increment_gap] must be positive, got [" + v + "]"); } }); - this.indexAnalyzers = indexAnalyzers; - } - - public NamedAnalyzer getIndexAnalyzer() { - return wrapAnalyzer(indexAnalyzer.getValue()); - } - - public NamedAnalyzer getSearchAnalyzer() { - return wrapAnalyzer(searchAnalyzer.getValue()); - } - - public NamedAnalyzer getSearchQuoteAnalyzer() { - return wrapAnalyzer(searchQuoteAnalyzer.getValue()); } - private NamedAnalyzer wrapAnalyzer(NamedAnalyzer a) { - if (positionIncrementGap.isConfigured() == false) { - return a; - } - return new NamedAnalyzer(a, positionIncrementGap.get()); + public Analyzers buildAnalyzers(IndexAnalyzers indexAnalyzers) { + AnalyzerConfiguration config = new AnalyzerConfiguration( + indexAnalyzer.get(), + searchAnalyzer.get(), + searchQuoteAnalyzer.get(), + positionIncrementGap.get() + ); + return config.buildAnalyzers(indexAnalyzers); } } From ddd666cda9ea81de4c86b3787219761ecf29b0e8 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Thu, 30 Mar 2023 14:31:58 +0100 Subject: [PATCH 2/4] more wipp, some :server:test tests failing --- .../extras/MatchOnlyTextFieldMapper.java | 34 +++++------ .../extras/SearchAsYouTypeFieldMapper.java | 25 +++++---- .../index/mapper/TextParams.java | 56 +++++++++++++------ .../MetadataIndexTemplateServiceTests.java | 2 +- .../index/mapper/TextFieldMapperTests.java | 10 ++-- 5 files changed, 76 insertions(+), 51 deletions(-) diff --git a/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/MatchOnlyTextFieldMapper.java b/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/MatchOnlyTextFieldMapper.java index cffa2e9f11b1e..17f5d00fa4cc3 100644 --- a/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/MatchOnlyTextFieldMapper.java +++ b/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/MatchOnlyTextFieldMapper.java @@ -95,7 +95,8 @@ public static class Builder extends FieldMapper.Builder { private final Parameter> meta = Parameter.metaParam(); - private final TextParams.Analyzers analyzers; + final TextParams.AnalyzerParameters analyzers; + final IndexAnalyzers indexAnalyzers; public Builder(String name, IndexAnalyzers indexAnalyzers) { this(name, Version.CURRENT, indexAnalyzers); @@ -104,10 +105,9 @@ public Builder(String name, IndexAnalyzers indexAnalyzers) { public Builder(String name, Version indexCreatedVersion, IndexAnalyzers indexAnalyzers) { super(name); this.indexCreatedVersion = indexCreatedVersion; - this.analyzers = new TextParams.Analyzers( - indexAnalyzers, - m -> ((MatchOnlyTextFieldMapper) m).indexAnalyzer, - m -> ((MatchOnlyTextFieldMapper) m).positionIncrementGap, + this.indexAnalyzers = indexAnalyzers; + this.analyzers = new TextParams.AnalyzerParameters( + m -> ((MatchOnlyTextFieldMapper) m).analyzerConfiguration, indexCreatedVersion ); } @@ -117,24 +117,24 @@ protected Parameter[] getParameters() { return new Parameter[] { meta }; } - private MatchOnlyTextFieldType buildFieldType(MapperBuilderContext context) { - NamedAnalyzer searchAnalyzer = analyzers.getSearchAnalyzer(); - NamedAnalyzer searchQuoteAnalyzer = analyzers.getSearchQuoteAnalyzer(); - NamedAnalyzer indexAnalyzer = analyzers.getIndexAnalyzer(); + private MatchOnlyTextFieldType buildFieldType(MapperBuilderContext context, TextParams.Analyzers analyzers) { + NamedAnalyzer searchAnalyzer = analyzers.searchAnalyzer(); + NamedAnalyzer searchQuoteAnalyzer = analyzers.searchQuoteAnalyzer(); + NamedAnalyzer indexAnalyzer = analyzers.indexAnalyzer(); TextSearchInfo tsi = new TextSearchInfo(Defaults.FIELD_TYPE, null, searchAnalyzer, searchQuoteAnalyzer); - MatchOnlyTextFieldType ft = new MatchOnlyTextFieldType( + return new MatchOnlyTextFieldType( context.buildFullName(name), tsi, indexAnalyzer, context.isSourceSynthetic(), meta.getValue() ); - return ft; } @Override public MatchOnlyTextFieldMapper build(MapperBuilderContext context) { - MatchOnlyTextFieldType tft = buildFieldType(context); + TextParams.Analyzers analyzers = this.analyzers.buildAnalyzers(this.indexAnalyzers); + MatchOnlyTextFieldType tft = buildFieldType(context, analyzers); MultiFields multiFields = multiFieldsBuilder.build(this, context); return new MatchOnlyTextFieldMapper( name, @@ -143,6 +143,7 @@ public MatchOnlyTextFieldMapper build(MapperBuilderContext context) { multiFields, copyTo.build(), context.isSourceSynthetic(), + analyzers, this ); } @@ -359,9 +360,9 @@ private String storedFieldNameForSyntheticSource() { private final Version indexCreatedVersion; private final IndexAnalyzers indexAnalyzers; private final NamedAnalyzer indexAnalyzer; - private final int positionIncrementGap; private final boolean storeSource; private final FieldType fieldType; + private final TextParams.AnalyzerConfiguration analyzerConfiguration; private MatchOnlyTextFieldMapper( String simpleName, @@ -370,6 +371,7 @@ private MatchOnlyTextFieldMapper( MultiFields multiFields, CopyTo copyTo, boolean storeSource, + TextParams.Analyzers analyzers, Builder builder ) { super(simpleName, mappedFieldType, multiFields, copyTo, false, null); @@ -377,9 +379,9 @@ private MatchOnlyTextFieldMapper( assert mappedFieldType.hasDocValues() == false; this.fieldType = fieldType; this.indexCreatedVersion = builder.indexCreatedVersion; - this.indexAnalyzers = builder.analyzers.indexAnalyzers; - this.indexAnalyzer = builder.analyzers.getIndexAnalyzer(); - this.positionIncrementGap = builder.analyzers.positionIncrementGap.getValue(); + this.indexAnalyzers = builder.indexAnalyzers; + this.indexAnalyzer = analyzers.indexAnalyzer(); + this.analyzerConfiguration = analyzers.configuration(); this.storeSource = storeSource; } diff --git a/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/SearchAsYouTypeFieldMapper.java b/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/SearchAsYouTypeFieldMapper.java index f7819ba8a34e2..11008f159cc34 100644 --- a/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/SearchAsYouTypeFieldMapper.java +++ b/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/SearchAsYouTypeFieldMapper.java @@ -133,7 +133,8 @@ public static class Builder extends FieldMapper.Builder { } }).alwaysSerialize(); - final TextParams.Analyzers analyzers; + final IndexAnalyzers indexAnalyzers; + final TextParams.AnalyzerParameters analyzers; final Parameter similarity = TextParams.similarity(m -> builder(m).similarity.get()); final Parameter indexOptions = TextParams.textIndexOptions(m -> builder(m).indexOptions.get()); @@ -147,10 +148,9 @@ public static class Builder extends FieldMapper.Builder { public Builder(String name, Version indexCreatedVersion, IndexAnalyzers indexAnalyzers) { super(name); this.indexCreatedVersion = indexCreatedVersion; - this.analyzers = new TextParams.Analyzers( - indexAnalyzers, - m -> builder(m).analyzers.getIndexAnalyzer(), - m -> builder(m).analyzers.positionIncrementGap.getValue(), + this.indexAnalyzers = indexAnalyzers; + this.analyzers = new TextParams.AnalyzerParameters( + m -> ((SearchAsYouTypeFieldMapper) m).analyzerConfiguration, indexCreatedVersion ); } @@ -183,15 +183,16 @@ public SearchAsYouTypeFieldMapper build(MapperBuilderContext context) { Map indexAnalyzers = new HashMap<>(); - NamedAnalyzer indexAnalyzer = analyzers.getIndexAnalyzer(); - NamedAnalyzer searchAnalyzer = analyzers.getSearchAnalyzer(); + TextParams.Analyzers analyzers = this.analyzers.buildAnalyzers(this.indexAnalyzers); + NamedAnalyzer indexAnalyzer = analyzers.indexAnalyzer(); + NamedAnalyzer searchAnalyzer = analyzers.searchAnalyzer(); SearchAsYouTypeFieldType ft = new SearchAsYouTypeFieldType( context.buildFullName(name), fieldType, similarity.getValue(), - analyzers.getSearchAnalyzer(), - analyzers.getSearchQuoteAnalyzer(), + analyzers.searchAnalyzer(), + analyzers.searchQuoteAnalyzer(), meta.getValue() ); @@ -267,6 +268,7 @@ public SearchAsYouTypeFieldMapper build(MapperBuilderContext context) { prefixFieldMapper, shingleFieldMappers, multiFieldsBuilder.build(this, context), + analyzers.configuration(), this ); } @@ -666,6 +668,7 @@ public SpanQuery spanPrefixQuery(String value, SpanMultiTermQueryWrapper.SpanRew private final PrefixFieldMapper prefixField; private final ShingleFieldMapper[] shingleFields; private final Builder builder; + private final TextParams.AnalyzerConfiguration analyzerConfiguration; private final Map indexAnalyzers; @@ -677,6 +680,7 @@ public SearchAsYouTypeFieldMapper( PrefixFieldMapper prefixField, ShingleFieldMapper[] shingleFields, MultiFields multiFields, + TextParams.AnalyzerConfiguration analyzerConfiguration, Builder builder ) { super(simpleName, mappedFieldType, multiFields, copyTo, false, null); @@ -685,6 +689,7 @@ public SearchAsYouTypeFieldMapper( this.maxShingleSize = builder.maxShingleSize.getValue(); this.builder = builder; this.indexAnalyzers = Map.copyOf(indexAnalyzers); + this.analyzerConfiguration = analyzerConfiguration; } @Override @@ -721,7 +726,7 @@ protected String contentType() { } public FieldMapper.Builder getMergeBuilder() { - return new Builder(simpleName(), builder.indexCreatedVersion, builder.analyzers.indexAnalyzers).init(this); + return new Builder(simpleName(), builder.indexCreatedVersion, builder.indexAnalyzers).init(this); } public static String getShingleFieldName(String parentField, int shingleSize) { diff --git a/server/src/main/java/org/elasticsearch/index/mapper/TextParams.java b/server/src/main/java/org/elasticsearch/index/mapper/TextParams.java index a2b14b9e73e89..71087837a840c 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/TextParams.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/TextParams.java @@ -46,52 +46,61 @@ private TextParams() {} public record AnalyzerConfiguration(String indexAnalyzer, String searchAnalyzer, String searchQuoteAnalyzer, int posIncrementGap) { - public Analyzers buildAnalyzers(IndexAnalyzers indexAnalyzers) { + public Analyzers buildAnalyzers(IndexAnalyzers indexAnalyzers, boolean isLegacyVersion) { return new Analyzers( - wrap(buildIndexAnalyzer(indexAnalyzers), AnalysisMode.INDEX_TIME), - wrap(buildSearchAnalyzer(indexAnalyzers), AnalysisMode.SEARCH_TIME), - wrap(buildSearchQuoteAnalyzer(indexAnalyzers), AnalysisMode.SEARCH_TIME), + wrap(buildIndexAnalyzer(indexAnalyzers, isLegacyVersion), AnalysisMode.INDEX_TIME), + wrap(buildSearchAnalyzer(indexAnalyzers, isLegacyVersion), AnalysisMode.SEARCH_TIME), + wrap(buildSearchQuoteAnalyzer(indexAnalyzers, isLegacyVersion), AnalysisMode.SEARCH_TIME), this ); } - private NamedAnalyzer buildIndexAnalyzer(IndexAnalyzers indexAnalyzers) { + private NamedAnalyzer buildIndexAnalyzer(IndexAnalyzers indexAnalyzers, boolean isLegacyVersion) { if (this.indexAnalyzer == null) { return indexAnalyzers.getDefaultIndexAnalyzer(); } NamedAnalyzer a = indexAnalyzers.get(this.indexAnalyzer); if (a == null) { + if (isLegacyVersion) { + return indexAnalyzers.getDefaultIndexAnalyzer(); + } throw new IllegalArgumentException("Unknown analyzer [" + this.indexAnalyzer + "]"); } return a; } - private NamedAnalyzer buildSearchAnalyzer(IndexAnalyzers indexAnalyzers) { + private NamedAnalyzer buildSearchAnalyzer(IndexAnalyzers indexAnalyzers, boolean isLegacyVersion) { if (this.searchAnalyzer == null) { if (this.indexAnalyzer == null) { return indexAnalyzers.getDefaultSearchAnalyzer(); } - return indexAnalyzers.get(this.indexAnalyzer); // null check will already have happened in buildIndexAnalyzer + return buildIndexAnalyzer(indexAnalyzers, isLegacyVersion); } NamedAnalyzer a = indexAnalyzers.get(this.searchAnalyzer); if (a == null) { + if (isLegacyVersion) { + return indexAnalyzers.getDefaultSearchAnalyzer(); + } throw new IllegalArgumentException("Unknown analyzer [" + this.searchAnalyzer + "]"); } return a; } - private NamedAnalyzer buildSearchQuoteAnalyzer(IndexAnalyzers indexAnalyzers) { + private NamedAnalyzer buildSearchQuoteAnalyzer(IndexAnalyzers indexAnalyzers, boolean isLegacyVersion) { if (this.searchQuoteAnalyzer == null) { if (this.searchAnalyzer == null) { if (this.indexAnalyzer == null) { return indexAnalyzers.getDefaultSearchQuoteAnalyzer(); } - return indexAnalyzers.get(this.indexAnalyzer); // null checked already in buildIndexAnalyzer + return buildIndexAnalyzer(indexAnalyzers, isLegacyVersion); } - return indexAnalyzers.get(this.searchAnalyzer); // null checked already in buildSearchAnalyzer + return buildSearchAnalyzer(indexAnalyzers, isLegacyVersion); } NamedAnalyzer a = indexAnalyzers.get(this.searchQuoteAnalyzer); if (a == null) { + if (isLegacyVersion) { + return indexAnalyzers.getDefaultSearchQuoteAnalyzer(); + } throw new IllegalArgumentException("Unknown analyzer [" + this.searchQuoteAnalyzer + "]"); } return a; @@ -120,34 +129,45 @@ public static final class AnalyzerParameters { public final Parameter searchQuoteAnalyzer; public final Parameter positionIncrementGap; + private final Version indexCreatedVersion; + public AnalyzerParameters( Function analyzerInitFunction, Version indexCreatedVersion ) { - + this.indexCreatedVersion = indexCreatedVersion; this.indexAnalyzer = Parameter.stringParam( "analyzer", - indexCreatedVersion.isLegacyIndexVersion(), + false, // sort of - see merge validator! mapper -> analyzerInitFunction.apply(mapper).indexAnalyzer, null ) - .setSerializerCheck((includeDefaults, isConfigured, value) -> value != null) + .acceptsNull() + .setSerializerCheck((includeDefaults, isConfigured, value) -> includeDefaults || value != null) .setMergeValidator( - // special case - we allow 'default' to be merged in to an unconfigured analyzer - (previous, toMerge, conflicts) -> Objects.equals(previous, toMerge) || (previous == null && "default".equals(toMerge)) + // special cases + // - we allow 'default' to be merged in to an unconfigured analyzer + // - is the index was created a long time ago we allow updates to remove warnings about vanished analyzers + (previous, toMerge, conflicts) -> indexCreatedVersion.isLegacyIndexVersion() + || Objects.equals(previous, toMerge) + || (previous == null && "default".equals(toMerge)) ); this.searchAnalyzer = Parameter.stringParam( "search_analyzer", true, mapper -> analyzerInitFunction.apply(mapper).searchAnalyzer, null - ).setSerializerCheck((includeDefaults, isConfigured, value) -> value != null); + ) + .acceptsNull() + .setSerializerCheck((includeDefaults, isConfigured, value) -> includeDefaults || value != null); this.searchQuoteAnalyzer = Parameter.stringParam( "search_quote_analyzer", true, mapper -> analyzerInitFunction.apply(mapper).searchQuoteAnalyzer, null - ).setSerializerCheck((includeDefaults, isConfigured, value) -> value != null); + ) + .acceptsNull() + .setSerializerCheck((includeDefaults, isConfigured, value) -> includeDefaults || value != null); this.positionIncrementGap = Parameter.intParam( "position_increment_gap", false, @@ -167,7 +187,7 @@ public Analyzers buildAnalyzers(IndexAnalyzers indexAnalyzers) { searchQuoteAnalyzer.get(), positionIncrementGap.get() ); - return config.buildAnalyzers(indexAnalyzers); + return config.buildAnalyzers(indexAnalyzers, indexCreatedVersion.isLegacyIndexVersion()); } } diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateServiceTests.java index d05c2420cefb1..f19ad0491eab8 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateServiceTests.java @@ -212,7 +212,7 @@ public void testIndexTemplateWithValidateMapping() throws Exception { List errors = putTemplateDetail(request); assertThat(errors.size(), equalTo(1)); assertThat(errors.get(0), instanceOf(MapperParsingException.class)); - assertThat(errors.get(0).getMessage(), containsString("analyzer [custom_1] has not been configured in mappings")); + assertThat(errors.get(0).getMessage(), containsString("Unknown analyzer [custom_1]")); } public void testAliasInvalidFilterInvalidJson() throws Exception { diff --git a/server/src/test/java/org/elasticsearch/index/mapper/TextFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/TextFieldMapperTests.java index 9c2cf06ba7497..052664b4427f8 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/TextFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/TextFieldMapperTests.java @@ -569,12 +569,10 @@ public void testFrequencyFilter() throws IOException { assertThat(fieldType.fielddataMinSegmentSize(), equalTo(1000)); } - public void testNullConfigValuesFail() throws MapperParsingException { - Exception e = expectThrows( - MapperParsingException.class, - () -> createDocumentMapper(fieldMapping(b -> b.field("type", "text").field("analyzer", (String) null))) - ); - assertThat(e.getMessage(), containsString("[analyzer] on mapper [field] of type [text] must not have a [null] value")); + public void testNullConfigValuesUseDefaults() throws IOException { + MapperService mapperService = createMapperService(fieldMapping(b -> b.field("type", "text").field("analyzer", (String) null))); + TextFieldMapper m = (TextFieldMapper) mapperService.mappingLookup().getMapper("field"); + assertEquals("default", m.indexAnalyzers().get("field").name()); } public void testNotIndexedFieldPositionIncrement() { From eaa1f62ef567a324e93714e1c9b420f1da281698 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Mon, 3 Apr 2023 12:46:55 +0100 Subject: [PATCH 3/4] tests --- .../index/mapper/TextFieldMapper.java | 19 +- .../index/mapper/TextParams.java | 26 ++- .../index/mapper/DynamicTemplatesTests.java | 2 +- .../mapper/TextFieldAnalyzerModeTests.java | 171 +++++------------- .../index/mapper/TextFieldMapperTests.java | 10 +- .../query/SearchExecutionContextTests.java | 5 +- 6 files changed, 80 insertions(+), 153 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/TextFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/TextFieldMapper.java index 70ed682b22117..b6763e381871c 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/TextFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/TextFieldMapper.java @@ -285,10 +285,7 @@ public Builder(String name, Version indexCreatedVersion, IndexAnalyzers indexAna super(name); this.indexCreatedVersion = indexCreatedVersion; this.indexAnalyzers = indexAnalyzers; - this.analyzers = new TextParams.AnalyzerParameters( - m -> ((TextFieldMapper) m).analyzerConfiguration, - indexCreatedVersion - ); + this.analyzers = new TextParams.AnalyzerParameters(m -> ((TextFieldMapper) m).analyzerConfiguration, indexCreatedVersion); } public Builder index(boolean index) { @@ -384,7 +381,12 @@ private KeywordFieldMapper.KeywordFieldType syntheticSourceDelegate(FieldType fi return null; } - private SubFieldInfo buildPrefixInfo(MapperBuilderContext context, TextParams.Analyzers analyzers, FieldType fieldType, TextFieldType tft) { + private SubFieldInfo buildPrefixInfo( + MapperBuilderContext context, + TextParams.Analyzers analyzers, + FieldType fieldType, + TextFieldType tft + ) { if (indexPrefixes.get() == null) { return null; } @@ -444,7 +446,12 @@ private SubFieldInfo buildPhraseInfo(TextParams.Analyzers analyzers, FieldType f return new SubFieldInfo(parent.name() + FAST_PHRASE_SUFFIX, phraseFieldType, a); } - private Map indexAnalyzers(String name, NamedAnalyzer main, SubFieldInfo phraseFieldInfo, SubFieldInfo prefixFieldInfo) { + private Map indexAnalyzers( + String name, + NamedAnalyzer main, + SubFieldInfo phraseFieldInfo, + SubFieldInfo prefixFieldInfo + ) { Map analyzers = new HashMap<>(); analyzers.put(name, main); if (phraseFieldInfo != null) { diff --git a/server/src/main/java/org/elasticsearch/index/mapper/TextParams.java b/server/src/main/java/org/elasticsearch/index/mapper/TextParams.java index 71087837a840c..7f1dd85a9d555 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/TextParams.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/TextParams.java @@ -131,18 +131,16 @@ public static final class AnalyzerParameters { private final Version indexCreatedVersion; - public AnalyzerParameters( - Function analyzerInitFunction, - Version indexCreatedVersion - ) { + public AnalyzerParameters(Function analyzerInitFunction, Version indexCreatedVersion) { this.indexCreatedVersion = indexCreatedVersion; this.indexAnalyzer = Parameter.stringParam( "analyzer", false, // sort of - see merge validator! mapper -> analyzerInitFunction.apply(mapper).indexAnalyzer, - null + null, + // serializer check means that if value is null then we're being asked for defaults + (builder, name, value) -> builder.field(name, value == null ? "default" : value) ) - .acceptsNull() .setSerializerCheck((includeDefaults, isConfigured, value) -> includeDefaults || value != null) .setMergeValidator( // special cases @@ -156,18 +154,18 @@ public AnalyzerParameters( "search_analyzer", true, mapper -> analyzerInitFunction.apply(mapper).searchAnalyzer, - null - ) - .acceptsNull() - .setSerializerCheck((includeDefaults, isConfigured, value) -> includeDefaults || value != null); + null, + // serializer check means that if value is null then we're being asked for defaults + (builder, name, value) -> builder.field(name, value == null ? "default" : value) + ).setSerializerCheck((includeDefaults, isConfigured, value) -> includeDefaults || value != null); this.searchQuoteAnalyzer = Parameter.stringParam( "search_quote_analyzer", true, mapper -> analyzerInitFunction.apply(mapper).searchQuoteAnalyzer, - null - ) - .acceptsNull() - .setSerializerCheck((includeDefaults, isConfigured, value) -> includeDefaults || value != null); + null, + // serializer check means that if value is null then we're being asked for defaults + (builder, name, value) -> builder.field(name, value == null ? "default" : value) + ).setSerializerCheck((includeDefaults, isConfigured, value) -> includeDefaults || value != null); this.positionIncrementGap = Parameter.intParam( "position_increment_gap", false, diff --git a/server/src/test/java/org/elasticsearch/index/mapper/DynamicTemplatesTests.java b/server/src/test/java/org/elasticsearch/index/mapper/DynamicTemplatesTests.java index c125fc4e8e664..b1c567d30af2b 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/DynamicTemplatesTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/DynamicTemplatesTests.java @@ -531,7 +531,7 @@ public void testIllegalDynamicTemplateInvalidAttribute() throws Exception { mapping.endObject(); MapperParsingException e = expectThrows(MapperParsingException.class, () -> createMapperService(mapping)); assertThat(e.getRootCause(), instanceOf(IllegalArgumentException.class)); - assertThat(e.getRootCause().getMessage(), equalTo("analyzer [foobar] has not been configured in mappings")); + assertThat(e.getRootCause().getMessage(), equalTo("Unknown analyzer [foobar]")); } public void testIllegalDynamicTemplateNoMappingType() throws Exception { diff --git a/server/src/test/java/org/elasticsearch/index/mapper/TextFieldAnalyzerModeTests.java b/server/src/test/java/org/elasticsearch/index/mapper/TextFieldAnalyzerModeTests.java index 9d86edd73adac..6a4f7f466c375 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/TextFieldAnalyzerModeTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/TextFieldAnalyzerModeTests.java @@ -8,12 +8,9 @@ package org.elasticsearch.index.mapper; -import org.apache.lucene.analysis.Analyzer; import org.apache.lucene.analysis.TokenStream; -import org.apache.lucene.analysis.standard.StandardAnalyzer; -import org.elasticsearch.Version; -import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.index.IndexSettings; import org.elasticsearch.index.analysis.AbstractTokenFilterFactory; import org.elasticsearch.index.analysis.AnalysisMode; import org.elasticsearch.index.analysis.AnalyzerScope; @@ -22,31 +19,30 @@ import org.elasticsearch.index.analysis.IndexAnalyzers; import org.elasticsearch.index.analysis.NamedAnalyzer; import org.elasticsearch.index.analysis.TokenFilterFactory; -import org.elasticsearch.test.ESTestCase; -import java.util.HashMap; +import java.io.IOException; import java.util.Map; -import static org.elasticsearch.index.analysis.AnalysisRegistry.DEFAULT_ANALYZER_NAME; import static org.hamcrest.Matchers.containsString; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; - -public class TextFieldAnalyzerModeTests extends ESTestCase { - - private static Map defaultAnalyzers() { - Map analyzers = new HashMap<>(); - analyzers.put(DEFAULT_ANALYZER_NAME, new NamedAnalyzer("default", AnalyzerScope.INDEX, null)); - return analyzers; +import static org.hamcrest.Matchers.equalTo; + +public class TextFieldAnalyzerModeTests extends MapperServiceTestCase { + + @Override + protected IndexAnalyzers createIndexAnalyzers(IndexSettings indexSettings) { + return IndexAnalyzers.of( + Map.of( + "all", + createAnalyzerWithMode("all", AnalysisMode.ALL), + "index", + createAnalyzerWithMode("index", AnalysisMode.INDEX_TIME), + "search", + createAnalyzerWithMode("search", AnalysisMode.SEARCH_TIME) + ) + ); } - private static final IndexMetadata EMPTY_INDEX_METADATA = IndexMetadata.builder("") - .settings(Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT)) - .numberOfShards(1) - .numberOfReplicas(0) - .build(); - - private Analyzer createAnalyzerWithMode(AnalysisMode mode) { + private static NamedAnalyzer createAnalyzerWithMode(String name, AnalysisMode mode) { TokenFilterFactory tokenFilter = new AbstractTokenFilterFactory("my_analyzer", Settings.EMPTY) { @Override public AnalysisMode getAnalysisMode() { @@ -58,118 +54,39 @@ public TokenStream create(TokenStream tokenStream) { return null; } }; - return new CustomAnalyzer(null, new CharFilterFactory[0], new TokenFilterFactory[] { tokenFilter }); - } - - public void testParseTextFieldCheckAnalyzerAnalysisMode() { - - Map fieldNode = new HashMap<>(); - fieldNode.put("analyzer", "my_analyzer"); - MappingParserContext parserContext = mock(MappingParserContext.class); - when(parserContext.indexVersionCreated()).thenReturn(Version.CURRENT); - - // check AnalysisMode.ALL works - Map analyzers = defaultAnalyzers(); - analyzers.put("my_analyzer", new NamedAnalyzer("my_named_analyzer", AnalyzerScope.INDEX, createAnalyzerWithMode(AnalysisMode.ALL))); - - IndexAnalyzers indexAnalyzers = IndexAnalyzers.of(analyzers); - when(parserContext.getIndexAnalyzers()).thenReturn(indexAnalyzers); - - TextFieldMapper.PARSER.parse("field", fieldNode, parserContext); - - // check that "analyzer" set to something that only supports AnalysisMode.SEARCH_TIME or AnalysisMode.INDEX_TIME is blocked - AnalysisMode mode = randomFrom(AnalysisMode.SEARCH_TIME, AnalysisMode.INDEX_TIME); - analyzers = defaultAnalyzers(); - analyzers.put("my_analyzer", new NamedAnalyzer("my_named_analyzer", AnalyzerScope.INDEX, createAnalyzerWithMode(mode))); - indexAnalyzers = IndexAnalyzers.of(analyzers); - when(parserContext.getIndexAnalyzers()).thenReturn(indexAnalyzers); - fieldNode.put("analyzer", "my_analyzer"); - MapperException ex = expectThrows(MapperException.class, () -> { TextFieldMapper.PARSER.parse("name", fieldNode, parserContext); }); - assertThat( - ex.getMessage(), - containsString("analyzer [my_named_analyzer] contains filters [my_analyzer] that are not allowed to run") + return new NamedAnalyzer( + name, + AnalyzerScope.INDEX, + new CustomAnalyzer(null, new CharFilterFactory[0], new TokenFilterFactory[] { tokenFilter }) ); } - public void testParseTextFieldCheckSearchAnalyzerAnalysisMode() { - - for (String settingToTest : new String[] { "search_analyzer", "search_quote_analyzer" }) { - Map fieldNode = new HashMap<>(); - fieldNode.put(settingToTest, "my_analyzer"); - fieldNode.put("analyzer", "standard"); - if (settingToTest.equals("search_quote_analyzer")) { - fieldNode.put("search_analyzer", "standard"); - } - MappingParserContext parserContext = mock(MappingParserContext.class); - when(parserContext.indexVersionCreated()).thenReturn(Version.CURRENT); - - // check AnalysisMode.ALL and AnalysisMode.SEARCH_TIME works - Map analyzers = defaultAnalyzers(); - AnalysisMode mode = randomFrom(AnalysisMode.ALL, AnalysisMode.SEARCH_TIME); - analyzers.put("my_analyzer", new NamedAnalyzer("my_named_analyzer", AnalyzerScope.INDEX, createAnalyzerWithMode(mode))); - analyzers.put("standard", new NamedAnalyzer("standard", AnalyzerScope.INDEX, new StandardAnalyzer())); - - IndexAnalyzers indexAnalyzers = IndexAnalyzers.of(analyzers); - when(parserContext.getIndexAnalyzers()).thenReturn(indexAnalyzers); - TextFieldMapper.PARSER.parse("textField", fieldNode, parserContext); + public void testParseTextFieldCheckAnalyzerAnalysisMode() throws IOException { - // check that "analyzer" set to AnalysisMode.INDEX_TIME is blocked - mode = AnalysisMode.INDEX_TIME; - analyzers = defaultAnalyzers(); - analyzers.put("my_analyzer", new NamedAnalyzer("my_named_analyzer", AnalyzerScope.INDEX, createAnalyzerWithMode(mode))); - analyzers.put("standard", new NamedAnalyzer("standard", AnalyzerScope.INDEX, new StandardAnalyzer())); - indexAnalyzers = IndexAnalyzers.of(analyzers); - when(parserContext.getIndexAnalyzers()).thenReturn(indexAnalyzers); - fieldNode.clear(); - fieldNode.put(settingToTest, "my_analyzer"); - fieldNode.put("analyzer", "standard"); - if (settingToTest.equals("search_quote_analyzer")) { - fieldNode.put("search_analyzer", "standard"); - } - MapperException ex = expectThrows( - MapperException.class, - () -> { TextFieldMapper.PARSER.parse("field", fieldNode, parserContext); } - ); - assertEquals( - "analyzer [my_named_analyzer] contains filters [my_analyzer] that are not allowed to run in search time mode.", - ex.getMessage() - ); - } - } - - public void testParseTextFieldCheckAnalyzerWithSearchAnalyzerAnalysisMode() { + MapperService mapperService = createMapperService(""" + { "_doc" : { "properties" : { + "text1" : { "type" : "text", "analyzer" : "all" }, + "text2" : { "type" : "text", "analyzer" : "index", "search_analyzer" : "search" } + }}} + """); - Map fieldNode = new HashMap<>(); - fieldNode.put("analyzer", "my_analyzer"); - MappingParserContext parserContext = mock(MappingParserContext.class); - when(parserContext.indexVersionCreated()).thenReturn(Version.CURRENT); + assertThat(mapperService.mappingLookup().indexAnalyzer("text1", field -> null).name(), equalTo("all")); + assertThat(mapperService.mappingLookup().indexAnalyzer("text2", field -> null).name(), equalTo("index")); - // check that "analyzer" set to AnalysisMode.INDEX_TIME is blocked if there is no search analyzer - AnalysisMode mode = AnalysisMode.INDEX_TIME; - Map analyzers = defaultAnalyzers(); - analyzers.put("my_analyzer", new NamedAnalyzer("my_named_analyzer", AnalyzerScope.INDEX, createAnalyzerWithMode(mode))); - IndexAnalyzers indexAnalyzers = IndexAnalyzers.of(analyzers); - when(parserContext.getIndexAnalyzers()).thenReturn(indexAnalyzers); - MapperException ex = expectThrows( - MapperException.class, - () -> { TextFieldMapper.PARSER.parse("field", fieldNode, parserContext); } - ); - assertThat( - ex.getMessage(), - containsString("analyzer [my_named_analyzer] contains filters [my_analyzer] that are not allowed to run") - ); + Exception e = expectThrows(MapperException.class, () -> createMapperService(""" + { "_doc" : { "properties" : { "text" : { "type" : "text", "analyzer" : "search" } } } } + """)); + assertThat(e.getMessage(), containsString("analyzer [search] contains filters [my_analyzer] that are not allowed to run")); - // check AnalysisMode.INDEX_TIME is okay if search analyzer is also set - fieldNode.put("analyzer", "my_analyzer"); - fieldNode.put("search_analyzer", "standard"); - analyzers = defaultAnalyzers(); - mode = randomFrom(AnalysisMode.ALL, AnalysisMode.INDEX_TIME); - analyzers.put("my_analyzer", new NamedAnalyzer("my_named_analyzer", AnalyzerScope.INDEX, createAnalyzerWithMode(mode))); - analyzers.put("standard", new NamedAnalyzer("standard", AnalyzerScope.INDEX, new StandardAnalyzer())); + Exception e2 = expectThrows(MapperException.class, () -> createMapperService(""" + { "_doc" : { "properties" : { "text" : { "type" : "text", "analyzer" : "index" } } } } + """)); + assertThat(e2.getMessage(), containsString("analyzer [index] contains filters [my_analyzer] that are not allowed to run")); - indexAnalyzers = IndexAnalyzers.of(analyzers); - when(parserContext.getIndexAnalyzers()).thenReturn(indexAnalyzers); - TextFieldMapper.PARSER.parse("field", fieldNode, parserContext); + Exception e3 = expectThrows(MapperException.class, () -> createMapperService(""" + { "_doc" : { "properties" : { "text" : { "type" : "text", "analyzer" : "index", "search_quote_analyzer" : "search" } } } } + """)); + assertThat(e.getMessage(), containsString("analyzer [search] contains filters [my_analyzer] that are not allowed to run")); } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/TextFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/TextFieldMapperTests.java index 052664b4427f8..9c2cf06ba7497 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/TextFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/TextFieldMapperTests.java @@ -569,10 +569,12 @@ public void testFrequencyFilter() throws IOException { assertThat(fieldType.fielddataMinSegmentSize(), equalTo(1000)); } - public void testNullConfigValuesUseDefaults() throws IOException { - MapperService mapperService = createMapperService(fieldMapping(b -> b.field("type", "text").field("analyzer", (String) null))); - TextFieldMapper m = (TextFieldMapper) mapperService.mappingLookup().getMapper("field"); - assertEquals("default", m.indexAnalyzers().get("field").name()); + public void testNullConfigValuesFail() throws MapperParsingException { + Exception e = expectThrows( + MapperParsingException.class, + () -> createDocumentMapper(fieldMapping(b -> b.field("type", "text").field("analyzer", (String) null))) + ); + assertThat(e.getMessage(), containsString("[analyzer] on mapper [field] of type [text] must not have a [null] value")); } public void testNotIndexedFieldPositionIncrement() { diff --git a/server/src/test/java/org/elasticsearch/index/query/SearchExecutionContextTests.java b/server/src/test/java/org/elasticsearch/index/query/SearchExecutionContextTests.java index 5eafdc0b6f0f7..d174626c2a09e 100644 --- a/server/src/test/java/org/elasticsearch/index/query/SearchExecutionContextTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/SearchExecutionContextTests.java @@ -7,6 +7,7 @@ */ package org.elasticsearch.index.query; +import org.apache.lucene.analysis.standard.StandardAnalyzer; import org.apache.lucene.document.Field; import org.apache.lucene.document.StringField; import org.apache.lucene.index.DirectoryReader; @@ -475,7 +476,9 @@ private static SearchExecutionContext createSearchExecutionContext( } private static MapperService createMapperService(IndexSettings indexSettings, MappingLookup mappingLookup) { - IndexAnalyzers indexAnalyzers = IndexAnalyzers.of(singletonMap("default", new NamedAnalyzer("default", AnalyzerScope.INDEX, null))); + IndexAnalyzers indexAnalyzers = IndexAnalyzers.of( + singletonMap("default", new NamedAnalyzer("default", AnalyzerScope.INDEX, new StandardAnalyzer())) + ); IndicesModule indicesModule = new IndicesModule(Collections.emptyList()); MapperRegistry mapperRegistry = indicesModule.getMapperRegistry(); Supplier searchExecutionContextSupplier = () -> { throw new UnsupportedOperationException(); }; From 319c4463ace0bf52ecf505f23a872ee8b721db4b Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Mon, 3 Apr 2023 14:10:50 +0100 Subject: [PATCH 4/4] annotated text --- .../AnnotatedTextFieldMapper.java | 30 +++++++++++-------- 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/plugins/mapper-annotated-text/src/main/java/org/elasticsearch/index/mapper/annotatedtext/AnnotatedTextFieldMapper.java b/plugins/mapper-annotated-text/src/main/java/org/elasticsearch/index/mapper/annotatedtext/AnnotatedTextFieldMapper.java index bb9879b124f79..d3dbe0e98d8be 100644 --- a/plugins/mapper-annotated-text/src/main/java/org/elasticsearch/index/mapper/annotatedtext/AnnotatedTextFieldMapper.java +++ b/plugins/mapper-annotated-text/src/main/java/org/elasticsearch/index/mapper/annotatedtext/AnnotatedTextFieldMapper.java @@ -77,7 +77,9 @@ public static class Builder extends FieldMapper.Builder { private final Parameter store = Parameter.storeParam(m -> builder(m).store.getValue(), false); - final TextParams.Analyzers analyzers; + final TextParams.AnalyzerParameters analyzers; + final IndexAnalyzers indexAnalyzers; + final Parameter similarity = TextParams.similarity(m -> builder(m).similarity.getValue()); final Parameter indexOptions = TextParams.textIndexOptions(m -> builder(m).indexOptions.getValue()); @@ -91,10 +93,9 @@ public static class Builder extends FieldMapper.Builder { public Builder(String name, Version indexCreatedVersion, IndexAnalyzers indexAnalyzers) { super(name); this.indexCreatedVersion = indexCreatedVersion; - this.analyzers = new TextParams.Analyzers( - indexAnalyzers, - m -> builder(m).analyzers.getIndexAnalyzer(), - m -> builder(m).analyzers.positionIncrementGap.getValue(), + this.indexAnalyzers = indexAnalyzers; + this.analyzers = new TextParams.AnalyzerParameters( + m -> ((AnnotatedTextFieldMapper) m).analyzerConfiguration, indexCreatedVersion ); } @@ -114,12 +115,12 @@ protected Parameter[] getParameters() { meta }; } - private AnnotatedTextFieldType buildFieldType(FieldType fieldType, MapperBuilderContext context) { + private AnnotatedTextFieldType buildFieldType(FieldType fieldType, MapperBuilderContext context, TextParams.Analyzers analyzers) { TextSearchInfo tsi = new TextSearchInfo( fieldType, similarity.get(), - wrapAnalyzer(analyzers.getSearchAnalyzer()), - wrapAnalyzer(analyzers.getSearchQuoteAnalyzer()) + wrapAnalyzer(analyzers.searchAnalyzer()), + wrapAnalyzer(analyzers.searchQuoteAnalyzer()) ); return new AnnotatedTextFieldType( context.buildFullName(name), @@ -132,11 +133,12 @@ private AnnotatedTextFieldType buildFieldType(FieldType fieldType, MapperBuilder @Override public AnnotatedTextFieldMapper build(MapperBuilderContext context) { + TextParams.Analyzers analyzers = this.analyzers.buildAnalyzers(indexAnalyzers); FieldType fieldType = TextParams.buildFieldType(() -> true, store, indexOptions, norms, termVectors); if (fieldType.indexOptions() == IndexOptions.NONE) { throw new IllegalArgumentException("[" + CONTENT_TYPE + "] fields must be indexed"); } - if (analyzers.positionIncrementGap.isConfigured()) { + if (this.analyzers.positionIncrementGap.isConfigured()) { if (fieldType.indexOptions().compareTo(IndexOptions.DOCS_AND_FREQS_AND_POSITIONS) < 0) { throw new IllegalArgumentException( "Cannot set position_increment_gap on field [" + name + "] without positions enabled" @@ -146,9 +148,10 @@ public AnnotatedTextFieldMapper build(MapperBuilderContext context) { return new AnnotatedTextFieldMapper( name, fieldType, - buildFieldType(fieldType, context), + buildFieldType(fieldType, context, analyzers), multiFieldsBuilder.build(this, context), copyTo.build(), + analyzers, this ); } @@ -494,6 +497,7 @@ public String typeName() { } private final FieldType fieldType; + private final TextParams.AnalyzerConfiguration analyzerConfiguration; private final Builder builder; private final NamedAnalyzer indexAnalyzer; @@ -504,13 +508,15 @@ protected AnnotatedTextFieldMapper( AnnotatedTextFieldType mappedFieldType, MultiFields multiFields, CopyTo copyTo, + TextParams.Analyzers analyzers, Builder builder ) { super(simpleName, mappedFieldType, multiFields, copyTo); assert fieldType.tokenized(); this.fieldType = fieldType; this.builder = builder; - this.indexAnalyzer = wrapAnalyzer(builder.analyzers.getIndexAnalyzer()); + this.indexAnalyzer = wrapAnalyzer(analyzers.indexAnalyzer()); + this.analyzerConfiguration = analyzers.configuration(); } @Override @@ -542,6 +548,6 @@ protected String contentType() { @Override public FieldMapper.Builder getMergeBuilder() { - return new Builder(simpleName(), builder.indexCreatedVersion, builder.analyzers.indexAnalyzers).init(this); + return new Builder(simpleName(), builder.indexCreatedVersion, builder.indexAnalyzers).init(this); } }