Skip to content

Commit

Permalink
Remove special handling for objects and arrays with dynamic overrid…
Browse files Browse the repository at this point in the history 
…es (elastic#113924) (elastic#113959)

* Remove special handling for objects and arrays with `dynamic` overrides

* add test

* add test

(cherry picked from commit c840ea3)
  • Loading branch information
@kkrik-es
kkrik-es authored Oct 2, 2024
1 parent 9457713 commit e5bebfe
Show file tree
Hide file tree
Showing 3 changed files with 127 additions and 24 deletions.
22 changes: 15 additions & 7 deletions ...spec/src/yamlRestTest/resources/rest-api-spec/test/indices.create/20_synthetic_source.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -535,6 +535,8 @@ object array in object with dynamic override:
_source:
mode: synthetic
properties:
id:
type: integer
path_no:
dynamic: false
properties:
Expand All @@ -552,19 +554,25 @@ object array in object with dynamic override:
refresh: true
body:
- '{ "create": { } }'
- '{ "path_no": [ { "some_int": 10 }, {"name": "foo"} ], "path_runtime": [ { "some_int": 20 }, {"name": "bar"} ], "name": "baz" }'
- '{ "id": 1, "path_no": [ { "some_int": 30 }, {"name": "baz"}, { "some_int": 20 }, {"name": "bar"} ], "name": "A" }'
- '{ "create": { } }'
- '{ "id": 2, "path_runtime": [ { "some_int": 30 }, {"name": "baz"}, { "some_int": 20 }, {"name": "bar"} ], "name": "B" }'
- match: { errors: false }

- do:
search:
index: test
sort: id

- match: { hits.total.value: 1 }
- match: { hits.hits.0._source.name: baz }
- match: { hits.hits.0._source.path_no.0.some_int: 10 }
- match: { hits.hits.0._source.path_no.1.name: foo }
- match: { hits.hits.0._source.path_runtime.0.some_int: 20 }
- match: { hits.hits.0._source.path_runtime.1.name: bar }
- match: { hits.hits.0._source.id: 1 }
- match: { hits.hits.0._source.name: A }
- match: { hits.hits.0._source.path_no.some_int: [ 30, 20 ] }
- match: { hits.hits.0._source.path_no.name: [ bar, baz ] }

- match: { hits.hits.1._source.id: 2 }
- match: { hits.hits.1._source.name: B }
- match: { hits.hits.1._source.path_runtime.some_int: [ 30, 20 ] }
- match: { hits.hits.1._source.path_runtime.name: [ bar, baz ] }


---
Expand Down
22 changes: 10 additions & 12 deletions server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ static void parseObjectOrNested(DocumentParserContext context) throws IOExceptio
context.addIgnoredField(
new IgnoredSourceFieldMapper.NameValue(
context.parent().fullPath(),
context.parent().fullPath().lastIndexOf(currentFieldName),
context.parent().fullPath().lastIndexOf(context.parent().leafName()),
XContentDataHelper.encodeToken(parser),
context.doc()
)
Expand Down Expand Up @@ -803,27 +803,25 @@ private static void parseNonDynamicArray(
boolean objectRequiresStoringSource = mapper instanceof ObjectMapper objectMapper
&& (objectMapper.storeArraySource()
|| (context.sourceKeepModeFromIndexSettings() == Mapper.SourceKeepMode.ARRAYS
&& objectMapper instanceof NestedObjectMapper == false)
|| objectMapper.dynamic == ObjectMapper.Dynamic.RUNTIME);
&& objectMapper instanceof NestedObjectMapper == false));
boolean fieldWithFallbackSyntheticSource = mapper instanceof FieldMapper fieldMapper
&& fieldMapper.syntheticSourceMode() == FieldMapper.SyntheticSourceMode.FALLBACK;
boolean fieldWithStoredArraySource = mapper instanceof FieldMapper fieldMapper
&& getSourceKeepMode(context, fieldMapper.sourceKeepMode()) != Mapper.SourceKeepMode.NONE;
boolean dynamicRuntimeContext = context.dynamic() == ObjectMapper.Dynamic.RUNTIME;
boolean copyToFieldHasValuesInDocument = context.isWithinCopyTo() == false && context.isCopyToDestinationField(fullPath);
if (objectRequiresStoringSource
|| fieldWithFallbackSyntheticSource
|| dynamicRuntimeContext
|| fieldWithStoredArraySource
|| copyToFieldHasValuesInDocument) {
context = context.addIgnoredFieldFromContext(IgnoredSourceFieldMapper.NameValue.fromContext(context, fullPath, null));
} else if (mapper instanceof ObjectMapper objectMapper
&& (objectMapper.isEnabled() == false || objectMapper.dynamic == ObjectMapper.Dynamic.FALSE)) {
context.addIgnoredField(
IgnoredSourceFieldMapper.NameValue.fromContext(context, fullPath, XContentDataHelper.encodeToken(context.parser()))
);
return;
}
} else if (mapper instanceof ObjectMapper objectMapper && (objectMapper.isEnabled() == false)) {
// No need to call #addIgnoredFieldFromContext as both singleton and array instances of this object
// get tracked through ignored source.
context.addIgnoredField(
IgnoredSourceFieldMapper.NameValue.fromContext(context, fullPath, XContentDataHelper.encodeToken(context.parser()))
);
return;
}
}

