Skip to content

Move MappedFieldType.similarity() to TextSearchInfo #58439

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.

Sign up for GitHub

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 10 commits into from
Jun 24, 2020
Merged

Move MappedFieldType.similarity() to TextSearchInfo #58439

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
18c1d83
Add TextSearchInfo class to wrap up text search details
romseygeek Jun 11, 2020
1ba04c8
Merge remote-tracking branch 'origin/master' into mapper/textsearchinfo
romseygeek Jun 17, 2020
9852fcd
javadocs
romseygeek Jun 17, 2020
9d30467
Merge remote-tracking branch 'origin/master' into mapper/textsearchinfo
romseygeek Jun 18, 2020
842883d
Don't expose lucene FieldType directly
romseygeek Jun 18, 2020
cedf0cf
rationalise; javadocs
romseygeek Jun 18, 2020
0dcbc22
Merge remote-tracking branch 'origin/master' into mapper/textsearchinfo
romseygeek Jun 23, 2020
c44158f
Add TextSearchInfo.NONE
romseygeek Jun 23, 2020
83bc7b6
Move MappedFieldType.similarity to TextSearchInfo
romseygeek Jun 23, 2020
5577d95
Merge remote-tracking branch 'origin/master' into mapper/similarity
romseygeek Jun 23, 2020
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
35 changes: 26 additions & 9 deletions ...apper-extras/src/main/java/org/elasticsearch/index/mapper/SearchAsYouTypeFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@
import org.elasticsearch.index.analysis.AnalyzerScope;
import org.elasticsearch.index.analysis.NamedAnalyzer;
import org.elasticsearch.index.query.QueryShardContext;
import org.elasticsearch.index.similarity.SimilarityProvider;
import org.elasticsearch.index.similarity.SimilarityService;

import java.io.IOException;
import java.util.ArrayList;
Expand All @@ -64,6 +66,7 @@

import static org.elasticsearch.common.xcontent.support.XContentMapValues.nodeIntegerValue;
import static org.elasticsearch.index.mapper.TextFieldMapper.TextFieldType.hasGaps;
import static org.elasticsearch.index.mapper.TypeParsers.checkNull;
import static org.elasticsearch.index.mapper.TypeParsers.parseTextField;

