Skip to content

Commit d251a48

Browse files
romseygeekromseygeek
committed
Move MappedFieldType.similarity() to TextSearchInfo (#58439)
Similarities only apply to a few text-based field types, but are currently set directly on the base MappedFieldType class. This commit moves similarity information into TextSearchInfo, and removes any mentions of it from MappedFieldType or FieldMapper. It was previously possible to include a similarity parameter on a number of field types that would then ignore this information. To make it obvious that this has no effect, setting this parameter on non-text field types now issues a deprecation warning.
1 parent fcd8a43 commit d251a48

File tree

18 files changed

+166
-101
lines changed

18 files changed

+166
-101
lines changed

‎modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/SearchAsYouTypeFieldMapper.java

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@
5252
import org.elasticsearch.index.analysis.AnalyzerScope;
5353
import org.elasticsearch.index.analysis.NamedAnalyzer;
5454
import org.elasticsearch.index.query.QueryShardContext;
55+
import org.elasticsearch.index.similarity.SimilarityProvider;
56+
import org.elasticsearch.index.similarity.SimilarityService;
5557

5658
import java.io.IOException;
5759
import java.util.ArrayList;
@@ -64,6 +66,7 @@
6466

6567
import static org.elasticsearch.common.xcontent.support.XContentMapValues.nodeIntegerValue;
6668
import static org.elasticsearch.index.mapper.TextFieldMapper.TextFieldType.hasGaps;
69+
import static org.elasticsearch.index.mapper.TypeParsers.checkNull;
6770
import static org.elasticsearch.index.mapper.TypeParsers.parseTextField;
6871

6972
/**
@@ -113,30 +116,39 @@ public Mapper.Builder<?> parse(String name,
113116
builder.indexAnalyzer(parserContext.getIndexAnalyzers().getDefaultIndexAnalyzer());
114117
builder.searchAnalyzer(parserContext.getIndexAnalyzers().getDefaultSearchAnalyzer());
115118
builder.searchQuoteAnalyzer(parserContext.getIndexAnalyzers().getDefaultSearchQuoteAnalyzer());
116-
parseTextField(builder, name, node, parserContext);
117119
for (Iterator<Map.Entry<String, Object>> iterator = node.entrySet().iterator(); iterator.hasNext();) {
118120
final Map.Entry<String, Object> entry = iterator.next();
119121
final String fieldName = entry.getKey();
120122
final Object fieldNode = entry.getValue();
121-
123+
checkNull(fieldName, fieldNode);
122124
if (fieldName.equals("max_shingle_size")) {
123125
builder.maxShingleSize(nodeIntegerValue(fieldNode));
124126
iterator.remove();
127+
} else if (fieldName.equals("similarity")) {
128+
SimilarityProvider similarityProvider = TypeParsers.resolveSimilarity(parserContext, fieldName, fieldNode.toString());
129+
builder.similarity(similarityProvider);
130+
iterator.remove();
125131
}
126132
// TODO should we allow to configure the prefix field
127133
}
134+
parseTextField(builder, name, node, parserContext);
128135
return builder;
129136
}
130137
}
131138

132139
public static class Builder extends FieldMapper.Builder<Builder> {
133140
private int maxShingleSize = Defaults.MAX_SHINGLE_SIZE;
141+
private SimilarityProvider similarity;
134142

135143
public Builder(String name) {
136144
super(name, Defaults.FIELD_TYPE);
137145
this.builder = this;
138146
}
139147

148+
public void similarity(SimilarityProvider similarity) {
149+
this.similarity = similarity;
150+
}
151+
140152
public Builder maxShingleSize(int maxShingleSize) {
141153
if (maxShingleSize < MAX_SHINGLE_SIZE_LOWER_BOUND || maxShingleSize > MAX_SHINGLE_SIZE_UPPER_BOUND) {
142154
throw new MapperParsingException("[max_shingle_size] must be at least [" + MAX_SHINGLE_SIZE_LOWER_BOUND + "] and at most " +
@@ -157,11 +169,10 @@ public Builder docValues(boolean docValues) {
157169
@Override
158170
public SearchAsYouTypeFieldMapper build(Mapper.BuilderContext context) {
159171

160-
SearchAsYouTypeFieldType ft = new SearchAsYouTypeFieldType(buildFullName(context), fieldType, meta);
172+
SearchAsYouTypeFieldType ft = new SearchAsYouTypeFieldType(buildFullName(context), fieldType, similarity, meta);
161173
ft.setIndexAnalyzer(indexAnalyzer);
162174
ft.setSearchAnalyzer(searchAnalyzer);
163175
ft.setSearchQuoteAnalyzer(searchQuoteAnalyzer);
164-
ft.setSimilarity(similarity);
165176

166177
// set up the prefix field
167178
FieldType prefixft = new FieldType(fieldType);
@@ -235,8 +246,8 @@ static class SearchAsYouTypeFieldType extends StringFieldType {
235246
PrefixFieldType prefixField;
236247
ShingleFieldType[] shingleFields = new ShingleFieldType[0];
237248

238-
SearchAsYouTypeFieldType(String name, FieldType fieldType, Map<String, String> meta) {
239-
super(name, fieldType.indexOptions() != IndexOptions.NONE, false, new TextSearchInfo(fieldType), meta);
249+
SearchAsYouTypeFieldType(String name, FieldType fieldType, SimilarityProvider similarity, Map<String, String> meta) {
250+
super(name, fieldType.indexOptions() != IndexOptions.NONE, false, new TextSearchInfo(fieldType, similarity), meta);
240251
}
241252

242253
SearchAsYouTypeFieldType(SearchAsYouTypeFieldType other) {
@@ -382,7 +393,7 @@ static final class PrefixFieldType extends StringFieldType {
382393
final String parentField;
383394

384395
PrefixFieldType(String parentField, FieldType fieldType, int minChars, int maxChars) {
385-
super(parentField + PREFIX_FIELD_SUFFIX, true, false, new TextSearchInfo(fieldType), Collections.emptyMap());
396+
super(parentField + PREFIX_FIELD_SUFFIX, true, false, new TextSearchInfo(fieldType, null), Collections.emptyMap());
386397
this.minChars = minChars;
387398
this.maxChars = maxChars;
388399
this.parentField = parentField;
@@ -535,7 +546,7 @@ static class ShingleFieldType extends StringFieldType {
535546
PrefixFieldType prefixFieldType;
536547

537548
ShingleFieldType(String name, int shingleSize, FieldType fieldType) {
538-
super(name, true, false, new TextSearchInfo(fieldType), Collections.emptyMap());
549+
super(name, true, false, new TextSearchInfo(fieldType, null), Collections.emptyMap());
539550
this.shingleSize = shingleSize;
540551
}
541552

@@ -689,7 +700,8 @@ protected void mergeOptions(FieldMapper other, List<String> conflicts) {
689700
this.shingleFields[i] = (ShingleFieldMapper) this.shingleFields[i].merge(m.shingleFields[i]);
690701
}
691702
}
692-
if (Objects.equals(this.fieldType().similarity(), other.fieldType().similarity()) == false) {
703+
if (Objects.equals(this.fieldType().getTextSearchInfo().getSimilarity(),
704+
other.fieldType().getTextSearchInfo().getSimilarity()) == false) {
693705
conflicts.add("mapper [" + name() + "] has different [similarity] settings");
694706
}
695707
}
@@ -719,6 +731,11 @@ public ShingleFieldMapper[] shingleFields() {
719731
protected void doXContentBody(XContentBuilder builder, boolean includeDefaults, Params params) throws IOException {
720732
super.doXContentBody(builder, includeDefaults, params);
721733
doXContentAnalyzers(builder, includeDefaults);
734+
if (fieldType().getTextSearchInfo().getSimilarity() != null) {
735+
builder.field("similarity", fieldType().getTextSearchInfo().getSimilarity().name());
736+
} else if (includeDefaults) {
737+
builder.field("similarity", SimilarityService.DEFAULT_SIMILARITY);
738+
}
722739
builder.field("max_shingle_size", maxShingleSize);
723740
}
724741

‎modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/SearchAsYouTypeFieldMapperTests.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@
3232
import org.apache.lucene.search.Query;
3333
import org.apache.lucene.search.SynonymQuery;
3434
import org.apache.lucene.search.TermQuery;
35+
import org.apache.lucene.search.similarities.BM25Similarity;
36+
import org.apache.lucene.search.similarities.BooleanSimilarity;
3537
import org.apache.lucene.search.spans.FieldMaskingSpanQuery;
3638
import org.apache.lucene.search.spans.SpanNearQuery;
3739
import org.apache.lucene.search.spans.SpanTermQuery;
@@ -56,6 +58,7 @@
5658
import org.elasticsearch.index.query.MatchPhraseQueryBuilder;
5759
import org.elasticsearch.index.query.MultiMatchQueryBuilder;
5860
import org.elasticsearch.index.query.QueryShardContext;
61+
import org.elasticsearch.index.similarity.SimilarityProvider;
5962
import org.elasticsearch.plugins.Plugin;
6063
import org.hamcrest.Matcher;
6164
import org.hamcrest.Matchers;
@@ -88,6 +91,10 @@ public void addModifiers() {
8891
a.maxShingleSize(3);
8992
b.maxShingleSize(2);
9093
});
94+
addModifier("similarity", false, (a, b) -> {
95+
a.similarity(new SimilarityProvider("BM25", new BM25Similarity()));
96+
b.similarity(new SimilarityProvider("boolean", new BooleanSimilarity()));
97+
});
9198
}
9299

93100
@Override

‎modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/SearchAsYouTypeFieldTypeTests.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ public class SearchAsYouTypeFieldTypeTests extends FieldTypeTestCase<MappedField
5151

5252
@Override
5353
protected SearchAsYouTypeFieldType createDefaultFieldType(String name, Map<String, String> meta) {
54-
final SearchAsYouTypeFieldType fieldType = new SearchAsYouTypeFieldType(name, Defaults.FIELD_TYPE, meta);
54+
final SearchAsYouTypeFieldType fieldType = new SearchAsYouTypeFieldType(name, Defaults.FIELD_TYPE, null, meta);
5555
fieldType.setPrefixField(new PrefixFieldType(NAME, Defaults.FIELD_TYPE, Defaults.MIN_GRAM, Defaults.MAX_GRAM));
5656
fieldType.setShingleFields(new ShingleFieldType[] { new ShingleFieldType(fieldType.name(), 2, Defaults.FIELD_TYPE) });
5757
return fieldType;
@@ -62,7 +62,7 @@ public void testTermQuery() {
6262

6363
assertThat(fieldType.termQuery("foo", null), equalTo(new TermQuery(new Term(NAME, "foo"))));
6464

65-
SearchAsYouTypeFieldType unsearchable = new SearchAsYouTypeFieldType(NAME, UNSEARCHABLE, Collections.emptyMap());
65+
SearchAsYouTypeFieldType unsearchable = new SearchAsYouTypeFieldType(NAME, UNSEARCHABLE, null, Collections.emptyMap());
6666
final IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> unsearchable.termQuery("foo", null));
6767
assertThat(e.getMessage(), equalTo("Cannot search on field [" + NAME + "] since it is not indexed."));
6868
}
@@ -73,7 +73,7 @@ public void testTermsQuery() {
7373
assertThat(fieldType.termsQuery(asList("foo", "bar"), null),
7474
equalTo(new TermInSetQuery(NAME, asList(new BytesRef("foo"), new BytesRef("bar")))));
7575

76-
SearchAsYouTypeFieldType unsearchable = new SearchAsYouTypeFieldType(NAME, UNSEARCHABLE, Collections.emptyMap());
76+
SearchAsYouTypeFieldType unsearchable = new SearchAsYouTypeFieldType(NAME, UNSEARCHABLE, null, Collections.emptyMap());
7777
final IllegalArgumentException e =
7878
expectThrows(IllegalArgumentException.class, () -> unsearchable.termsQuery(asList("foo", "bar"), null));
7979
assertThat(e.getMessage(), equalTo("Cannot search on field [" + NAME + "] since it is not indexed."));

‎server/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ public static final class CompletionFieldType extends TermBasedFieldType {
195195
private ContextMappings contextMappings = null;
196196

197197
public CompletionFieldType(String name, FieldType luceneFieldType, Map<String, String> meta) {
198-
super(name, true, false, new TextSearchInfo(luceneFieldType), meta);
198+
super(name, true, false, new TextSearchInfo(luceneFieldType, null), meta);
199199
}
200200

201201
public CompletionFieldType(String name) {
@@ -402,7 +402,6 @@ public CompletionFieldMapper build(BuilderContext context) {
402402
ft.setIndexAnalyzer(indexAnalyzer);
403403
ft.setSearchAnalyzer(searchAnalyzer);
404404
ft.setSearchQuoteAnalyzer(searchQuoteAnalyzer);
405-
ft.setSimilarity(similarity);
406405
return new CompletionFieldMapper(name, this.fieldType, ft,
407406
multiFieldsBuilder.build(this, context), copyTo, maxInputLength);
408407
}

‎server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,6 @@
3232
import org.elasticsearch.common.xcontent.support.AbstractXContentParser;
3333
import org.elasticsearch.index.analysis.NamedAnalyzer;
3434
import org.elasticsearch.index.mapper.FieldNamesFieldMapper.FieldNamesFieldType;
35-
import org.elasticsearch.index.similarity.SimilarityProvider;
36-
import org.elasticsearch.index.similarity.SimilarityService;
3735

3836
import java.io.IOException;
3937
import java.util.ArrayList;
@@ -70,7 +68,6 @@ public abstract static class Builder<T extends Builder<T>> extends Mapper.Builde
7068
protected NamedAnalyzer indexAnalyzer;
7169
protected NamedAnalyzer searchAnalyzer;
7270
protected NamedAnalyzer searchQuoteAnalyzer;
73-
protected SimilarityProvider similarity;
7471

7572
protected Builder(String name, FieldType fieldType) {
7673
super(name);
@@ -159,11 +156,6 @@ public T searchQuoteAnalyzer(NamedAnalyzer searchQuoteAnalyzer) {
159156
return builder;
160157
}
161158

162-
public T similarity(SimilarityProvider similarity) {
163-
this.similarity = similarity;
164-
return builder;
165-
}
166-
167159
public T setEagerGlobalOrdinals(boolean eagerGlobalOrdinals) {
168160
this.eagerGlobalOrdinals = eagerGlobalOrdinals;
169161
return builder;
@@ -374,10 +366,6 @@ private void mergeSharedOptions(FieldMapper mergeWith, List<String> conflicts) {
374366
} else if (mappedFieldType.indexAnalyzer().name().equals(otherm.indexAnalyzer().name()) == false) {
375367
conflicts.add("mapper [" + name() + "] has different [analyzer]");
376368
}
377-
378-
if (Objects.equals(mappedFieldType.similarity(), otherm.similarity()) == false) {
379-
conflicts.add("mapper [" + name() + "] has different [similarity]");
380-
}
381369
}
382370

383371
/**
@@ -423,12 +411,6 @@ protected void doXContentBody(XContentBuilder builder, boolean includeDefaults,
423411
builder.field("store", fieldType.stored());
424412
}
425413

426-
if (fieldType().similarity() != null) {
427-
builder.field("similarity", fieldType().similarity().name());
428-
} else if (includeDefaults) {
429-
builder.field("similarity", SimilarityService.DEFAULT_SIMILARITY);
430-
}
431-
432414
multiFields.toXContent(builder, params);
433415
copyTo.toXContent(builder, params);
434416

‎server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
import org.elasticsearch.index.fielddata.plain.SortedSetOrdinalsIndexFieldData;
4444
import org.elasticsearch.index.query.QueryShardContext;
4545
import org.elasticsearch.index.similarity.SimilarityProvider;
46+
import org.elasticsearch.index.similarity.SimilarityService;
4647
import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;
4748

4849
import java.io.IOException;
@@ -52,6 +53,7 @@
5253
import java.util.Map;
5354
import java.util.Objects;
5455

56+
import static org.elasticsearch.index.mapper.TypeParsers.checkNull;
5557
import static org.elasticsearch.index.mapper.TypeParsers.parseField;
5658

5759
/**
@@ -97,6 +99,7 @@ public static class Builder extends FieldMapper.Builder<Builder> {
9799
private String normalizerName = "default";
98100
private boolean eagerGlobalOrdinals = Defaults.EAGER_GLOBAL_ORDINALS;
99101
private boolean splitQueriesOnWhitespace = Defaults.SPLIT_QUERIES_ON_WHITESPACE;
102+
private SimilarityProvider similarity;
100103

101104
public Builder(String name) {
102105
super(name, Defaults.FIELD_TYPE);
@@ -109,6 +112,10 @@ public Builder omitNorms(boolean omitNorms) {
109112
return builder;
110113
}
111114

115+
public void similarity(SimilarityProvider similarity) {
116+
this.similarity = similarity;
117+
}
118+
112119
public Builder ignoreAbove(int ignoreAbove) {
113120
if (ignoreAbove < 0) {
114121
throw new IllegalArgumentException("[ignore_above] must be positive, got " + ignoreAbove);
@@ -181,11 +188,11 @@ public static class TypeParser implements Mapper.TypeParser {
181188
@Override
182189
public Mapper.Builder<?> parse(String name, Map<String, Object> node, ParserContext parserContext) throws MapperParsingException {
183190
KeywordFieldMapper.Builder builder = new KeywordFieldMapper.Builder(name);
184-
parseField(builder, name, node, parserContext);
185191
for (Iterator<Map.Entry<String, Object>> iterator = node.entrySet().iterator(); iterator.hasNext();) {
186192
Map.Entry<String, Object> entry = iterator.next();
187193
String propName = entry.getKey();
188194
Object propNode = entry.getValue();
195+
checkNull(propName, propNode);
189196
if (propName.equals("null_value")) {
190197
if (propNode == null) {
191198
throw new MapperParsingException("Property [null_value] cannot be null.");
@@ -209,8 +216,13 @@ public Mapper.Builder<?> parse(String name, Map<String, Object> node, ParserCont
209216
} else if (propName.equals("split_queries_on_whitespace")) {
210217
builder.splitQueriesOnWhitespace(XContentMapValues.nodeBooleanValue(propNode, "split_queries_on_whitespace"));
211218
iterator.remove();
219+
} else if (propName.equals("similarity")) {
220+
SimilarityProvider similarityProvider = TypeParsers.resolveSimilarity(parserContext, name, propNode.toString());
221+
builder.similarity(similarityProvider);
222+
iterator.remove();
212223
}
213224
}
225+
parseField(builder, name, node, parserContext);
214226
return builder;
215227
}
216228
}
@@ -223,12 +235,11 @@ public KeywordFieldType(String name, boolean hasDocValues, FieldType fieldType,
223235
boolean eagerGlobalOrdinals, NamedAnalyzer normalizer, NamedAnalyzer searchAnalyzer,
224236
SimilarityProvider similarity, Map<String, String> meta, float boost) {
225237
super(name, fieldType.indexOptions() != IndexOptions.NONE,
226-
hasDocValues, new TextSearchInfo(fieldType), meta);
238+
hasDocValues, new TextSearchInfo(fieldType, similarity), meta);
227239
this.hasNorms = fieldType.omitNorms() == false;
228240
setEagerGlobalOrdinals(eagerGlobalOrdinals);
229241
setIndexAnalyzer(normalizer);
230242
setSearchAnalyzer(searchAnalyzer);
231-
setSimilarity(similarity);
232243
setBoost(boost);
233244
}
234245

@@ -422,6 +433,10 @@ protected void mergeOptions(FieldMapper other, List<String> conflicts) {
422433
if (Objects.equals(fieldType().indexAnalyzer(), k.fieldType().indexAnalyzer()) == false) {
423434
conflicts.add("mapper [" + name() + "] has different [normalizer]");
424435
}
436+
if (Objects.equals(mappedFieldType.getTextSearchInfo().getSimilarity(),
437+
k.fieldType().getTextSearchInfo().getSimilarity()) == false) {
438+
conflicts.add("mapper [" + name() + "] has different [similarity]");
439+
}
425440
this.ignoreAbove = k.ignoreAbove;
426441
this.splitQueriesOnWhitespace = k.splitQueriesOnWhitespace;
427442
this.fieldType().setSearchAnalyzer(k.fieldType().searchAnalyzer());
@@ -456,6 +471,12 @@ else if (includeDefaults) {
456471
builder.field("normalizer", "default");
457472
}
458473

474+
if (fieldType().getTextSearchInfo().getSimilarity() != null) {
475+
builder.field("similarity", fieldType().getTextSearchInfo().getSimilarity().name());
476+
} else if (includeDefaults) {
477+
builder.field("similarity", SimilarityService.DEFAULT_SIMILARITY);
478+
}
479+
459480
if (includeDefaults || splitQueriesOnWhitespace) {
460481
builder.field("split_queries_on_whitespace", splitQueriesOnWhitespace);
461482
}

0 commit comments

Comments
 (0)