From 9580e2d956611ef6bd2c610201d906e958e074b5 Mon Sep 17 00:00:00 2001 From: Luca Cavanna Date: Fri, 21 Jun 2024 21:40:53 +0200 Subject: [PATCH] Propagate mapper builder context flags across nested mapper builder context creation (#109963) MapperBuilderContext includes information about whether synthetic source is configured, we are in a data streams and there are dimensions. This info is currently not propagated across the constructor of NestedMapperBuilderContext, which means that the builder context for the nested type receives hardcoded (false) info. This commit fixes that. One symptom of this, due to not propagating the info about whether synthetic source is configured, is that numeric fields that have ignore_malformed set to true, mapped under a nested type, will cause shard failures whenever a document with ignored values is retrieved in the fetch phase --- docs/changelog/109963.yaml | 6 ++ .../search/aggregations/bucket/NestedIT.java | 44 ++++++++++++ .../index/mapper/NestedObjectMapper.java | 28 +++++++- .../index/mapper/NestedObjectMapperTests.java | 70 +++++++++++++++++++ .../index/mapper/MapperTestCase.java | 15 ++++ 5 files changed, 160 insertions(+), 3 deletions(-) create mode 100644 docs/changelog/109963.yaml diff --git a/docs/changelog/109963.yaml b/docs/changelog/109963.yaml new file mode 100644 index 0000000000000..1745d549582d4 --- /dev/null +++ b/docs/changelog/109963.yaml @@ -0,0 +1,6 @@ +pr: 109963 +summary: Propagate mapper builder context flags across nested mapper builder context + creation +area: Mapping +type: bug +issues: [] diff --git a/server/src/internalClusterTest/java/org/elasticsearch/search/aggregations/bucket/NestedIT.java b/server/src/internalClusterTest/java/org/elasticsearch/search/aggregations/bucket/NestedIT.java index 9a27b0d8f75a3..1cce1ab3b1c4c 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/search/aggregations/bucket/NestedIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/search/aggregations/bucket/NestedIT.java @@ -12,6 +12,7 @@ import org.elasticsearch.action.search.SearchRequestBuilder; import org.elasticsearch.index.query.InnerHitBuilder; import org.elasticsearch.rest.RestStatus; +import org.elasticsearch.search.SearchHit; import org.elasticsearch.search.aggregations.Aggregator.SubAggCollectionMode; import org.elasticsearch.search.aggregations.InternalAggregation; import org.elasticsearch.search.aggregations.bucket.filter.Filter; @@ -31,6 +32,7 @@ import java.util.ArrayList; import java.util.List; +import java.util.Map; import static org.elasticsearch.index.query.QueryBuilders.boolQuery; import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery; @@ -47,6 +49,7 @@ import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertFailures; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailuresAndResponse; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertResponse; import static org.elasticsearch.xcontent.XContentFactory.jsonBuilder; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; @@ -845,4 +848,45 @@ public void testExtractInnerHitBuildersWithDuplicatePath() throws Exception { containsString("[inner_hits] already contains an entry for key [property]") ); } + + public void testSyntheticSource() throws Exception { + assertAcked( + prepareCreate("synthetic").setMapping( + jsonBuilder().startObject() + .startObject("_source") + .field("mode", "synthetic") + .endObject() + .startObject("properties") + .startObject("nested") + .field("type", "nested") + .startObject("properties") + .startObject("number") + .field("type", "long") + .field("ignore_malformed", true) + .endObject() + .endObject() + .endObject() + .endObject() + .endObject() + ) + ); + ensureGreen("synthetic"); + + prepareIndex("synthetic").setId("1") + .setSource( + jsonBuilder().startObject().startArray("nested").startObject().field("number", "a").endObject().endArray().endObject() + ) + .get(); + refresh("synthetic"); + + assertResponse(client().prepareSearch("synthetic").addFetchField("_ignored"), searchResponse -> { + assertHitCount(searchResponse, 1); + assertEquals(1, searchResponse.getHits().getHits().length); + SearchHit searchHit = searchResponse.getHits().getAt(0); + assertEquals("nested.number", searchHit.getFields().get("_ignored").getValue()); + @SuppressWarnings("unchecked") + Map nested = (Map) searchHit.getSourceAsMap().get("nested"); + assertEquals("a", nested.get("number")); + }); + } } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/NestedObjectMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/NestedObjectMapper.java index 5c2880a4bf760..10ad421f98fe9 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/NestedObjectMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/NestedObjectMapper.java @@ -64,6 +64,9 @@ public NestedObjectMapper build(MapperBuilderContext context) { } NestedMapperBuilderContext nestedContext = new NestedMapperBuilderContext( context.buildFullName(name()), + context.isSourceSynthetic(), + context.isDataStream(), + context.parentObjectContainsDimensions(), parentIncludedInRoot, context.getDynamic(dynamic), context.getMergeReason() @@ -122,14 +125,30 @@ private static class NestedMapperBuilderContext extends MapperBuilderContext { final boolean parentIncludedInRoot; - NestedMapperBuilderContext(String path, boolean parentIncludedInRoot, Dynamic dynamic, MapperService.MergeReason mergeReason) { - super(path, false, false, false, dynamic, mergeReason); + NestedMapperBuilderContext( + String path, + boolean isSourceSynthetic, + boolean isDataStream, + boolean parentObjectContainsDimensions, + boolean parentIncludedInRoot, + Dynamic dynamic, + MapperService.MergeReason mergeReason + ) { + super(path, isSourceSynthetic, isDataStream, parentObjectContainsDimensions, dynamic, mergeReason); this.parentIncludedInRoot = parentIncludedInRoot; } @Override public MapperBuilderContext createChildContext(String name, Dynamic dynamic) { - return new NestedMapperBuilderContext(buildFullName(name), parentIncludedInRoot, getDynamic(dynamic), getMergeReason()); + return new NestedMapperBuilderContext( + buildFullName(name), + isSourceSynthetic(), + isDataStream(), + parentObjectContainsDimensions(), + parentIncludedInRoot, + getDynamic(dynamic), + getMergeReason() + ); } } @@ -285,6 +304,9 @@ protected MapperMergeContext createChildContext(MapperMergeContext mapperMergeCo return mapperMergeContext.createChildContext( new NestedMapperBuilderContext( mapperBuilderContext.buildFullName(name), + mapperBuilderContext.isSourceSynthetic(), + mapperBuilderContext.isDataStream(), + mapperBuilderContext.parentObjectContainsDimensions(), parentIncludedInRoot, mapperBuilderContext.getDynamic(dynamic), mapperBuilderContext.getMergeReason() diff --git a/server/src/test/java/org/elasticsearch/index/mapper/NestedObjectMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/NestedObjectMapperTests.java index 25e4ccdf4d3a9..6336b7dbff10d 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/NestedObjectMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/NestedObjectMapperTests.java @@ -1551,4 +1551,74 @@ private NestedObjectMapper createNestedObjectMapperWithAllParametersSet(CheckedC })); return (NestedObjectMapper) mapper.mapping().getRoot().getMapper("nested_object"); } + + public void testNestedMapperBuilderContextConstructor() { + boolean isSourceSynthetic = randomBoolean(); + boolean isDataStream = randomBoolean(); + boolean parentContainsDimensions = randomBoolean(); + MergeReason mergeReason = randomFrom(MergeReason.values()); + MapperBuilderContext mapperBuilderContext = MapperBuilderContext.root(isSourceSynthetic, isDataStream, mergeReason); + mapperBuilderContext = mapperBuilderContext.createChildContext("name", parentContainsDimensions, randomFrom(Dynamic.values())); + NestedObjectMapper.Builder builder = new NestedObjectMapper.Builder("name", IndexVersion.current(), query -> null); + builder.add(new Mapper.Builder("name") { + @Override + public Mapper build(MapperBuilderContext context) { + assertEquals(isSourceSynthetic, context.isSourceSynthetic()); + assertEquals(isDataStream, context.isDataStream()); + assertEquals(parentContainsDimensions, context.parentObjectContainsDimensions()); + return new MockFieldMapper("name"); + } + }); + NestedObjectMapper nestedObjectMapper = builder.build(mapperBuilderContext); + assertNotNull(nestedObjectMapper.getMapper("name")); + } + + public void testNestedMapperMergeContextRootConstructor() { + boolean isSourceSynthetic = randomBoolean(); + boolean isDataStream = randomBoolean(); + boolean parentContainsDimensions = randomBoolean(); + MergeReason mergeReason = randomFrom(MergeReason.values()); + { + MapperBuilderContext mapperBuilderContext = MapperBuilderContext.root(false, false, mergeReason); + NestedObjectMapper.Builder builder = new NestedObjectMapper.Builder("name", IndexVersion.current(), query -> null); + NestedObjectMapper nestedObjectMapper = builder.build(mapperBuilderContext); + MapperMergeContext mapperMergeContext = MapperMergeContext.root(isSourceSynthetic, isDataStream, mergeReason, randomLong()); + MapperMergeContext childMergeContext = nestedObjectMapper.createChildContext(mapperMergeContext, "name"); + MapperBuilderContext nestedBuilderContext = childMergeContext.getMapperBuilderContext(); + assertEquals(isSourceSynthetic, nestedBuilderContext.isSourceSynthetic()); + assertEquals(isDataStream, nestedBuilderContext.isDataStream()); + } + { + MapperBuilderContext mapperBuilderContext = MapperBuilderContext.root(isSourceSynthetic, isDataStream, mergeReason); + MapperMergeContext mapperMergeContext = MapperMergeContext.root(isSourceSynthetic, isDataStream, mergeReason, randomLong()); + MapperBuilderContext childMapperBuilderContext = mapperBuilderContext.createChildContext( + "name", + parentContainsDimensions, + randomFrom(Dynamic.values()) + ); + MapperMergeContext childMergeContext = mapperMergeContext.createChildContext(childMapperBuilderContext); + MapperBuilderContext nestedBuilderContext = childMergeContext.getMapperBuilderContext(); + assertEquals(isSourceSynthetic, nestedBuilderContext.isSourceSynthetic()); + assertEquals(isDataStream, nestedBuilderContext.isDataStream()); + assertEquals(parentContainsDimensions, nestedBuilderContext.parentObjectContainsDimensions()); + } + } + + public void testNestedMapperMergeContextFromConstructor() { + boolean isSourceSynthetic = randomBoolean(); + boolean isDataStream = randomBoolean(); + boolean parentContainsDimensions = randomBoolean(); + MergeReason mergeReason = randomFrom(MergeReason.values()); + MapperBuilderContext mapperBuilderContext = MapperBuilderContext.root(isSourceSynthetic, isDataStream, mergeReason); + mapperBuilderContext = mapperBuilderContext.createChildContext("name", parentContainsDimensions, randomFrom(Dynamic.values())); + NestedObjectMapper.Builder builder = new NestedObjectMapper.Builder("name", IndexVersion.current(), query -> null); + NestedObjectMapper nestedObjectMapper = builder.build(mapperBuilderContext); + + MapperMergeContext mapperMergeContext = MapperMergeContext.from(mapperBuilderContext, randomLong()); + MapperMergeContext childMergeContext = nestedObjectMapper.createChildContext(mapperMergeContext, "name"); + MapperBuilderContext nestedBuilderContext = childMergeContext.getMapperBuilderContext(); + assertEquals(isSourceSynthetic, nestedBuilderContext.isSourceSynthetic()); + assertEquals(isDataStream, nestedBuilderContext.isDataStream()); + assertEquals(parentContainsDimensions, nestedBuilderContext.parentObjectContainsDimensions()); + } } diff --git a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java index fa0f0e1b95f54..839e9735be751 100644 --- a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java @@ -1494,6 +1494,21 @@ public final void testSyntheticSourceInvalid() throws IOException { } } + public final void testSyntheticSourceInNestedObject() throws IOException { + boolean ignoreMalformed = supportsIgnoreMalformed() ? rarely() : false; + SyntheticSourceExample syntheticSourceExample = syntheticSourceSupport(ignoreMalformed).example(5); + DocumentMapper mapper = createDocumentMapper(syntheticSourceMapping(b -> { + b.startObject("obj").field("type", "nested").startObject("properties").startObject("field"); + syntheticSourceExample.mapping().accept(b); + b.endObject().endObject().endObject(); + })); + assertThat(syntheticSource(mapper, b -> { + b.startObject("obj"); + syntheticSourceExample.buildInput(b); + b.endObject(); + }), equalTo("{\"obj\":" + syntheticSourceExample.expected() + "}")); + } + @Override protected final T compileScript(Script script, ScriptContext context) { return ingestScriptSupport().compileScript(script, context);