/**
Expand Down Expand Up @@ -113,30 +116,39 @@ public Mapper.Builder<?> parse(String name,
builder.indexAnalyzer(parserContext.getIndexAnalyzers().getDefaultIndexAnalyzer());
builder.searchAnalyzer(parserContext.getIndexAnalyzers().getDefaultSearchAnalyzer());
builder.searchQuoteAnalyzer(parserContext.getIndexAnalyzers().getDefaultSearchQuoteAnalyzer());
parseTextField(builder, name, node, parserContext);
for (Iterator<Map.Entry<String, Object>> iterator = node.entrySet().iterator(); iterator.hasNext();) {
final Map.Entry<String, Object> entry = iterator.next();
final String fieldName = entry.getKey();
final Object fieldNode = entry.getValue();

checkNull(fieldName, fieldNode);
if (fieldName.equals("max_shingle_size")) {
builder.maxShingleSize(nodeIntegerValue(fieldNode));
iterator.remove();
} else if (fieldName.equals("similarity")) {
SimilarityProvider similarityProvider = TypeParsers.resolveSimilarity(parserContext, fieldName, fieldNode.toString());
builder.similarity(similarityProvider);
iterator.remove();
}
// TODO should we allow to configure the prefix field
}
parseTextField(builder, name, node, parserContext);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The shared parseTextField() method does a deprecation check for similarity now, so we need to move it to after we've processed the similarity field. To ensure that mapper-specific fields still get checked for null, a new TypeParsers.checkNull() method is called for each entry.

return builder;
}
}

public static class Builder extends FieldMapper.Builder<Builder> {
private int maxShingleSize = Defaults.MAX_SHINGLE_SIZE;
private SimilarityProvider similarity;

public Builder(String name) {
super(name, Defaults.FIELD_TYPE);
this.builder = this;
}

public void similarity(SimilarityProvider similarity) {
this.similarity = similarity;
}

public Builder maxShingleSize(int maxShingleSize) {
if (maxShingleSize < MAX_SHINGLE_SIZE_LOWER_BOUND || maxShingleSize > MAX_SHINGLE_SIZE_UPPER_BOUND) {
throw new MapperParsingException("[max_shingle_size] must be at least [" + MAX_SHINGLE_SIZE_LOWER_BOUND + "] and at most " +
Expand All @@ -157,11 +169,10 @@ public Builder docValues(boolean docValues) {
@Override
public SearchAsYouTypeFieldMapper build(Mapper.BuilderContext context) {

SearchAsYouTypeFieldType ft = new SearchAsYouTypeFieldType(buildFullName(context), fieldType, meta);
SearchAsYouTypeFieldType ft = new SearchAsYouTypeFieldType(buildFullName(context), fieldType, similarity, meta);
ft.setIndexAnalyzer(indexAnalyzer);
ft.setSearchAnalyzer(searchAnalyzer);
ft.setSearchQuoteAnalyzer(searchQuoteAnalyzer);
ft.setSimilarity(similarity);

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

SearchAsYouTypeFieldType(String name, FieldType fieldType, Map<String, String> meta) {
super(name, fieldType.indexOptions() != IndexOptions.NONE, false, new TextSearchInfo(fieldType), meta);
SearchAsYouTypeFieldType(String name, FieldType fieldType, SimilarityProvider similarity, Map<String, String> meta) {
super(name, fieldType.indexOptions() != IndexOptions.NONE, false, new TextSearchInfo(fieldType, similarity), meta);
}

SearchAsYouTypeFieldType(SearchAsYouTypeFieldType other) {
Expand Down Expand Up @@ -382,7 +393,7 @@ static final class PrefixFieldType extends StringFieldType {
final String parentField;

PrefixFieldType(String parentField, FieldType fieldType, int minChars, int maxChars) {
super(parentField + PREFIX_FIELD_SUFFIX, true, false, new TextSearchInfo(fieldType), Collections.emptyMap());
super(parentField + PREFIX_FIELD_SUFFIX, true, false, new TextSearchInfo(fieldType, null), Collections.emptyMap());
this.minChars = minChars;
this.maxChars = maxChars;
this.parentField = parentField;
Expand Down Expand Up @@ -535,7 +546,7 @@ static class ShingleFieldType extends StringFieldType {
PrefixFieldType prefixFieldType;

ShingleFieldType(String name, int shingleSize, FieldType fieldType) {
super(name, true, false, new TextSearchInfo(fieldType), Collections.emptyMap());
super(name, true, false, new TextSearchInfo(fieldType, null), Collections.emptyMap());
this.shingleSize = shingleSize;
}

Expand Down Expand Up @@ -689,7 +700,8 @@ protected void mergeOptions(FieldMapper other, List<String> conflicts) {
this.shingleFields[i] = (ShingleFieldMapper) this.shingleFields[i].merge(m.shingleFields[i]);
}
}
if (Objects.equals(this.fieldType().similarity(), other.fieldType().similarity()) == false) {
if (Objects.equals(this.fieldType().getTextSearchInfo().getSimilarity(),
other.fieldType().getTextSearchInfo().getSimilarity()) == false) {
conflicts.add("mapper [" + name() + "] has different [similarity] settings");
}
}
Expand Down Expand Up @@ -719,6 +731,11 @@ public ShingleFieldMapper[] shingleFields() {
protected void doXContentBody(XContentBuilder builder, boolean includeDefaults, Params params) throws IOException {
super.doXContentBody(builder, includeDefaults, params);
doXContentAnalyzers(builder, includeDefaults);
if (fieldType().getTextSearchInfo().getSimilarity() != null) {
builder.field("similarity", fieldType().getTextSearchInfo().getSimilarity().name());
} else if (includeDefaults) {
builder.field("similarity", SimilarityService.DEFAULT_SIMILARITY);
}
builder.field("max_shingle_size", maxShingleSize);
}

Expand Down
7 changes: 7 additions & 0 deletions ...-extras/src/test/java/org/elasticsearch/index/mapper/SearchAsYouTypeFieldMapperTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@
import org.apache.lucene.search.Query;
import org.apache.lucene.search.SynonymQuery;
import org.apache.lucene.search.TermQuery;
import org.apache.lucene.search.similarities.BM25Similarity;
import org.apache.lucene.search.similarities.BooleanSimilarity;
import org.apache.lucene.search.spans.FieldMaskingSpanQuery;
import org.apache.lucene.search.spans.SpanNearQuery;
import org.apache.lucene.search.spans.SpanTermQuery;
Expand All @@ -56,6 +58,7 @@
import org.elasticsearch.index.query.MatchPhraseQueryBuilder;
import org.elasticsearch.index.query.MultiMatchQueryBuilder;
import org.elasticsearch.index.query.QueryShardContext;
import org.elasticsearch.index.similarity.SimilarityProvider;
import org.elasticsearch.plugins.Plugin;
import org.hamcrest.Matcher;
import org.hamcrest.Matchers;
Expand Down Expand Up @@ -88,6 +91,10 @@ public void addModifiers() {
a.maxShingleSize(3);
b.maxShingleSize(2);
});
addModifier("similarity", false, (a, b) -> {
a.similarity(new SimilarityProvider("BM25", new BM25Similarity()));
b.similarity(new SimilarityProvider("boolean", new BooleanSimilarity()));
});
}

@Override
Expand Down
6 changes: 3 additions & 3 deletions ...er-extras/src/test/java/org/elasticsearch/index/mapper/SearchAsYouTypeFieldTypeTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public class SearchAsYouTypeFieldTypeTests extends FieldTypeTestCase<MappedField

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

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

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

SearchAsYouTypeFieldType unsearchable = new SearchAsYouTypeFieldType(NAME, UNSEARCHABLE, Collections.emptyMap());
SearchAsYouTypeFieldType unsearchable = new SearchAsYouTypeFieldType(NAME, UNSEARCHABLE, null, Collections.emptyMap());
final IllegalArgumentException e =
expectThrows(IllegalArgumentException.class, () -> unsearchable.termsQuery(asList("foo", "bar"), null));
assertThat(e.getMessage(), equalTo("Cannot search on field [" + NAME + "] since it is not indexed."));
Expand Down
3 changes: 1 addition & 2 deletions server/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ public static final class CompletionFieldType extends TermBasedFieldType {
private ContextMappings contextMappings = null;

public CompletionFieldType(String name, FieldType luceneFieldType, Map<String, String> meta) {
super(name, true, false, new TextSearchInfo(luceneFieldType), meta);
super(name, true, false, new TextSearchInfo(luceneFieldType, null), meta);
}

public CompletionFieldType(String name) {
Expand Down Expand Up @@ -402,7 +402,6 @@ public CompletionFieldMapper build(BuilderContext context) {
ft.setIndexAnalyzer(indexAnalyzer);
ft.setSearchAnalyzer(searchAnalyzer);
ft.setSearchQuoteAnalyzer(searchQuoteAnalyzer);
ft.setSimilarity(similarity);
return new CompletionFieldMapper(name, this.fieldType, ft,
multiFieldsBuilder.build(this, context), copyTo, maxInputLength);
}
Expand Down
18 changes: 0 additions & 18 deletions server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@
import org.elasticsearch.common.xcontent.support.AbstractXContentParser;
import org.elasticsearch.index.analysis.NamedAnalyzer;
import org.elasticsearch.index.mapper.FieldNamesFieldMapper.FieldNamesFieldType;
import org.elasticsearch.index.similarity.SimilarityProvider;
import org.elasticsearch.index.similarity.SimilarityService;

import java.io.IOException;
import java.util.ArrayList;
Expand Down Expand Up @@ -70,7 +68,6 @@ public abstract static class Builder<T extends Builder<T>> extends Mapper.Builde
protected NamedAnalyzer indexAnalyzer;
protected NamedAnalyzer searchAnalyzer;
protected NamedAnalyzer searchQuoteAnalyzer;
protected SimilarityProvider similarity;

protected Builder(String name, FieldType fieldType) {
super(name);
Expand Down Expand Up @@ -159,11 +156,6 @@ public T searchQuoteAnalyzer(NamedAnalyzer searchQuoteAnalyzer) {
return builder;
}

public T similarity(SimilarityProvider similarity) {
this.similarity = similarity;
return builder;
}

public T setEagerGlobalOrdinals(boolean eagerGlobalOrdinals) {
this.eagerGlobalOrdinals = eagerGlobalOrdinals;
return builder;
Expand Down Expand Up @@ -374,10 +366,6 @@ private void mergeSharedOptions(FieldMapper mergeWith, List<String> conflicts) {
} else if (mappedFieldType.indexAnalyzer().name().equals(otherm.indexAnalyzer().name()) == false) {
conflicts.add("mapper [" + name() + "] has different [analyzer]");
}

if (Objects.equals(mappedFieldType.similarity(), otherm.similarity()) == false) {
conflicts.add("mapper [" + name() + "] has different [similarity]");
}
}

/**
Expand Down Expand Up @@ -423,12 +411,6 @@ protected void doXContentBody(XContentBuilder builder, boolean includeDefaults,
builder.field("store", fieldType.stored());
}

if (fieldType().similarity() != null) {
builder.field("similarity", fieldType().similarity().name());
} else if (includeDefaults) {
builder.field("similarity", SimilarityService.DEFAULT_SIMILARITY);
}

multiFields.toXContent(builder, params);
copyTo.toXContent(builder, params);

Expand Down
27 changes: 24 additions & 3 deletions server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import org.elasticsearch.index.fielddata.plain.SortedSetOrdinalsIndexFieldData;
import org.elasticsearch.index.query.QueryShardContext;
import org.elasticsearch.index.similarity.SimilarityProvider;
import org.elasticsearch.index.similarity.SimilarityService;
import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;

import java.io.IOException;
Expand All @@ -52,6 +53,7 @@
import java.util.Map;
import java.util.Objects;

import static org.elasticsearch.index.mapper.TypeParsers.checkNull;
import static org.elasticsearch.index.mapper.TypeParsers.parseField;

/**
Expand Down Expand Up @@ -97,6 +99,7 @@ public static class Builder extends FieldMapper.Builder<Builder> {
private String normalizerName = "default";
private boolean eagerGlobalOrdinals = Defaults.EAGER_GLOBAL_ORDINALS;
private boolean splitQueriesOnWhitespace = Defaults.SPLIT_QUERIES_ON_WHITESPACE;
private SimilarityProvider similarity;

public Builder(String name) {
super(name, Defaults.FIELD_TYPE);
Expand All @@ -109,6 +112,10 @@ public Builder omitNorms(boolean omitNorms) {
return builder;
}

public void similarity(SimilarityProvider similarity) {
this.similarity = similarity;
}

public Builder ignoreAbove(int ignoreAbove) {
if (ignoreAbove < 0) {
throw new IllegalArgumentException("[ignore_above] must be positive, got " + ignoreAbove);
Expand Down Expand Up @@ -181,11 +188,11 @@ public static class TypeParser implements Mapper.TypeParser {
@Override
public Mapper.Builder<?> parse(String name, Map<String, Object> node, ParserContext parserContext) throws MapperParsingException {
KeywordFieldMapper.Builder builder = new KeywordFieldMapper.Builder(name);
parseField(builder, name, node, parserContext);
for (Iterator<Map.Entry<String, Object>> iterator = node.entrySet().iterator(); iterator.hasNext();) {
Map.Entry<String, Object> entry = iterator.next();
String propName = entry.getKey();
Object propNode = entry.getValue();
checkNull(propName, propNode);
if (propName.equals("null_value")) {
if (propNode == null) {
throw new MapperParsingException("Property [null_value] cannot be null.");
Expand All @@ -209,8 +216,13 @@ public Mapper.Builder<?> parse(String name, Map<String, Object> node, ParserCont
} else if (propName.equals("split_queries_on_whitespace")) {
builder.splitQueriesOnWhitespace(XContentMapValues.nodeBooleanValue(propNode, "split_queries_on_whitespace"));
iterator.remove();
} else if (propName.equals("similarity")) {
SimilarityProvider similarityProvider = TypeParsers.resolveSimilarity(parserContext, name, propNode.toString());
builder.similarity(similarityProvider);
iterator.remove();
}
}
parseField(builder, name, node, parserContext);
return builder;
}
}
Expand All @@ -223,12 +235,11 @@ public KeywordFieldType(String name, boolean hasDocValues, FieldType fieldType,
boolean eagerGlobalOrdinals, NamedAnalyzer normalizer, NamedAnalyzer searchAnalyzer,
SimilarityProvider similarity, Map<String, String> meta, float boost) {
super(name, fieldType.indexOptions() != IndexOptions.NONE,
hasDocValues, new TextSearchInfo(fieldType), meta);
hasDocValues, new TextSearchInfo(fieldType, similarity), meta);
this.hasNorms = fieldType.omitNorms() == false;
setEagerGlobalOrdinals(eagerGlobalOrdinals);
setIndexAnalyzer(normalizer);
setSearchAnalyzer(searchAnalyzer);
setSimilarity(similarity);
setBoost(boost);
}

Expand Down Expand Up @@ -422,6 +433,10 @@ protected void mergeOptions(FieldMapper other, List<String> conflicts) {
if (Objects.equals(fieldType().indexAnalyzer(), k.fieldType().indexAnalyzer()) == false) {
conflicts.add("mapper [" + name() + "] has different [normalizer]");
}
if (Objects.equals(mappedFieldType.getTextSearchInfo().getSimilarity(),
k.fieldType().getTextSearchInfo().getSimilarity()) == false) {
conflicts.add("mapper [" + name() + "] has different [similarity]");
}
this.ignoreAbove = k.ignoreAbove;
this.splitQueriesOnWhitespace = k.splitQueriesOnWhitespace;
this.fieldType().setSearchAnalyzer(k.fieldType().searchAnalyzer());
Expand Down Expand Up @@ -456,6 +471,12 @@ else if (includeDefaults) {
builder.field("normalizer", "default");
}

if (fieldType().getTextSearchInfo().getSimilarity() != null) {
builder.field("similarity", fieldType().getTextSearchInfo().getSimilarity().name());
} else if (includeDefaults) {
builder.field("similarity", SimilarityService.DEFAULT_SIMILARITY);
}

if (includeDefaults || splitQueriesOnWhitespace) {
builder.field("split_queries_on_whitespace", splitQueriesOnWhitespace);
}
Expand Down
Loading