Skip to content

Commit

Permalink
Report synthetic source status in MapperBuilderContext (#91400)
Browse files Browse the repository at this point in the history
We currently work out whether or not a mapper should be storing additional
values for synthetic source by looking at the DocumentParserContext. However,
this value does not change for the lifetime of the mapper - it is defined by
metadata on the root mapper and is immutable - and DocumentParserContext
feels like the wrong place for this information as it holds context specific
to the document being parsed.

This commit moves synthetic source status information from DocumentParserContext
to MapperBuilderContext instead. Mappers which need this information retrieve
it at build time and hold it on final fields.
  • Loading branch information
romseygeek authored Nov 8, 2022
1 parent fef70ad commit 41ab45a
Show file tree
Hide file tree
Showing 64 changed files with 245 additions and 166 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ private List<String> findRoutingPaths(String indexName, Settings allSettings, Li
MappingParserContext parserContext = mapperService.parserContext();
var mapper = parserContext.typeParser(mappingSnippetType)
.parse(template.pathMatch(), mappingSnippet, parserContext)
.build(MapperBuilderContext.ROOT);
.build(MapperBuilderContext.root(false));
extractPath(routingPaths, mapper);
}
return routingPaths;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ public void setup() throws Exception {
ScriptCompiler.NONE,
false,
Version.CURRENT
).build(MapperBuilderContext.ROOT);
).build(MapperBuilderContext.root(false));
RootObjectMapper.Builder root = new RootObjectMapper.Builder("_doc", ObjectMapper.Defaults.SUBOBJECTS);
root.add(
new DateFieldMapper.Builder(
Expand All @@ -233,7 +233,7 @@ public void setup() throws Exception {
);
MetadataFieldMapper dtfm = DataStreamTestHelper.getDataStreamTimestampFieldMapper();
Mapping mapping = new Mapping(
root.build(MapperBuilderContext.ROOT),
root.build(MapperBuilderContext.root(false)),
new MetadataFieldMapper[] { dtfm },
Collections.emptyMap()
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ public void testParse3DPolygon() throws IOException, ParseException {
Polygon expected = GEOMETRY_FACTORY.createPolygon(shell, null);
final Version version = VersionUtils.randomPreviousCompatibleVersion(random(), Version.V_8_0_0);
final LegacyGeoShapeFieldMapper mapperBuilder = new LegacyGeoShapeFieldMapper.Builder("test", version, false, true).build(
MapperBuilderContext.ROOT
MapperBuilderContext.root(false)
);
try (XContentParser parser = createParser(polygonGeoJson)) {
parser.nextToken();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ public void testParseMixedDimensionPolyWithHole() throws IOException, ParseExcep
parser.nextToken();

final LegacyGeoShapeFieldMapper mapperBuilder = new LegacyGeoShapeFieldMapper.Builder("test", Version.CURRENT, false, true).build(
MapperBuilderContext.ROOT
MapperBuilderContext.root(false)
);

// test store z disabled
Expand Down Expand Up @@ -326,7 +326,7 @@ public void testParseMixedDimensionPolyWithHoleStoredZ() throws IOException {

final Version version = VersionUtils.randomPreviousCompatibleVersion(random(), Version.V_8_0_0);
final LegacyGeoShapeFieldMapper mapperBuilder = new LegacyGeoShapeFieldMapper.Builder("test", version, false, true).build(
MapperBuilderContext.ROOT
MapperBuilderContext.root(false)
);

// test store z disabled
Expand All @@ -350,7 +350,7 @@ public void testParsePolyWithStoredZ() throws IOException {

final Version version = VersionUtils.randomPreviousCompatibleVersion(random(), Version.V_8_0_0);
final LegacyGeoShapeFieldMapper mapperBuilder = new LegacyGeoShapeFieldMapper.Builder("test", version, false, true).build(
MapperBuilderContext.ROOT
MapperBuilderContext.root(false)
);

ShapeBuilder<?, ?, ?> shapeBuilder = ShapeParser.parse(parser, mapperBuilder);
Expand All @@ -367,7 +367,7 @@ public void testParseOpenPolygon() throws IOException {
final Version version = VersionUtils.randomPreviousCompatibleVersion(random(), Version.V_8_0_0);
final LegacyGeoShapeFieldMapper defaultMapperBuilder = new LegacyGeoShapeFieldMapper.Builder("test", version, false, true).coerce(
false
).build(MapperBuilderContext.ROOT);
).build(MapperBuilderContext.root(false));
ElasticsearchParseException exception = expectThrows(
ElasticsearchParseException.class,
() -> ShapeParser.parse(parser, defaultMapperBuilder)
Expand All @@ -376,7 +376,7 @@ public void testParseOpenPolygon() throws IOException {

final LegacyGeoShapeFieldMapper coercingMapperBuilder = new LegacyGeoShapeFieldMapper.Builder("test", Version.CURRENT, false, true)
.coerce(true)
.build(MapperBuilderContext.ROOT);
.build(MapperBuilderContext.root(false));
ShapeBuilder<?, ?, ?> shapeBuilder = ShapeParser.parse(parser, coercingMapperBuilder);
assertNotNull(shapeBuilder);
assertEquals("polygon ((100.0 5.0, 100.0 10.0, 90.0 10.0, 90.0 5.0, 100.0 5.0))", shapeBuilder.toWKT());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,9 @@ public void testSetStrategyName() {

public void testFetchSourceValue() throws IOException {
Version version = VersionUtils.randomPreviousCompatibleVersion(random(), Version.V_8_0_0);
MappedFieldType mapper = new LegacyGeoShapeFieldMapper.Builder("field", version, false, true).build(MapperBuilderContext.ROOT)
.fieldType();
MappedFieldType mapper = new LegacyGeoShapeFieldMapper.Builder("field", version, false, true).build(
MapperBuilderContext.root(false)
).fieldType();

Map<String, Object> jsonLineString = Map.of("type", "LineString", "coordinates", List.of(List.of(42.0, 27.1), List.of(30.0, 50.0)));
Map<String, Object> jsonPoint = Map.of("type", "Point", "coordinates", List.of(14.0, 15.0));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,15 @@ private MatchOnlyTextFieldType buildFieldType(MapperBuilderContext context) {
public MatchOnlyTextFieldMapper build(MapperBuilderContext context) {
MatchOnlyTextFieldType tft = buildFieldType(context);
MultiFields multiFields = multiFieldsBuilder.build(this, context);
return new MatchOnlyTextFieldMapper(name, Defaults.FIELD_TYPE, tft, multiFields, copyTo.build(), this);
return new MatchOnlyTextFieldMapper(
name,
Defaults.FIELD_TYPE,
tft,
multiFields,
copyTo.build(),
context.isSourceSynthetic(),
this
);
}
}

Expand Down Expand Up @@ -319,6 +327,7 @@ private String storedFieldNameForSyntheticSource() {
private final IndexAnalyzers indexAnalyzers;
private final NamedAnalyzer indexAnalyzer;
private final int positionIncrementGap;
private final boolean storeSource;
private final FieldType fieldType;

private MatchOnlyTextFieldMapper(
Expand All @@ -327,6 +336,7 @@ private MatchOnlyTextFieldMapper(
MatchOnlyTextFieldType mappedFieldType,
MultiFields multiFields,
CopyTo copyTo,
boolean storeSource,
Builder builder
) {
super(simpleName, mappedFieldType, multiFields, copyTo, false, null);
Expand All @@ -337,6 +347,7 @@ private MatchOnlyTextFieldMapper(
this.indexAnalyzers = builder.analyzers.indexAnalyzers;
this.indexAnalyzer = builder.analyzers.getIndexAnalyzer();
this.positionIncrementGap = builder.analyzers.positionIncrementGap.getValue();
this.storeSource = storeSource;
}

@Override
Expand All @@ -361,7 +372,7 @@ protected void parseCreateField(DocumentParserContext context) throws IOExceptio
context.doc().add(field);
context.addToFieldNames(fieldType().name());

if (context.isSyntheticSource()) {
if (storeSource) {
context.doc().add(new StoredField(fieldType().storedFieldNameForSyntheticSource(), value));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public void testIsNotAggregatable() {
}

public void testFetchSourceValue() throws IOException {
MappedFieldType mapper = new RankFeatureFieldMapper.Builder("field").build(MapperBuilderContext.ROOT).fieldType();
MappedFieldType mapper = new RankFeatureFieldMapper.Builder("field").build(MapperBuilderContext.root(false)).fieldType();

assertEquals(List.of(3.14f), fetchSourceValue(mapper, 3.14));
assertEquals(List.of(42.9f), fetchSourceValue(mapper, "42.9"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,15 +217,15 @@ public void testFieldData() throws IOException {

public void testFetchSourceValue() throws IOException {
MappedFieldType mapper = new ScaledFloatFieldMapper.Builder("field", false, false).scalingFactor(100)
.build(MapperBuilderContext.ROOT)
.build(MapperBuilderContext.root(false))
.fieldType();
assertEquals(List.of(3.14), fetchSourceValue(mapper, 3.1415926));
assertEquals(List.of(3.14), fetchSourceValue(mapper, "3.1415"));
assertEquals(List.of(), fetchSourceValue(mapper, ""));

MappedFieldType nullValueMapper = new ScaledFloatFieldMapper.Builder("field", false, false).scalingFactor(100)
.nullValue(2.71)
.build(MapperBuilderContext.ROOT)
.build(MapperBuilderContext.root(false))
.fieldType();
assertEquals(List.of(2.71), fetchSourceValue(nullValueMapper, ""));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ static MappedFieldType[] withJoinFields(MappedFieldType... fieldTypes) {

int i = fieldTypes.length;
result[i++] = new ParentJoinFieldMapper.Builder("join_field").addRelation(PARENT_TYPE, Collections.singleton(CHILD_TYPE))
.build(MapperBuilderContext.ROOT)
.build(MapperBuilderContext.root(false))
.fieldType();
result[i++] = new ParentIdFieldMapper.ParentIdFieldType("join_field#" + PARENT_TYPE, false);
assert i == result.length;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
public class JoinFieldTypeTests extends FieldTypeTestCase {

public void testFetchSourceValue() throws IOException {
MappedFieldType fieldType = new ParentJoinFieldMapper.Builder("field").build(MapperBuilderContext.ROOT).fieldType();
MappedFieldType fieldType = new ParentJoinFieldMapper.Builder("field").build(MapperBuilderContext.root(false)).fieldType();

Map<String, String> parentValue = Map.of("relation", "parent");
assertEquals(List.of(parentValue), fetchSourceValue(fieldType, parentValue));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public void testStoringQueryBuilders() throws IOException {
TermQueryBuilder[] queryBuilders = new TermQueryBuilder[randomIntBetween(1, 16)];
IndexWriterConfig config = new IndexWriterConfig(new WhitespaceAnalyzer());
config.setMergePolicy(NoMergePolicy.INSTANCE);
BinaryFieldMapper fieldMapper = PercolatorFieldMapper.Builder.createQueryBuilderFieldBuilder(MapperBuilderContext.ROOT);
BinaryFieldMapper fieldMapper = PercolatorFieldMapper.Builder.createQueryBuilderFieldBuilder(MapperBuilderContext.root(false));
MappedFieldType.FielddataOperation fielddataOperation = MappedFieldType.FielddataOperation.SEARCH;

Version version = Version.CURRENT;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,18 @@ public class ICUCollationKeywordFieldTypeTests extends FieldTypeTestCase {

public void testFetchSourceValue() throws IOException {

ICUCollationKeywordFieldMapper mapper = new ICUCollationKeywordFieldMapper.Builder("field").build(MapperBuilderContext.ROOT);
ICUCollationKeywordFieldMapper mapper = new ICUCollationKeywordFieldMapper.Builder("field").build(MapperBuilderContext.root(false));
assertEquals(List.of("42"), fetchSourceValue(mapper.fieldType(), 42L));
assertEquals(List.of("true"), fetchSourceValue(mapper.fieldType(), true));

ICUCollationKeywordFieldMapper ignoreAboveMapper = new ICUCollationKeywordFieldMapper.Builder("field").ignoreAbove(4)
.build(MapperBuilderContext.ROOT);
.build(MapperBuilderContext.root(false));
assertEquals(List.of(), fetchSourceValue(ignoreAboveMapper.fieldType(), "value"));
assertEquals(List.of("42"), fetchSourceValue(ignoreAboveMapper.fieldType(), 42L));
assertEquals(List.of("true"), fetchSourceValue(ignoreAboveMapper.fieldType(), true));

ICUCollationKeywordFieldMapper nullValueMapper = new ICUCollationKeywordFieldMapper.Builder("field").nullValue("NULL")
.build(MapperBuilderContext.ROOT);
.build(MapperBuilderContext.root(false));
assertEquals(List.of("NULL"), fetchSourceValue(nullValueMapper.fieldType(), null));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public void testIntervals() throws IOException {

public void testFetchSourceValue() throws IOException {
MappedFieldType fieldType = new AnnotatedTextFieldMapper.Builder("field", Version.CURRENT, createDefaultIndexAnalyzers()).build(
MapperBuilderContext.ROOT
MapperBuilderContext.root(false)
).fieldType();

assertEquals(List.of("value"), fetchSourceValue(fieldType, "value"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public class DocumentMapper {
*/
public static DocumentMapper createEmpty(MapperService mapperService) {
RootObjectMapper root = new RootObjectMapper.Builder(MapperService.SINGLE_MAPPING_NAME, ObjectMapper.Defaults.SUBOBJECTS).build(
MapperBuilderContext.ROOT
MapperBuilderContext.root(false)
);
MetadataFieldMapper[] metadata = mapperService.getMetadataMappers().values().toArray(new MetadataFieldMapper[0]);
Mapping mapping = new Mapping(root, metadata, null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ static Mapping createDynamicUpdate(DocumentParserContext context) {
for (RuntimeField runtimeField : context.getDynamicRuntimeFields()) {
rootBuilder.addRuntimeField(runtimeField);
}
RootObjectMapper root = rootBuilder.build(MapperBuilderContext.ROOT);
RootObjectMapper root = rootBuilder.build(MapperBuilderContext.root(context.mappingLookup().isSourceSynthetic()));
root.fixRedundantIncludes();
return context.mappingLookup().getMapping().mappingUpdate(root);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -400,12 +400,15 @@ public DocumentDimensions getDimensions() {

public abstract ContentPath path();

public final MapperBuilderContext createMapperBuilderContext() {
/**
* Creates a context to build dynamic mappers
*/
public final MapperBuilderContext createDynamicMapperBuilderContext() {
String p = path().pathAsText("");
if (p.endsWith(".")) {
p = p.substring(0, p.length() - 1);
}
return new MapperBuilderContext(p);
return new MapperBuilderContext(p, mappingLookup().isSourceSynthetic());
}

public abstract XContentParser parser();
Expand Down Expand Up @@ -440,14 +443,6 @@ public final DynamicTemplate findDynamicTemplate(String fieldName, DynamicTempla
return null;
}

/**
* Is this index configured to use synthetic source?
*/
public final boolean isSyntheticSource() {
SourceFieldMapper sft = mappingLookup.getMapping().getMetadataMapperByClass(SourceFieldMapper.class);
return sft == null ? false : sft.isSynthetic();
}

// XContentParser that wraps an existing parser positioned on a value,
// and a field name, and returns a stream that looks like { 'field' : 'value' }
private static class CopyToParser extends FilterXContentParserWrapper {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,15 +162,15 @@ static Mapper createDynamicObjectMapper(DocumentParserContext context, String na
return mapper != null
? mapper
: new ObjectMapper.Builder(name, ObjectMapper.Defaults.SUBOBJECTS).enabled(ObjectMapper.Defaults.ENABLED)
.build(context.createMapperBuilderContext());
.build(context.createDynamicMapperBuilderContext());
}

/**
* Returns a dynamically created object mapper, based exclusively on a matching dynamic template, null otherwise.
*/
static Mapper createObjectMapperFromTemplate(DocumentParserContext context, String name) {
Mapper.Builder templateBuilder = findTemplateBuilderForObject(context, name);
return templateBuilder == null ? null : templateBuilder.build(context.createMapperBuilderContext());
return templateBuilder == null ? null : templateBuilder.build(context.createDynamicMapperBuilderContext());
}

/**
Expand Down Expand Up @@ -305,7 +305,7 @@ private static final class Concrete implements Strategy {
}

void createDynamicField(Mapper.Builder builder, DocumentParserContext context) throws IOException {
Mapper mapper = builder.build(context.createMapperBuilderContext());
Mapper mapper = builder.build(context.createDynamicMapperBuilderContext());
context.addDynamicMapper(mapper);
parseField.accept(context, mapper);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ public IpFieldMapper build(MapperBuilderContext context) {
),
multiFieldsBuilder.build(this, context),
copyTo.build(),
context.isSourceSynthetic(),
this
);
}
Expand Down Expand Up @@ -428,6 +429,7 @@ public boolean isDimension() {
private final boolean hasDocValues;
private final boolean stored;
private final boolean ignoreMalformed;
private final boolean storeIgnored;
private final boolean dimension;

private final InetAddress nullValue;
Expand All @@ -440,7 +442,14 @@ public boolean isDimension() {
private final FieldValues<InetAddress> scriptValues;
private final ScriptCompiler scriptCompiler;

private IpFieldMapper(String simpleName, MappedFieldType mappedFieldType, MultiFields multiFields, CopyTo copyTo, Builder builder) {
private IpFieldMapper(
String simpleName,
MappedFieldType mappedFieldType,
MultiFields multiFields,
CopyTo copyTo,
boolean storeIgnored,
Builder builder
) {
super(simpleName, mappedFieldType, multiFields, copyTo, builder.script.get() != null, builder.onScriptError.get());
this.ignoreMalformedByDefault = builder.ignoreMalformedByDefault;
this.indexed = builder.indexed.getValue();
Expand All @@ -454,6 +463,7 @@ private IpFieldMapper(String simpleName, MappedFieldType mappedFieldType, MultiF
this.scriptValues = builder.scriptValues();
this.scriptCompiler = builder.scriptCompiler;
this.dimension = builder.dimension.getValue();
this.storeIgnored = storeIgnored;
}

@Override
Expand All @@ -479,7 +489,7 @@ protected void parseCreateField(DocumentParserContext context) throws IOExceptio
} catch (IllegalArgumentException e) {
if (ignoreMalformed) {
context.addIgnoredField(fieldType().name());
if (context.isSyntheticSource()) {
if (storeIgnored) {
// Save a copy of the field so synthetic source can load it
context.doc().add(IgnoreMalformedStoredValues.storedField(name(), context.parser()));
}
Expand Down
Loading

0 comments on commit 41ab45a

Please sign in to comment.