Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move index analyzer management to FieldMapper/MapperService #63937

Merged
merged 31 commits into from
Nov 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
34977e4
Centralize index analyzer management
romseygeek Oct 20, 2020
3d8dfc7
Merge remote-tracking branch 'origin/master' into mapper/indexanalyzer
romseygeek Oct 20, 2020
d098bcd
precommit
romseygeek Oct 20, 2020
3a8807a
Merge remote-tracking branch 'origin/master' into mapper/indexanalyzer
romseygeek Oct 20, 2020
4cccf09
delegating analyzers
romseygeek Oct 20, 2020
c9437df
precommit
romseygeek Oct 21, 2020
1885058
Merge remote-tracking branch 'origin/master' into mapper/indexanalyzer
romseygeek Oct 21, 2020
32382ad
position increments on annotations
romseygeek Oct 21, 2020
0b963be
Merge remote-tracking branch 'origin/master' into mapper/indexanalyzer
romseygeek Oct 26, 2020
d6adf80
Merge remote-tracking branch 'origin/master' into mapper/indexanalyzer
romseygeek Oct 27, 2020
f1b8095
Move to returning maps
romseygeek Oct 27, 2020
bb81e25
imports
romseygeek Oct 27, 2020
2585186
Merge remote-tracking branch 'origin/master' into mapper/indexanalyzer
romseygeek Oct 27, 2020
4f3da1d
Make everything a NamedAnalyzer
romseygeek Oct 29, 2020
be55995
Merge remote-tracking branch 'origin/master' into mapper/indexanalyzer
romseygeek Oct 29, 2020
73044b5
warnings
romseygeek Oct 29, 2020
25f3c0d
sigtext
romseygeek Oct 29, 2020
c04c7d1
Collapse ParametrizedFieldMapper into FieldMapper
romseygeek Oct 29, 2020
639126e
SAYT shouldn't try to add fields if index=false and store=false
romseygeek Oct 29, 2020
feea031
Merge remote-tracking branch 'origin/master' into mapper/indexanalyzer
romseygeek Nov 2, 2020
9ee7dda
Merge branch 'mapper/fieldmapper' into mapper/indexanalyzer
romseygeek Nov 2, 2020
88ad7e5
Make indexAnalyzers passed via constructor
romseygeek Nov 2, 2020
040ec02
Merge remote-tracking branch 'origin/master' into mapper/indexanalyzer
romseygeek Nov 2, 2020
0d2a081
SearchAsYouType tests
romseygeek Nov 2, 2020
7b28b2c
Merge remote-tracking branch 'origin/master' into mapper/indexanalyzer
romseygeek Nov 3, 2020
22357b7
Merge branch 'master' into mapper/indexanalyzer
elasticmachine Nov 3, 2020
4793097
Merge branch 'master' into mapper/indexanalyzer
elasticmachine Nov 4, 2020
a9051a4
Merge remote-tracking branch 'origin/master' into mapper/indexanalyzer
romseygeek Nov 4, 2020
342525c
Merge remote-tracking branch 'romseygeek/mapper/indexanalyzer' into m…
romseygeek Nov 4, 2020
5099a60
Don't call FetchContext#mapperService
romseygeek Nov 4, 2020
e4f7882
No really, don't use mapperservice
romseygeek Nov 4, 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
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ public static final class RankFeatureFieldType extends MappedFieldType {
public RankFeatureFieldType(String name, Map<String, String> meta, boolean positiveScoreImpact) {
super(name, true, false, false, TextSearchInfo.NONE, meta);
this.positiveScoreImpact = positiveScoreImpact;
setIndexAnalyzer(Lucene.KEYWORD_ANALYZER);
}

@Override
Expand Down Expand Up @@ -136,7 +135,7 @@ public Query termQuery(Object value, QueryShardContext context) {

private RankFeatureFieldMapper(String simpleName, MappedFieldType mappedFieldType,
MultiFields multiFields, CopyTo copyTo, boolean positiveScoreImpact) {
super(simpleName, mappedFieldType, multiFields, copyTo);
super(simpleName, mappedFieldType, Lucene.KEYWORD_ANALYZER, multiFields, copyTo);
this.positiveScoreImpact = positiveScoreImpact;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ public static final class RankFeaturesFieldType extends MappedFieldType {

public RankFeaturesFieldType(String name, Map<String, String> meta) {
super(name, false, false, false, TextSearchInfo.NONE, meta);
setIndexAnalyzer(Lucene.KEYWORD_ANALYZER);
}

@Override
Expand Down Expand Up @@ -99,7 +98,7 @@ public Query termQuery(Object value, QueryShardContext context) {

private RankFeaturesFieldMapper(String simpleName, MappedFieldType mappedFieldType,
MultiFields multiFields, CopyTo copyTo) {
super(simpleName, mappedFieldType, multiFields, copyTo);
super(simpleName, mappedFieldType, Lucene.KEYWORD_ANALYZER, multiFields, copyTo);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -95,18 +96,14 @@ public static class Defaults {
public static final TypeParser PARSER
= new TypeParser((n, c) -> new Builder(n, () -> c.getIndexAnalyzers().getDefaultIndexAnalyzer()));

private static SearchAsYouTypeFieldMapper toType(FieldMapper in) {
return (SearchAsYouTypeFieldMapper) in;
}

private static SearchAsYouTypeFieldType ft(FieldMapper in) {
return toType(in).fieldType();
private static Builder builder(FieldMapper in) {
return ((SearchAsYouTypeFieldMapper)in).builder;
}

public static class Builder extends FieldMapper.Builder {

private final Parameter<Boolean> index = Parameter.indexParam(m -> toType(m).index, true);
private final Parameter<Boolean> store = Parameter.storeParam(m -> toType(m).store, false);
private final Parameter<Boolean> index = Parameter.indexParam(m -> builder(m).index.get(), true);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These changes are due to position increment handling moving directly into TextParams.Analyzers, which simplifies a lot of the palaver around building analyzers for text fields. SearchAsYouType doesn't actually expose a position increment field so doesn't get the added benefits here, but it still requires a small refactor.

private final Parameter<Boolean> store = Parameter.storeParam(m -> builder(m).store.get(), false);

// This is only here because for some reason the initial impl of this always serialized
// `doc_values=false`, even though it cannot be set; and so we need to continue
Expand All @@ -120,7 +117,7 @@ public static class Builder extends FieldMapper.Builder {
.alwaysSerialize();

private final Parameter<Integer> maxShingleSize = Parameter.intParam("max_shingle_size", false,
m -> toType(m).maxShingleSize, Defaults.MAX_SHINGLE_SIZE)
m -> builder(m).maxShingleSize.get(), Defaults.MAX_SHINGLE_SIZE)
.setValidator(v -> {
if (v < MAX_SHINGLE_SIZE_LOWER_BOUND || v > MAX_SHINGLE_SIZE_UPPER_BOUND) {
throw new MapperParsingException("[max_shingle_size] must be at least [" + MAX_SHINGLE_SIZE_LOWER_BOUND
Expand All @@ -130,17 +127,17 @@ public static class Builder extends FieldMapper.Builder {
.alwaysSerialize();

final TextParams.Analyzers analyzers;
final Parameter<SimilarityProvider> similarity = TextParams.similarity(m -> ft(m).getTextSearchInfo().getSimilarity());
final Parameter<SimilarityProvider> similarity = TextParams.similarity(m -> builder(m).similarity.get());

final Parameter<String> indexOptions = TextParams.indexOptions(m -> toType(m).indexOptions);
final Parameter<Boolean> norms = TextParams.norms(true, m -> ft(m).getTextSearchInfo().hasNorms());
final Parameter<String> termVectors = TextParams.termVectors(m -> toType(m).termVectors);
final Parameter<String> indexOptions = TextParams.indexOptions(m -> builder(m).indexOptions.get());
final Parameter<Boolean> norms = TextParams.norms(true, m -> builder(m).norms.get());
final Parameter<String> termVectors = TextParams.termVectors(m -> builder(m).termVectors.get());

private final Parameter<Map<String, String>> meta = Parameter.metaParam();

public Builder(String name, Supplier<NamedAnalyzer> defaultAnalyzer) {
super(name);
this.analyzers = new TextParams.Analyzers(defaultAnalyzer);
this.analyzers = new TextParams.Analyzers(defaultAnalyzer, m -> builder(m).analyzers);
}

@Override
Expand All @@ -159,12 +156,15 @@ public SearchAsYouTypeFieldMapper build(Mapper.BuilderContext context) {
fieldType.setStored(store.getValue());
TextParams.setTermVectorParams(termVectors.getValue(), fieldType);

Map<String, NamedAnalyzer> indexAnalyzers = new HashMap<>();

NamedAnalyzer indexAnalyzer = analyzers.getIndexAnalyzer();
NamedAnalyzer searchAnalyzer = analyzers.getSearchAnalyzer();

SearchAsYouTypeFieldType ft = new SearchAsYouTypeFieldType(buildFullName(context), fieldType, similarity.getValue(),
analyzers.getSearchAnalyzer(), analyzers.getSearchQuoteAnalyzer(), meta.getValue());
ft.setIndexAnalyzer(analyzers.getIndexAnalyzer());

indexAnalyzers.put(ft.name(), indexAnalyzer);

// set up the prefix field
FieldType prefixft = new FieldType(fieldType);
Expand All @@ -182,8 +182,10 @@ public SearchAsYouTypeFieldMapper build(Mapper.BuilderContext context) {
TextSearchInfo prefixSearchInfo = new TextSearchInfo(prefixft, similarity.getValue(), prefixSearchWrapper, searchAnalyzer);
final PrefixFieldType prefixFieldType
= new PrefixFieldType(fullName, prefixSearchInfo, Defaults.MIN_GRAM, Defaults.MAX_GRAM);
prefixFieldType.setIndexAnalyzer(new NamedAnalyzer(indexAnalyzer.name(), AnalyzerScope.INDEX, prefixIndexWrapper));
final NamedAnalyzer prefixAnalyzer
= new NamedAnalyzer(indexAnalyzer.name(), AnalyzerScope.INDEX, prefixIndexWrapper);
final PrefixFieldMapper prefixFieldMapper = new PrefixFieldMapper(prefixft, prefixFieldType);
indexAnalyzers.put(prefixFieldType.name(), prefixAnalyzer);

// set up the shingle fields
final ShingleFieldMapper[] shingleFieldMappers = new ShingleFieldMapper[maxShingleSize.getValue() - 1];
Expand All @@ -203,14 +205,16 @@ public SearchAsYouTypeFieldMapper build(Mapper.BuilderContext context) {
TextSearchInfo textSearchInfo
= new TextSearchInfo(shingleft, similarity.getValue(), shingleSearchWrapper, shingleSearchQuoteWrapper);
final ShingleFieldType shingleFieldType = new ShingleFieldType(fieldName, shingleSize, textSearchInfo);
shingleFieldType.setIndexAnalyzer(new NamedAnalyzer(indexAnalyzer.name(), AnalyzerScope.INDEX, shingleIndexWrapper));
shingleFieldType.setPrefixFieldType(prefixFieldType);
shingleFieldTypes[i] = shingleFieldType;
NamedAnalyzer shingleAnalyzer
= new NamedAnalyzer(indexAnalyzer.name(), AnalyzerScope.INDEX, shingleIndexWrapper);
shingleFieldMappers[i] = new ShingleFieldMapper(shingleft, shingleFieldType);
indexAnalyzers.put(shingleFieldType.name(), shingleAnalyzer);
}
ft.setPrefixField(prefixFieldType);
ft.setShingleFields(shingleFieldTypes);
return new SearchAsYouTypeFieldMapper(name, ft, copyTo.build(), prefixFieldMapper, shingleFieldMappers, this);
return new SearchAsYouTypeFieldMapper(name, ft, copyTo.build(), indexAnalyzers, prefixFieldMapper, shingleFieldMappers, this);
}
}

Expand Down Expand Up @@ -546,29 +550,23 @@ public SpanQuery spanPrefixQuery(String value, SpanMultiTermQueryWrapper.SpanRew
}
}

private final boolean index;
private final boolean store;
private final String indexOptions;
private final String termVectors;

private final int maxShingleSize;
private final PrefixFieldMapper prefixField;
private final ShingleFieldMapper[] shingleFields;
private final Builder builder;

public SearchAsYouTypeFieldMapper(String simpleName,
SearchAsYouTypeFieldType mappedFieldType,
CopyTo copyTo,
Map<String, NamedAnalyzer> indexAnalyzers,
PrefixFieldMapper prefixField,
ShingleFieldMapper[] shingleFields,
Builder builder) {
super(simpleName, mappedFieldType, MultiFields.empty(), copyTo);
super(simpleName, mappedFieldType, indexAnalyzers, MultiFields.empty(), copyTo);
this.prefixField = prefixField;
this.shingleFields = shingleFields;
this.maxShingleSize = builder.maxShingleSize.getValue();
this.index = builder.index.getValue();
this.store = builder.store.getValue();
this.indexOptions = builder.indexOptions.getValue();
this.termVectors = builder.termVectors.getValue();
this.builder = builder;
}

@Override
Expand All @@ -578,12 +576,12 @@ protected void parseCreateField(ParseContext context) throws IOException {
return;
}

if (this.index == false && this.store == false) {
if (this.builder.index.get() == false && this.builder.store.get() == false) {
return;
}

context.doc().add(new Field(fieldType().name(), value, fieldType().fieldType));
if (this.index) {
if (this.builder.index.get()) {
for (ShingleFieldMapper subFieldMapper : shingleFields) {
context.doc().add(new Field(subFieldMapper.fieldType().name(), value, subFieldMapper.getLuceneFieldType()));
}
Expand All @@ -599,9 +597,8 @@ protected String contentType() {
return CONTENT_TYPE;
}

@Override
public FieldMapper.Builder getMergeBuilder() {
return new Builder(simpleName(), () -> fieldType().indexAnalyzer()).init(this);
return new Builder(simpleName(), builder.analyzers.indexAnalyzer::getDefaultValue).init(this);
}

public static String getShingleFieldName(String parentField, int shingleSize) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,12 +179,12 @@ public void testDefaultConfiguration() throws IOException {
assertRootFieldMapper(rootMapper, 3, "default");

PrefixFieldMapper prefixFieldMapper = getPrefixFieldMapper(defaultMapper, "field._index_prefix");
assertPrefixFieldType(prefixFieldMapper.fieldType(), 3, "default");
assertPrefixFieldType(prefixFieldMapper, rootMapper.indexAnalyzers(), 3, "default");

assertShingleFieldType(
getShingleFieldMapper(defaultMapper, "field._2gram").fieldType(), 2, "default", prefixFieldMapper.fieldType());
assertShingleFieldType(
getShingleFieldMapper(defaultMapper, "field._3gram").fieldType(), 3, "default", prefixFieldMapper.fieldType());
assertShingleFieldType(getShingleFieldMapper(defaultMapper, "field._2gram"),
rootMapper.indexAnalyzers(), 2, "default", prefixFieldMapper.fieldType());
assertShingleFieldType(getShingleFieldMapper(defaultMapper, "field._3gram"),
rootMapper.indexAnalyzers(), 3, "default", prefixFieldMapper.fieldType());
}

public void testConfiguration() throws IOException {
Expand All @@ -200,14 +200,14 @@ public void testConfiguration() throws IOException {
assertRootFieldMapper(rootMapper, maxShingleSize, analyzerName);

PrefixFieldMapper prefixFieldMapper = getPrefixFieldMapper(defaultMapper, "field._index_prefix");
assertPrefixFieldType(prefixFieldMapper.fieldType(), maxShingleSize, analyzerName);
assertPrefixFieldType(prefixFieldMapper, rootMapper.indexAnalyzers(), maxShingleSize, analyzerName);

assertShingleFieldType(
getShingleFieldMapper(defaultMapper, "field._2gram").fieldType(), 2, analyzerName, prefixFieldMapper.fieldType());
assertShingleFieldType(
getShingleFieldMapper(defaultMapper, "field._3gram").fieldType(), 3, analyzerName, prefixFieldMapper.fieldType());
assertShingleFieldType(
getShingleFieldMapper(defaultMapper, "field._4gram").fieldType(), 4, analyzerName, prefixFieldMapper.fieldType());
assertShingleFieldType(getShingleFieldMapper(defaultMapper, "field._2gram"),
rootMapper.indexAnalyzers(), 2, analyzerName, prefixFieldMapper.fieldType());
assertShingleFieldType(getShingleFieldMapper(defaultMapper, "field._3gram"),
rootMapper.indexAnalyzers(), 3, analyzerName, prefixFieldMapper.fieldType());
assertShingleFieldType(getShingleFieldMapper(defaultMapper, "field._4gram"),
rootMapper.indexAnalyzers(), 4, analyzerName, prefixFieldMapper.fieldType());
}

public void testSimpleMerge() throws IOException {
Expand Down Expand Up @@ -599,47 +599,53 @@ private static void assertRootFieldMapper(SearchAsYouTypeFieldMapper mapper,

assertThat(mapper.maxShingleSize(), equalTo(maxShingleSize));
assertThat(mapper.fieldType(), notNullValue());
assertSearchAsYouTypeFieldType(mapper.fieldType(), maxShingleSize, analyzerName, mapper.prefixField().fieldType());
assertSearchAsYouTypeFieldType(mapper, mapper.fieldType(), maxShingleSize, analyzerName, mapper.prefixField().fieldType());

assertThat(mapper.prefixField(), notNullValue());
assertThat(mapper.prefixField().fieldType().parentField, equalTo(mapper.name()));
assertPrefixFieldType(mapper.prefixField().fieldType(), maxShingleSize, analyzerName);
assertPrefixFieldType(mapper.prefixField(), mapper.indexAnalyzers, maxShingleSize, analyzerName);


for (int shingleSize = 2; shingleSize <= maxShingleSize; shingleSize++) {
final ShingleFieldMapper shingleFieldMapper = mapper.shingleFields()[shingleSize - 2];
assertThat(shingleFieldMapper, notNullValue());
assertShingleFieldType(shingleFieldMapper.fieldType(), shingleSize, analyzerName, mapper.prefixField().fieldType());
assertShingleFieldType(shingleFieldMapper, mapper.indexAnalyzers, shingleSize,
analyzerName, mapper.prefixField().fieldType());
}

final int numberOfShingleSubfields = (maxShingleSize - 2) + 1;
assertThat(mapper.shingleFields().length, equalTo(numberOfShingleSubfields));
}

private static void assertSearchAsYouTypeFieldType(SearchAsYouTypeFieldType fieldType, int maxShingleSize,
private static void assertSearchAsYouTypeFieldType(SearchAsYouTypeFieldMapper mapper,
SearchAsYouTypeFieldType fieldType,
int maxShingleSize,
String analyzerName,
PrefixFieldType prefixFieldType) {

assertThat(fieldType.shingleFields.length, equalTo(maxShingleSize - 1));
for (NamedAnalyzer analyzer : asList(fieldType.indexAnalyzer(), fieldType.getTextSearchInfo().getSearchAnalyzer())) {
NamedAnalyzer indexAnalyzer = mapper.indexAnalyzers().get(fieldType.name());
for (NamedAnalyzer analyzer : asList(indexAnalyzer, fieldType.getTextSearchInfo().getSearchAnalyzer())) {
assertThat(analyzer.name(), equalTo(analyzerName));
}
int shingleSize = 2;
for (ShingleFieldType shingleField : fieldType.shingleFields) {
assertShingleFieldType(shingleField, shingleSize++, analyzerName, prefixFieldType);
for (ShingleFieldMapper shingleField : mapper.shingleFields()) {
assertShingleFieldType(shingleField, mapper.indexAnalyzers(), shingleSize++, analyzerName, prefixFieldType);
}

assertThat(fieldType.prefixField, equalTo(prefixFieldType));
}

private static void assertShingleFieldType(ShingleFieldType fieldType,
private static void assertShingleFieldType(ShingleFieldMapper mapper,
Map<String, NamedAnalyzer> indexAnalyzers,
int shingleSize,
String analyzerName,
PrefixFieldType prefixFieldType) {

ShingleFieldType fieldType = mapper.fieldType();
assertThat(fieldType.shingleSize, equalTo(shingleSize));

for (NamedAnalyzer analyzer : asList(fieldType.indexAnalyzer(), fieldType.getTextSearchInfo().getSearchAnalyzer())) {
for (NamedAnalyzer analyzer : asList(indexAnalyzers.get(fieldType.name()), fieldType.getTextSearchInfo().getSearchAnalyzer())) {
assertThat(analyzer.name(), equalTo(analyzerName));
if (shingleSize > 1) {
final SearchAsYouTypeAnalyzer wrappedAnalyzer = (SearchAsYouTypeAnalyzer) analyzer.analyzer();
Expand All @@ -652,12 +658,15 @@ private static void assertShingleFieldType(ShingleFieldType fieldType,

}

private static void assertPrefixFieldType(PrefixFieldType fieldType, int shingleSize, String analyzerName) {
for (NamedAnalyzer analyzer : asList(fieldType.indexAnalyzer(), fieldType.getTextSearchInfo().getSearchAnalyzer())) {
private static void assertPrefixFieldType(PrefixFieldMapper mapper, Map<String, NamedAnalyzer> indexAnalyzers,
int shingleSize, String analyzerName) {
PrefixFieldType fieldType = mapper.fieldType();
NamedAnalyzer indexAnalyzer = indexAnalyzers.get(fieldType.name());
for (NamedAnalyzer analyzer : asList(indexAnalyzer, fieldType.getTextSearchInfo().getSearchAnalyzer())) {
assertThat(analyzer.name(), equalTo(analyzerName));
}

final SearchAsYouTypeAnalyzer wrappedIndexAnalyzer = (SearchAsYouTypeAnalyzer) fieldType.indexAnalyzer().analyzer();
final SearchAsYouTypeAnalyzer wrappedIndexAnalyzer = (SearchAsYouTypeAnalyzer) indexAnalyzer.analyzer();
final SearchAsYouTypeAnalyzer wrappedSearchAnalyzer
= (SearchAsYouTypeAnalyzer) fieldType.getTextSearchInfo().getSearchAnalyzer().analyzer();
for (SearchAsYouTypeAnalyzer analyzer : asList(wrappedIndexAnalyzer, wrappedSearchAnalyzer)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,6 @@ public void testPrefixQuery() {

public void testFetchSourceValue() throws IOException {
SearchAsYouTypeFieldType fieldType = createFieldType();
fieldType.setIndexAnalyzer(Lucene.STANDARD_ANALYZER);

assertEquals(List.of("value"), fetchSourceValue(fieldType, "value"));
assertEquals(List.of("42"), fetchSourceValue(fieldType, 42L));
Expand Down
Loading