Skip to content

Commit

Permalink
Remove builder instance from TextFieldMapper (opensearch-project#7159) (
Browse files Browse the repository at this point in the history 
opensearch-project#7519)

* Remove builder instance from TextFieldMapper
* Add test coverage for mapping params serialization


(cherry picked from commit dffd822)

Signed-off-by: Sooraj Sinha <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
  • Loading branch information
@opensearch-trigger-bot @github-actions
opensearch-trigger-bot[bot] and github-actions[bot] authored May 11, 2023
1 parent 05fa28b commit 336453e
Show file tree
Hide file tree
Showing 2 changed files with 96 additions and 36 deletions.
94 changes: 60 additions & 34 deletions server/src/main/java/org/opensearch/index/mapper/TextFieldMapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@

package org.opensearch.index.mapper;

import java.util.Optional;
import org.apache.lucene.analysis.Analyzer;
import org.apache.lucene.analysis.AnalyzerWrapper;
import org.apache.lucene.analysis.CachingTokenFilter;
Expand Down Expand Up @@ -143,8 +144,8 @@ public static class Defaults {
public static final int POSITION_INCREMENT_GAP = 100;
}

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

/**
Expand Down Expand Up @@ -280,44 +281,56 @@ public static class Builder extends ParametrizedFieldMapper.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 -> toType(m).mappedFieldType.isSearchable(), true);
private final Parameter<Boolean> store = Parameter.storeParam(m -> toType(m).fieldType.stored(), false);

final Parameter<SimilarityProvider> similarity = TextParams.similarity(m -> builder(m).similarity.getValue());
final Parameter<SimilarityProvider> similarity = TextParams.similarity(m -> toType(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 -> toType(m).indexOptions);
final Parameter<Boolean> norms = TextParams.norms(true, m -> toType(m).fieldType.omitNorms() == false);
final Parameter<String> termVectors = TextParams.termVectors(m -> toType(m).termVectors);

final Parameter<Integer> positionIncrementGap = Parameter.intParam(
"position_increment_gap",
false,
m -> builder(m).positionIncrementGap.getValue(),
m -> toType(m).positionIncrementGap,
POSITION_INCREMENT_GAP_USE_ANALYZER
);

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

final Parameter<Boolean> indexPhrases = Parameter.boolParam("index_phrases", false, m -> builder(m).indexPhrases.getValue(), false);
final Parameter<Boolean> indexPhrases = Parameter.boolParam(
"index_phrases",
false,
m -> ((TextFieldType) toType(m).mappedFieldType).indexPhrases,
false
);
final Parameter<PrefixConfig> indexPrefixes = new Parameter<>(
"index_prefixes",
false,
() -> null,
TextFieldMapper::parsePrefixConfig,
m -> builder(m).indexPrefixes.getValue()
m -> Optional.ofNullable(((TextFieldType) toType(m).mappedFieldType).prefixFieldType)
.map(p -> new PrefixConfig(p.minChars, p.maxChars))
.orElse(null)
).acceptsNull();

private final Parameter<Float> boost = Parameter.boostParam();
Expand Down Expand Up @@ -956,10 +969,16 @@ public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName, S

}

private final Builder builder;
private final FieldType fieldType;
private final PrefixFieldMapper prefixFieldMapper;
private final PhraseFieldMapper phraseFieldMapper;
private final SimilarityProvider similarity;
private final String indexOptions;
private final String termVectors;
private final int positionIncrementGap;
private final Version indexCreatedVersion;
private final IndexAnalyzers indexAnalyzers;
private final FielddataFrequencyFilter freqFilter;

protected TextFieldMapper(
String simpleName,
Expand All @@ -980,7 +999,13 @@ protected TextFieldMapper(
this.fieldType = fieldType;
this.prefixFieldMapper = prefixFieldMapper;
this.phraseFieldMapper = phraseFieldMapper;
this.builder = builder;
this.similarity = builder.similarity.getValue();
this.indexOptions = builder.indexOptions.getValue();
this.termVectors = builder.termVectors.getValue();
this.positionIncrementGap = builder.positionIncrementGap.getValue();
this.indexCreatedVersion = builder.indexCreatedVersion;
this.indexAnalyzers = builder.analyzers.indexAnalyzers;
this.freqFilter = builder.freqFilter.getValue();
}

@Override
Expand All @@ -990,7 +1015,7 @@ protected TextFieldMapper clone() {

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

@Override
Expand Down Expand Up @@ -1169,24 +1194,25 @@ public static Query createPhrasePrefixQuery(
protected void doXContentBody(XContentBuilder builder, boolean includeDefaults, Params params) throws IOException {
// this is a pain, but we have to do this to maintain BWC
builder.field("type", contentType());
this.builder.boost.toXContent(builder, includeDefaults);
this.builder.index.toXContent(builder, includeDefaults);
this.builder.store.toXContent(builder, includeDefaults);
Builder mapperBuilder = (TextFieldMapper.Builder) getMergeBuilder();
mapperBuilder.boost.toXContent(builder, includeDefaults);
mapperBuilder.index.toXContent(builder, includeDefaults);
mapperBuilder.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.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);
mapperBuilder.meta.toXContent(builder, includeDefaults);
mapperBuilder.indexOptions.toXContent(builder, includeDefaults);
mapperBuilder.termVectors.toXContent(builder, includeDefaults);
mapperBuilder.norms.toXContent(builder, includeDefaults);
mapperBuilder.analyzers.indexAnalyzer.toXContent(builder, includeDefaults);
mapperBuilder.analyzers.searchAnalyzer.toXContent(builder, includeDefaults);
mapperBuilder.analyzers.searchQuoteAnalyzer.toXContent(builder, includeDefaults);
mapperBuilder.similarity.toXContent(builder, includeDefaults);
mapperBuilder.eagerGlobalOrdinals.toXContent(builder, includeDefaults);
mapperBuilder.positionIncrementGap.toXContent(builder, includeDefaults);
mapperBuilder.fieldData.toXContent(builder, includeDefaults);
mapperBuilder.freqFilter.toXContent(builder, includeDefaults);
mapperBuilder.indexPrefixes.toXContent(builder, includeDefaults);
mapperBuilder.indexPhrases.toXContent(builder, includeDefaults);
}
}
38 changes: 36 additions & 2 deletions server/src/test/java/org/opensearch/index/mapper/TextFieldMapperTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -270,10 +270,44 @@ public void testBWCSerialization() throws IOException {
b.startObject("subfield").field("type", "long").endObject();
}
b.endObject();
b.field("store", true);
b.field("similarity", "BM25");
b.field("index_options", "offsets");
b.field("norms", false);
b.field("term_vector", "yes");
b.field("position_increment_gap", 0);
b.startObject("fielddata_frequency_filter");
{
b.field("min", 0.001);
b.field("max", 0.1);
b.field("min_segment_size", 500);
}
b.endObject();
b.field("eager_global_ordinals", true);
b.field("index_phrases", true);
b.startObject("index_prefixes");
{
b.field("min_chars", 1);
b.field("max_chars", 10);
}
b.endObject();
b.startObject("meta");
{
b.field("unit", "min");
}
b.endObject();
b.startArray("copy_to");
{
b.value("target");
}
b.endArray();
}));

assertEquals(
"{\"_doc\":{\"properties\":{\"field\":{\"type\":\"text\",\"fields\":{\"subfield\":{\"type\":\"long\"}},\"fielddata\":true}}}}",
"{\"_doc\":{\"properties\":{\"field\":{\"type\":\"text\",\"store\":true,\"fields\":{\"subfield\":{\"type\":\"long\"}},"
+ "\"copy_to\":[\"target\"],\"meta\":{\"unit\":\"min\"},\"index_options\":\"offsets\",\"term_vector\":\"yes\",\"norms\":false,"
+ "\"similarity\":\"BM25\",\"eager_global_ordinals\":true,\"position_increment_gap\":0,"
+ "\"fielddata\":true,\"fielddata_frequency_filter\":{\"min\":0.001,\"max\":0.1,\"min_segment_size\":500},"
+ "\"index_prefixes\":{\"min_chars\":1,\"max_chars\":10},\"index_phrases\":true}}}}",
Strings.toString(XContentType.JSON, mapperService.documentMapper())
);
}
Expand Down

0 comments on commit 336453e

Please sign in to comment.