Skip to content

Commit 86bcb99

Browse files
authored
[8.x] Apply workaround for synthetic source of object arrays inside nested objects (#115275) (#115467)
* Apply workaround for synthetic source of object arrays inside nested objects (#115275) (cherry picked from commit f04bf5c) # Conflicts: # rest-api-spec/build.gradle # rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.create/21_synthetic_source_stored.yml * Fix merge
1 parent cd2a884 commit 86bcb99

File tree

6 files changed

+131
-19
lines changed

6 files changed

+131
-19
lines changed

rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.create/21_synthetic_source_stored.yml

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -319,8 +319,8 @@ object param - nested object array next to other fields:
319319
---
320320
object param - nested object with stored array:
321321
- requires:
322-
cluster_features: ["mapper.synthetic_source_keep"]
323-
reason: requires tracking ignored source
322+
cluster_features: ["mapper.ignored_source.always_store_object_arrays_in_nested"]
323+
reason: requires fix to object array handling
324324

325325
- do:
326326
indices.create:
@@ -356,8 +356,11 @@ object param - nested object with stored array:
356356
sort: name
357357
- match: { hits.total.value: 2 }
358358
- match: { hits.hits.0._source.name: A }
359-
- match: { hits.hits.0._source.nested_array_regular.0.b.c: [ 10, 100] }
360-
- match: { hits.hits.0._source.nested_array_regular.1.b.c: [ 20, 200] }
359+
# due to a workaround for #115261
360+
- match: { hits.hits.0._source.nested_array_regular.0.b.0.c: 10 }
361+
- match: { hits.hits.0._source.nested_array_regular.0.b.1.c: 100 }
362+
- match: { hits.hits.0._source.nested_array_regular.1.b.0.c: 20 }
363+
- match: { hits.hits.0._source.nested_array_regular.1.b.1.c: 200 }
361364
- match: { hits.hits.1._source.name: B }
362365
- match: { hits.hits.1._source.nested_array_stored.0.b.0.c: 10 }
363366
- match: { hits.hits.1._source.nested_array_stored.0.b.1.c: 100 }

server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -810,8 +810,10 @@ private static void parseNonDynamicArray(
810810
boolean objectWithFallbackSyntheticSource = false;
811811
if (mapper instanceof ObjectMapper objectMapper) {
812812
mode = getSourceKeepMode(context, objectMapper.sourceKeepMode());
813-
objectWithFallbackSyntheticSource = (mode == Mapper.SourceKeepMode.ALL
814-
|| (mode == Mapper.SourceKeepMode.ARRAYS && objectMapper instanceof NestedObjectMapper == false));
813+
objectWithFallbackSyntheticSource = mode == Mapper.SourceKeepMode.ALL
814+
// Inside nested objects we always store object arrays as a workaround for #115261.
815+
|| ((context.inNestedScope() || mode == Mapper.SourceKeepMode.ARRAYS)
816+
&& objectMapper instanceof NestedObjectMapper == false);
815817
}
816818
boolean fieldWithFallbackSyntheticSource = false;
817819
boolean fieldWithStoredArraySource = false;

server/src/main/java/org/elasticsearch/index/mapper/DocumentParserContext.java

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,16 @@ public int get() {
104104
}
105105
}
106106

107+
/**
108+
* Defines the scope parser is currently in.
109+
* This is used for synthetic source related logic during parsing.
110+
*/
111+
private enum Scope {
112+
SINGLETON,
113+
ARRAY,
114+
NESTED
115+
}
116+
107117
private final MappingLookup mappingLookup;
108118
private final MappingParserContext mappingParserContext;
109119
private final SourceToParse sourceToParse;
@@ -112,7 +122,7 @@ public int get() {
112122
private final List<IgnoredSourceFieldMapper.NameValue> ignoredFieldValues;
113123
private final List<IgnoredSourceFieldMapper.NameValue> ignoredFieldsMissingValues;
114124
private boolean inArrayScopeEnabled;
115-
private boolean inArrayScope;
125+
private Scope currentScope;
116126

117127
private final Map<String, List<Mapper>> dynamicMappers;
118128
private final DynamicMapperSize dynamicMappersSize;
@@ -145,7 +155,7 @@ private DocumentParserContext(
145155
List<IgnoredSourceFieldMapper.NameValue> ignoredFieldValues,
146156
List<IgnoredSourceFieldMapper.NameValue> ignoredFieldsWithNoSource,
147157
boolean inArrayScopeEnabled,
148-
boolean inArrayScope,
158+
Scope currentScope,
149159
Map<String, List<Mapper>> dynamicMappers,
150160
Map<String, ObjectMapper> dynamicObjectMappers,
151161
Map<String, List<RuntimeField>> dynamicRuntimeFields,
@@ -167,7 +177,7 @@ private DocumentParserContext(
167177
this.ignoredFieldValues = ignoredFieldValues;
168178
this.ignoredFieldsMissingValues = ignoredFieldsWithNoSource;
169179
this.inArrayScopeEnabled = inArrayScopeEnabled;
170-
this.inArrayScope = inArrayScope;
180+
this.currentScope = currentScope;
171181
this.dynamicMappers = dynamicMappers;
172182
this.dynamicObjectMappers = dynamicObjectMappers;
173183
this.dynamicRuntimeFields = dynamicRuntimeFields;
@@ -192,7 +202,7 @@ private DocumentParserContext(ObjectMapper parent, ObjectMapper.Dynamic dynamic,
192202
in.ignoredFieldValues,
193203
in.ignoredFieldsMissingValues,
194204
in.inArrayScopeEnabled,
195-
in.inArrayScope,
205+
in.currentScope,
196206
in.dynamicMappers,
197207
in.dynamicObjectMappers,
198208
in.dynamicRuntimeFields,
@@ -224,7 +234,7 @@ protected DocumentParserContext(
224234
new ArrayList<>(),
225235
new ArrayList<>(),
226236
mappingParserContext.getIndexSettings().isSyntheticSourceSecondDocParsingPassEnabled(),
227-
false,
237+
Scope.SINGLETON,
228238
new HashMap<>(),
229239
new HashMap<>(),
230240
new HashMap<>(),
@@ -335,7 +345,7 @@ public final void deduplicateIgnoredFieldValues(final Set<String> fullNames) {
335345
public final DocumentParserContext addIgnoredFieldFromContext(IgnoredSourceFieldMapper.NameValue ignoredFieldWithNoSource)
336346
throws IOException {
337347
if (canAddIgnoredField()) {
338-
if (inArrayScope) {
348+
if (currentScope == Scope.ARRAY) {
339349
// The field is an array within an array, store all sub-array elements.
340350
ignoredFieldsMissingValues.add(ignoredFieldWithNoSource);
341351
return cloneWithRecordedSource();
@@ -379,10 +389,10 @@ public final DocumentParserContext maybeCloneForArray(Mapper mapper) throws IOEx
379389
if (canAddIgnoredField()
380390
&& mapper instanceof ObjectMapper
381391
&& mapper instanceof NestedObjectMapper == false
382-
&& inArrayScope == false
392+
&& currentScope != Scope.ARRAY
383393
&& inArrayScopeEnabled) {
384394
DocumentParserContext subcontext = switchParser(parser());
385-
subcontext.inArrayScope = true;
395+
subcontext.currentScope = Scope.ARRAY;
386396
return subcontext;
387397
}
388398
return this;
@@ -673,6 +683,10 @@ public boolean isWithinCopyTo() {
673683
return false;
674684
}
675685

686+
public boolean inNestedScope() {
687+
return currentScope == Scope.NESTED;
688+
}
689+
676690
public final DocumentParserContext createChildContext(ObjectMapper parent) {
677691
return new Wrapper(parent, this);
678692
}
@@ -716,10 +730,11 @@ public LuceneDocument doc() {
716730
return document;
717731
}
718732
};
719-
// Disable tracking array scopes for ignored source, as it would be added to the parent doc.
720-
// Nested documents are added to preserve object structure within arrays of objects, so the use
721-
// of ignored source for arrays inside them should be mostly redundant.
722-
cloned.inArrayScope = false;
733+
734+
cloned.currentScope = Scope.NESTED;
735+
// Disable using second parsing pass since it currently can not determine which parts
736+
// of source belong to which nested document.
737+
// See #115261.
723738
cloned.inArrayScopeEnabled = false;
724739
return cloned;
725740
}

server/src/main/java/org/elasticsearch/index/mapper/IgnoredSourceFieldMapper.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,9 @@ public class IgnoredSourceFieldMapper extends MetadataFieldMapper {
5858

5959
static final NodeFeature TRACK_IGNORED_SOURCE = new NodeFeature("mapper.track_ignored_source");
6060
static final NodeFeature DONT_EXPAND_DOTS_IN_IGNORED_SOURCE = new NodeFeature("mapper.ignored_source.dont_expand_dots");
61+
static final NodeFeature ALWAYS_STORE_OBJECT_ARRAYS_IN_NESTED_OBJECTS = new NodeFeature(
62+
"mapper.ignored_source.always_store_object_arrays_in_nested"
63+
);
6164

6265
/*
6366
Setting to disable encoding and writing values for this field.

server/src/main/java/org/elasticsearch/index/mapper/MapperFeatures.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,8 @@ public Set<NodeFeature> getTestFeatures() {
5656
return Set.of(
5757
RangeFieldMapper.DATE_RANGE_INDEXING_FIX,
5858
IgnoredSourceFieldMapper.DONT_EXPAND_DOTS_IN_IGNORED_SOURCE,
59-
SourceFieldMapper.REMOVE_SYNTHETIC_SOURCE_ONLY_VALIDATION
59+
SourceFieldMapper.REMOVE_SYNTHETIC_SOURCE_ONLY_VALIDATION,
60+
IgnoredSourceFieldMapper.ALWAYS_STORE_OBJECT_ARRAYS_IN_NESTED_OBJECTS
6061
);
6162
}
6263
}

server/src/test/java/org/elasticsearch/index/mapper/IgnoredSourceFieldMapperTests.java

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -962,6 +962,94 @@ public void testArrayWithNestedObjects() throws IOException {
962962
{"path":{"to":[{"id":[1,20,3]},{"id":10},{"id":0}]}}""", syntheticSource);
963963
}
964964

965+
public void testObjectArrayWithinNestedObjects() throws IOException {
966+
DocumentMapper documentMapper = createMapperService(syntheticSourceMapping(b -> {
967+
b.startObject("path").startObject("properties");
968+
{
969+
b.startObject("to").field("type", "nested").startObject("properties");
970+
{
971+
b.startObject("obj").startObject("properties");
972+
{
973+
b.startObject("id").field("type", "integer").field("synthetic_source_keep", "arrays").endObject();
974+
}
975+
b.endObject().endObject();
976+
}
977+
b.endObject().endObject();
978+
}
979+
b.endObject().endObject();
980+
})).documentMapper();
981+
982+
var syntheticSource = syntheticSource(documentMapper, b -> {
983+
b.startObject("path");
984+
{
985+
b.startObject("to");
986+
{
987+
b.startArray("obj");
988+
{
989+
b.startObject().array("id", 1, 20, 3).endObject();
990+
b.startObject().field("id", 10).endObject();
991+
}
992+
b.endArray();
993+
}
994+
b.endObject();
995+
}
996+
b.endObject();
997+
});
998+
assertEquals("""
999+
{"path":{"to":{"obj":[{"id":[1,20,3]},{"id":10}]}}}""", syntheticSource);
1000+
}
1001+
1002+
public void testObjectArrayWithinNestedObjectsArray() throws IOException {
1003+
DocumentMapper documentMapper = createMapperService(syntheticSourceMapping(b -> {
1004+
b.startObject("path").startObject("properties");
1005+
{
1006+
b.startObject("to").field("type", "nested").startObject("properties");
1007+
{
1008+
b.startObject("obj").startObject("properties");
1009+
{
1010+
b.startObject("id").field("type", "integer").field("synthetic_source_keep", "arrays").endObject();
1011+
}
1012+
b.endObject().endObject();
1013+
}
1014+
b.endObject().endObject();
1015+
}
1016+
b.endObject().endObject();
1017+
})).documentMapper();
1018+
1019+
var syntheticSource = syntheticSource(documentMapper, b -> {
1020+
b.startObject("path");
1021+
{
1022+
b.startArray("to");
1023+
{
1024+
b.startObject();
1025+
{
1026+
b.startArray("obj");
1027+
{
1028+
b.startObject().array("id", 1, 20, 3).endObject();
1029+
b.startObject().field("id", 10).endObject();
1030+
}
1031+
b.endArray();
1032+
}
1033+
b.endObject();
1034+
b.startObject();
1035+
{
1036+
b.startArray("obj");
1037+
{
1038+
b.startObject().array("id", 200, 300, 500).endObject();
1039+
b.startObject().field("id", 100).endObject();
1040+
}
1041+
b.endArray();
1042+
}
1043+
b.endObject();
1044+
}
1045+
b.endArray();
1046+
}
1047+
b.endObject();
1048+
});
1049+
assertEquals("""
1050+
{"path":{"to":[{"obj":[{"id":[1,20,3]},{"id":10}]},{"obj":[{"id":[200,300,500]},{"id":100}]}]}}""", syntheticSource);
1051+
}
1052+
9651053
public void testArrayWithinArray() throws IOException {
9661054
DocumentMapper documentMapper = createMapperService(syntheticSourceMapping(b -> {
9671055
b.startObject("path");

0 commit comments

Comments
 (0)