Skip to content

Commit

Permalink
Propagate mapper builder context flags across nested mapper builder c…
Browse files Browse the repository at this point in the history
…ontext creation (elastic#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
  • Loading branch information
javanna committed Jun 24, 2024
1 parent da1ba77 commit 9580e2d
Show file tree
Hide file tree
Showing 5 changed files with 160 additions and 3 deletions.
6 changes: 6 additions & 0 deletions docs/changelog/109963.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pr: 109963
summary: Propagate mapper builder context flags across nested mapper builder context
creation
area: Mapping
type: bug
issues: []
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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<String, ?> nested = (Map<String, ?>) searchHit.getSourceAsMap().get("nested");
assertEquals("a", nested.get("number"));
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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()
);
}
}

Expand Down Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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> T compileScript(Script script, ScriptContext<T> context) {
return ingestScriptSupport().compileScript(script, context);
Expand Down

0 comments on commit 9580e2d

Please sign in to comment.