// In synthetic source, if any array element requires storing its source as-is, it takes precedence over
Expand Down
107 changes: 102 additions & 5 deletions server/src/test/java/org/elasticsearch/index/mapper/IgnoredSourceFieldMapperTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -902,7 +902,6 @@ public void testObjectArrayAndValue() throws IOException {
}
b.endObject();
})).documentMapper();
// { "path": [ { "stored":[ { "leaf": 10 } ] }, { "stored": { "leaf": 20 } } ] }
var syntheticSource = syntheticSource(documentMapper, b -> {
b.startArray("path");
{
Expand All @@ -927,6 +926,91 @@ public void testObjectArrayAndValue() throws IOException {
{"path":{"stored":[{"leaf":10},{"leaf":20}]}}""", syntheticSource);
}

public void testObjectArrayAndValueDisabledObject() throws IOException {
DocumentMapper documentMapper = createMapperService(syntheticSourceMapping(b -> {
b.startObject("path").field("type", "object").startObject("properties");
{
b.startObject("regular");
{
b.startObject("properties").startObject("leaf").field("type", "integer").endObject().endObject();
}
b.endObject();
b.startObject("disabled").field("type", "object").field("enabled", false);
{
b.startObject("properties").startObject("leaf").field("type", "integer").endObject().endObject();
}
b.endObject();
}
b.endObject().endObject();
})).documentMapper();
var syntheticSource = syntheticSource(documentMapper, b -> {
b.startArray("path");
{
b.startObject().startArray("disabled").startObject().field("leaf", 10).endObject().endArray().endObject();
b.startObject().startObject("disabled").field("leaf", 20).endObject().endObject();
b.startObject().startArray("regular").startObject().field("leaf", 10).endObject().endArray().endObject();
b.startObject().startObject("regular").field("leaf", 20).endObject().endObject();
}
b.endArray();
});
assertEquals("""
{"path":{"disabled":[{"leaf":10},{"leaf":20}],"regular":{"leaf":[10,20]}}}""", syntheticSource);
}

public void testObjectArrayAndValueNonDynamicObject() throws IOException {
DocumentMapper documentMapper = createMapperService(syntheticSourceMapping(b -> {
b.startObject("path").field("type", "object").startObject("properties");
{
b.startObject("regular");
{
b.startObject("properties").startObject("leaf").field("type", "integer").endObject().endObject();
}
b.endObject();
b.startObject("disabled").field("type", "object").field("dynamic", "false").endObject();
}
b.endObject().endObject();
})).documentMapper();
var syntheticSource = syntheticSource(documentMapper, b -> {
b.startArray("path");
{
b.startObject().startArray("disabled").startObject().field("leaf", 10).endObject().endArray().endObject();
b.startObject().startObject("disabled").field("leaf", 20).endObject().endObject();
b.startObject().startArray("regular").startObject().field("leaf", 10).endObject().endArray().endObject();
b.startObject().startObject("regular").field("leaf", 20).endObject().endObject();
}
b.endArray();
});
assertEquals("""
{"path":{"disabled":{"leaf":[10,20]},"regular":{"leaf":[10,20]}}}""", syntheticSource);
}

public void testObjectArrayAndValueDynamicRuntimeObject() throws IOException {
DocumentMapper documentMapper = createMapperService(syntheticSourceMapping(b -> {
b.startObject("path").field("type", "object").startObject("properties");
{
b.startObject("regular");
{
b.startObject("properties").startObject("leaf").field("type", "integer").endObject().endObject();
}
b.endObject();
b.startObject("runtime").field("type", "object").field("dynamic", "runtime").endObject();
}
b.endObject().endObject();
})).documentMapper();
var syntheticSource = syntheticSource(documentMapper, b -> {
b.startArray("path");
{
b.startObject().startArray("runtime").startObject().field("leaf", 10).endObject().endArray().endObject();
b.startObject().startObject("runtime").field("leaf", 20).endObject().endObject();
b.startObject().startArray("regular").startObject().field("leaf", 10).endObject().endArray().endObject();
b.startObject().startObject("regular").field("leaf", 20).endObject().endObject();
}
b.endArray();
});
assertEquals("""
{"path":{"regular":{"leaf":[10,20]},"runtime":{"leaf":[10,20]}}}""", syntheticSource);
}

public void testDisabledObjectWithinHigherLevelArray() throws IOException {
DocumentMapper documentMapper = createMapperService(syntheticSourceMapping(b -> {
b.startObject("path");
Expand Down Expand Up @@ -1337,7 +1421,7 @@ public void testNoDynamicObjectSimpleArray() throws IOException {
b.endArray();
});
assertEquals("""
{"path":[{"name":"foo"},{"name":"bar"}]}""", syntheticSource);
{"path":{"name":["foo","bar"]}}""", syntheticSource);
}

public void testNoDynamicObjectSimpleValueArray() throws IOException {
Expand Down Expand Up @@ -1365,7 +1449,20 @@ public void testNoDynamicObjectNestedArray() throws IOException {
b.endArray();
});
assertEquals("""
{"path":[{"to":{"foo":"A","bar":"B"}},{"to":{"foo":"C","bar":"D"}}]}""", syntheticSource);
{"path":{"to":[{"foo":"A","bar":"B"},{"foo":"C","bar":"D"}]}}""", syntheticSource);
}

public void testNoDynamicRootObject() throws IOException {
DocumentMapper documentMapper = createMapperService(topMapping(b -> {
b.startObject("_source").field("mode", "synthetic").endObject().field("dynamic", "false");
})).documentMapper();
var syntheticSource = syntheticSource(documentMapper, b -> {
b.field("foo", "bar");
b.startObject("path").field("X", "Y").endObject();
b.array("name", "A", "D", "C", "B");
});
assertEquals("""
{"foo":"bar","name":["A","D","C","B"],"path":{"X":"Y"}}""", syntheticSource);
}

public void testRuntimeDynamicObjectSingleField() throws IOException {
Expand Down Expand Up @@ -1445,7 +1542,7 @@ public void testRuntimeDynamicObjectSimpleArray() throws IOException {
b.endArray();
});
assertEquals("""
{"path":[{"name":"foo"},{"name":"bar"}]}""", syntheticSource);
{"path":{"name":["foo","bar"]}}""", syntheticSource);
}

public void testRuntimeDynamicObjectSimpleValueArray() throws IOException {
Expand Down Expand Up @@ -1473,7 +1570,7 @@ public void testRuntimeDynamicObjectNestedArray() throws IOException {
b.endArray();
});
assertEquals("""
{"path":[{"to":{"foo":"A","bar":"B"}},{"to":{"foo":"C","bar":"D"}}]}""", syntheticSource);
{"path":{"to":[{"foo":"A","bar":"B"},{"foo":"C","bar":"D"}]}}""", syntheticSource);
}

public void testDisabledSubObjectWithNameOverlappingParentName() throws IOException {
Expand Down

0 comments on commit e5bebfe

Please sign in to comment.