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.

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

Fix TextFieldMapper Retaining a Reference to its Builder #77251

Merged
merged 1 commit into from
Sep 3, 2021
Merged

Fix TextFieldMapper Retaining a Reference to its Builder #77251

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
14 changes: 10 additions & 4 deletions .../mapper-extras/src/main/java/org/elasticsearch/index/mapper/MatchOnlyTextFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,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).analyzers);
this.analyzers = new TextParams.Analyzers(indexAnalyzers,
m -> ((MatchOnlyTextFieldMapper) m).indexAnalyzer,
m -> ((MatchOnlyTextFieldMapper) m).positionIncrementGap);
}

@Override
Expand Down Expand Up @@ -268,7 +270,9 @@ public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName, S
}

private final Version indexCreatedVersion;
private final TextParams.Analyzers analyzers;
private final IndexAnalyzers indexAnalyzers;
private final NamedAnalyzer indexAnalyzer;
private final int positionIncrementGap;
private final FieldType fieldType;

private MatchOnlyTextFieldMapper(
Expand All @@ -284,12 +288,14 @@ private MatchOnlyTextFieldMapper(
assert mappedFieldType.hasDocValues() == false;
this.fieldType = fieldType;
this.indexCreatedVersion = builder.indexCreatedVersion;
this.analyzers = builder.analyzers;
this.indexAnalyzers = builder.analyzers.indexAnalyzers;
this.indexAnalyzer = builder.analyzers.getIndexAnalyzer();
this.positionIncrementGap = builder.analyzers.positionIncrementGap.getValue();
}

@Override
public FieldMapper.Builder getMergeBuilder() {
return new Builder(simpleName(), indexCreatedVersion, analyzers.indexAnalyzers).init(this);
return new Builder(simpleName(), indexCreatedVersion, indexAnalyzers).init(this);
}

@Override
Expand Down
6 changes: 5 additions & 1 deletion ...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 @@ -125,7 +125,11 @@ public static class Builder extends FieldMapper.Builder {

public Builder(String name, IndexAnalyzers indexAnalyzers) {
super(name);
this.analyzers = new TextParams.Analyzers(indexAnalyzers, m -> builder(m).analyzers);
this.analyzers = new TextParams.Analyzers(
indexAnalyzers,
m -> builder(m).analyzers.getIndexAnalyzer(),
m -> builder(m).analyzers.positionIncrementGap.getValue()
);
}

@Override
Expand Down
4 changes: 3 additions & 1 deletion .../src/main/java/org/elasticsearch/index/mapper/annotatedtext/AnnotatedTextFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,9 @@ public static class Builder extends FieldMapper.Builder {

public Builder(String name, IndexAnalyzers indexAnalyzers) {
super(name);
this.analyzers = new TextParams.Analyzers(indexAnalyzers, m -> builder(m).analyzers);
this.analyzers = new TextParams.Analyzers(indexAnalyzers,
m -> builder(m).analyzers.getIndexAnalyzer(),
m -> builder(m).analyzers.positionIncrementGap.getValue());
}

@Override
Expand Down
100 changes: 64 additions & 36 deletions server/src/main/java/org/elasticsearch/index/mapper/TextFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,10 +106,6 @@ public static class Defaults {
public static final int POSITION_INCREMENT_GAP = 100;
}

private static Builder builder(FieldMapper in) {
return ((TextFieldMapper) in).builder;
}

private static final class PrefixConfig implements ToXContent {
final int minChars;
final int maxChars;
Expand Down Expand Up @@ -229,27 +225,26 @@ public static class Builder extends FieldMapper.Builder {

private final Version indexCreatedVersion;

private final Parameter<Boolean> index = Parameter.indexParam(m -> builder(m).index.getValue(), true);
private final Parameter<Boolean> store = Parameter.storeParam(m -> builder(m).store.getValue(), false);
private final Parameter<Boolean> index = Parameter.indexParam(m -> ((TextFieldMapper) m).index, true);
private final Parameter<Boolean> store = Parameter.storeParam(m -> ((TextFieldMapper) m).store, false);

final Parameter<SimilarityProvider> similarity
= TextParams.similarity(m -> builder(m).similarity.getValue());
final Parameter<SimilarityProvider> similarity = TextParams.similarity(m -> ((TextFieldMapper) m).similarity);

final Parameter<String> indexOptions = TextParams.indexOptions(m -> builder(m).indexOptions.getValue());
final Parameter<Boolean> norms = TextParams.norms(true, m -> builder(m).norms.getValue());
final Parameter<String> termVectors = TextParams.termVectors(m -> builder(m).termVectors.getValue());
final Parameter<String> indexOptions = TextParams.indexOptions(m -> ((TextFieldMapper) m).indexOptions);
final Parameter<Boolean> norms = TextParams.norms(true, m -> ((TextFieldMapper) m).norms);
final Parameter<String> termVectors = TextParams.termVectors(m -> ((TextFieldMapper) m).termVectors);

final Parameter<Boolean> fieldData
= Parameter.boolParam("fielddata", true, m -> builder(m).fieldData.getValue(), false);
= Parameter.boolParam("fielddata", true, m -> ((TextFieldMapper) m).fieldData, false);
final Parameter<FielddataFrequencyFilter> freqFilter = new Parameter<>("fielddata_frequency_filter", true,
() -> DEFAULT_FILTER, TextFieldMapper::parseFrequencyFilter, m -> builder(m).freqFilter.getValue());
() -> DEFAULT_FILTER, TextFieldMapper::parseFrequencyFilter, m -> ((TextFieldMapper) m).freqFilter);
final Parameter<Boolean> eagerGlobalOrdinals
= Parameter.boolParam("eager_global_ordinals", true, m -> builder(m).eagerGlobalOrdinals.getValue(), false);
= Parameter.boolParam("eager_global_ordinals", true, m -> ((TextFieldMapper) m).eagerGlobalOrdinals, false);

final Parameter<Boolean> indexPhrases
= Parameter.boolParam("index_phrases", false, m -> builder(m).indexPhrases.getValue(), false);
= Parameter.boolParam("index_phrases", false, m -> ((TextFieldMapper) m).indexPhrases, false);
final Parameter<PrefixConfig> indexPrefixes = new Parameter<>("index_prefixes", false,
() -> null, TextFieldMapper::parsePrefixConfig, m -> builder(m).indexPrefixes.getValue()).acceptsNull();
() -> null, TextFieldMapper::parsePrefixConfig, m -> ((TextFieldMapper) m).indexPrefixes).acceptsNull();

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

Expand All @@ -262,7 +257,11 @@ 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 -> builder(m).analyzers);
this.analyzers = new TextParams.Analyzers(
indexAnalyzers,
m -> ((TextFieldMapper) m).indexAnalyzer,
m -> (((TextFieldMapper) m).positionIncrementGap)
);
}

public Builder index(boolean index) {
Expand Down Expand Up @@ -820,7 +819,21 @@ public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName, S

}

private final Builder builder;
private final Version indexCreatedVersion;
private final boolean index;
private final boolean store;
private final String indexOptions;
private final boolean norms;
private final String termVectors;
private final SimilarityProvider similarity;
private final NamedAnalyzer indexAnalyzer;
private final IndexAnalyzers indexAnalyzers;
private final int positionIncrementGap;
private final boolean eagerGlobalOrdinals;
private final PrefixConfig indexPrefixes;
private final FielddataFrequencyFilter freqFilter;
private final boolean fieldData;
private final boolean indexPhrases;
private final FieldType fieldType;
private final SubFieldInfo prefixFieldInfo;
private final SubFieldInfo phraseFieldInfo;
Expand All @@ -840,12 +853,26 @@ protected TextFieldMapper(String simpleName, FieldType fieldType,
this.fieldType = fieldType;
this.prefixFieldInfo = prefixFieldInfo;
this.phraseFieldInfo = phraseFieldInfo;
this.builder = builder;
this.indexCreatedVersion = builder.indexCreatedVersion;
this.indexAnalyzer = builder.analyzers.getIndexAnalyzer();
this.indexAnalyzers = builder.analyzers.indexAnalyzers;
this.positionIncrementGap = builder.analyzers.positionIncrementGap.getValue();
this.index = builder.index.getValue();
this.store = builder.store.getValue();
this.similarity = builder.similarity.getValue();
this.indexOptions = builder.indexOptions.getValue();
this.norms = builder.norms.getValue();
this.termVectors = builder.termVectors.getValue();
this.eagerGlobalOrdinals = builder.eagerGlobalOrdinals.getValue();
this.indexPrefixes = builder.indexPrefixes.getValue();
this.freqFilter = builder.freqFilter.getValue();
this.fieldData = builder.fieldData.get();
this.indexPhrases = builder.indexPhrases.getValue();
}

@Override
public FieldMapper.Builder getMergeBuilder() {
return new Builder(simpleName(), builder.indexCreatedVersion, builder.analyzers.indexAnalyzers).init(this);
return new Builder(simpleName(), indexCreatedVersion, indexAnalyzers).init(this);
}

@Override
Expand Down Expand Up @@ -1003,23 +1030,24 @@ protected void doXContentBody(XContentBuilder builder, Params params) throws IOE
// this is a pain, but we have to do this to maintain BWC
boolean includeDefaults = params.paramAsBoolean("include_defaults", false);
builder.field("type", contentType());
this.builder.index.toXContent(builder, includeDefaults);
this.builder.store.toXContent(builder, includeDefaults);
final Builder b = (Builder) getMergeBuilder();
Copy link
Member Author

Choose a reason for hiding this comment

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

Not the nicest approach to this I guess, but it should create the correct serialization as far as I understand?

b.index.toXContent(builder, includeDefaults);
b.store.toXContent(builder, includeDefaults);
this.multiFields.toXContent(builder, params);
this.copyTo.toXContent(builder, params);
this.builder.meta.toXContent(builder, includeDefaults);
this.builder.indexOptions.toXContent(builder, includeDefaults);
this.builder.termVectors.toXContent(builder, includeDefaults);
this.builder.norms.toXContent(builder, includeDefaults);
this.builder.analyzers.indexAnalyzer.toXContent(builder, includeDefaults);
this.builder.analyzers.searchAnalyzer.toXContent(builder, includeDefaults);
this.builder.analyzers.searchQuoteAnalyzer.toXContent(builder, includeDefaults);
this.builder.similarity.toXContent(builder, includeDefaults);
this.builder.eagerGlobalOrdinals.toXContent(builder, includeDefaults);
this.builder.analyzers.positionIncrementGap.toXContent(builder, includeDefaults);
this.builder.fieldData.toXContent(builder, includeDefaults);
this.builder.freqFilter.toXContent(builder, includeDefaults);
this.builder.indexPrefixes.toXContent(builder, includeDefaults);
this.builder.indexPhrases.toXContent(builder, includeDefaults);
b.meta.toXContent(builder, includeDefaults);
b.indexOptions.toXContent(builder, includeDefaults);
b.termVectors.toXContent(builder, includeDefaults);
b.norms.toXContent(builder, includeDefaults);
b.analyzers.indexAnalyzer.toXContent(builder, includeDefaults);
b.analyzers.searchAnalyzer.toXContent(builder, includeDefaults);
b.analyzers.searchQuoteAnalyzer.toXContent(builder, includeDefaults);
b.similarity.toXContent(builder, includeDefaults);
b.eagerGlobalOrdinals.toXContent(builder, includeDefaults);
b.analyzers.positionIncrementGap.toXContent(builder, includeDefaults);
b.fieldData.toXContent(builder, includeDefaults);
b.freqFilter.toXContent(builder, includeDefaults);
b.indexPrefixes.toXContent(builder, includeDefaults);
b.indexPhrases.toXContent(builder, includeDefaults);
}
}
7 changes: 4 additions & 3 deletions server/src/main/java/org/elasticsearch/index/mapper/TextParams.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,10 @@ public static final class Analyzers {
public final IndexAnalyzers indexAnalyzers;

public Analyzers(IndexAnalyzers indexAnalyzers,
Function<FieldMapper, Analyzers> analyzerInitFunction) {
Function<FieldMapper, NamedAnalyzer> analyzerInitFunction,
Function<FieldMapper, Integer> positionGapInitFunction) {
this.indexAnalyzer = Parameter.analyzerParam("analyzer", false,
m -> analyzerInitFunction.apply(m).indexAnalyzer.get(), indexAnalyzers::getDefaultIndexAnalyzer)
analyzerInitFunction, indexAnalyzers::getDefaultIndexAnalyzer)
.setSerializerCheck((id, ic, a) -> id || ic ||
Objects.equals(a, getSearchAnalyzer()) == false || Objects.equals(a, getSearchQuoteAnalyzer()) == false)
.addValidator(a -> a.checkAllowedInMode(AnalysisMode.INDEX_TIME));
Expand Down Expand Up @@ -68,7 +69,7 @@ public Analyzers(IndexAnalyzers indexAnalyzers,
})
.addValidator(a -> a.checkAllowedInMode(AnalysisMode.SEARCH_TIME));
this.positionIncrementGap = Parameter.intParam("position_increment_gap", false,
m -> analyzerInitFunction.apply(m).positionIncrementGap.get(), TextFieldMapper.Defaults.POSITION_INCREMENT_GAP)
positionGapInitFunction, TextFieldMapper.Defaults.POSITION_INCREMENT_GAP)
.addValidator(v -> {
if (v < 0) {
throw new MapperParsingException("[position_increment_gap] must be positive, got [" + v + "]");
Expand Down