Skip to content

Commit cfbe148

Browse files
authored
Use second pass for deeply nested array elements (#114060) (#114068)
Deeply nested array elements with ignored source need to use the second parsing pass, to avoid missing the source from siblings that go through stored source. (cherry picked from commit 5531e5d)
1 parent 0b224dc commit cfbe148

File tree

3 files changed

+90
-17
lines changed

3 files changed

+90
-17
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -824,8 +824,8 @@ private static void parseNonDynamicArray(
824824

825825
// In synthetic source, if any array element requires storing its source as-is, it takes precedence over
826826
// elements from regular source loading that are then skipped from the synthesized array source.
827-
// To prevent this, we track each array name, to check if it contains any sub-arrays in its elements.
828-
context = context.cloneForArray(fullPath);
827+
// To prevent this, we track that parsing sub-context is within array scope.
828+
context = context.maybeCloneForArray(mapper);
829829

830830
XContentParser parser = context.parser();
831831
XContentParser.Token token;

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

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ public int get() {
111111
private final Set<String> ignoredFields;
112112
private final List<IgnoredSourceFieldMapper.NameValue> ignoredFieldValues;
113113
private final List<IgnoredSourceFieldMapper.NameValue> ignoredFieldsMissingValues;
114-
private String parentArrayField;
114+
private boolean inArrayScope;
115115

116116
private final Map<String, List<Mapper>> dynamicMappers;
117117
private final DynamicMapperSize dynamicMappersSize;
@@ -143,7 +143,7 @@ private DocumentParserContext(
143143
Set<String> ignoreFields,
144144
List<IgnoredSourceFieldMapper.NameValue> ignoredFieldValues,
145145
List<IgnoredSourceFieldMapper.NameValue> ignoredFieldsWithNoSource,
146-
String parentArrayField,
146+
boolean inArrayScope,
147147
Map<String, List<Mapper>> dynamicMappers,
148148
Map<String, ObjectMapper> dynamicObjectMappers,
149149
Map<String, List<RuntimeField>> dynamicRuntimeFields,
@@ -164,7 +164,7 @@ private DocumentParserContext(
164164
this.ignoredFields = ignoreFields;
165165
this.ignoredFieldValues = ignoredFieldValues;
166166
this.ignoredFieldsMissingValues = ignoredFieldsWithNoSource;
167-
this.parentArrayField = parentArrayField;
167+
this.inArrayScope = inArrayScope;
168168
this.dynamicMappers = dynamicMappers;
169169
this.dynamicObjectMappers = dynamicObjectMappers;
170170
this.dynamicRuntimeFields = dynamicRuntimeFields;
@@ -188,7 +188,7 @@ private DocumentParserContext(ObjectMapper parent, ObjectMapper.Dynamic dynamic,
188188
in.ignoredFields,
189189
in.ignoredFieldValues,
190190
in.ignoredFieldsMissingValues,
191-
in.parentArrayField,
191+
in.inArrayScope,
192192
in.dynamicMappers,
193193
in.dynamicObjectMappers,
194194
in.dynamicRuntimeFields,
@@ -219,7 +219,7 @@ protected DocumentParserContext(
219219
new HashSet<>(),
220220
new ArrayList<>(),
221221
new ArrayList<>(),
222-
null,
222+
false,
223223
new HashMap<>(),
224224
new HashMap<>(),
225225
new HashMap<>(),
@@ -324,10 +324,7 @@ public final void deduplicateIgnoredFieldValues(final Set<String> fullNames) {
324324
public final DocumentParserContext addIgnoredFieldFromContext(IgnoredSourceFieldMapper.NameValue ignoredFieldWithNoSource)
325325
throws IOException {
326326
if (canAddIgnoredField()) {
327-
if (parentArrayField != null
328-
&& parent != null
329-
&& parentArrayField.equals(parent.fullPath())
330-
&& parent instanceof NestedObjectMapper == false) {
327+
if (inArrayScope) {
331328
// The field is an array within an array, store all sub-array elements.
332329
ignoredFieldsMissingValues.add(ignoredFieldWithNoSource);
333330
return cloneWithRecordedSource();
@@ -364,14 +361,17 @@ public final Collection<IgnoredSourceFieldMapper.NameValue> getIgnoredFieldsMiss
364361
}
365362

366363
/**
367-
* Clones the current context to mark it as an array. Records the full name of the array field, to check for sub-arrays.
364+
* Clones the current context to mark it as an array, if it's not already marked, or restore it if it's within a nested object.
368365
* Applies to synthetic source only.
369366
*/
370-
public final DocumentParserContext cloneForArray(String fullName) throws IOException {
371-
if (canAddIgnoredField()) {
372-
DocumentParserContext subcontext = switchParser(parser());
373-
subcontext.parentArrayField = fullName;
374-
return subcontext;
367+
public final DocumentParserContext maybeCloneForArray(Mapper mapper) throws IOException {
368+
if (canAddIgnoredField() && mapper instanceof ObjectMapper) {
369+
boolean isNested = mapper instanceof NestedObjectMapper;
370+
if ((inArrayScope == false && isNested == false) || (inArrayScope && isNested)) {
371+
DocumentParserContext subcontext = switchParser(parser());
372+
subcontext.inArrayScope = inArrayScope == false;
373+
return subcontext;
374+
}
375375
}
376376
return this;
377377
}

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

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -964,6 +964,79 @@ public void testObjectArrayAndValue() throws IOException {
964964
{"path":{"stored":[{"leaf":10},{"leaf":20}]}}""", syntheticSource);
965965
}
966966

967+
public void testDeeplyNestedObjectArrayAndValue() throws IOException {
968+
DocumentMapper documentMapper = createMapperService(syntheticSourceMapping(b -> {
969+
b.startObject("path").startObject("properties").startObject("to").startObject("properties");
970+
{
971+
b.startObject("stored");
972+
{
973+
b.field("type", "object").field("store_array_source", true);
974+
b.startObject("properties").startObject("leaf").field("type", "integer").endObject().endObject();
975+
}
976+
b.endObject();
977+
}
978+
b.endObject().endObject().endObject().endObject();
979+
})).documentMapper();
980+
var syntheticSource = syntheticSource(documentMapper, b -> {
981+
b.startArray("path");
982+
{
983+
b.startObject();
984+
{
985+
b.startObject("to").startArray("stored");
986+
{
987+
b.startObject().field("leaf", 10).endObject();
988+
}
989+
b.endArray().endObject();
990+
}
991+
b.endObject();
992+
b.startObject();
993+
{
994+
b.startObject("to").startObject("stored").field("leaf", 20).endObject().endObject();
995+
}
996+
b.endObject();
997+
}
998+
b.endArray();
999+
});
1000+
assertEquals("""
1001+
{"path":{"to":{"stored":[{"leaf":10},{"leaf":20}]}}}""", syntheticSource);
1002+
}
1003+
1004+
public void testObjectArrayAndValueInNestedObject() throws IOException {
1005+
DocumentMapper documentMapper = createMapperService(syntheticSourceMapping(b -> {
1006+
b.startObject("path").startObject("properties").startObject("to").startObject("properties");
1007+
{
1008+
b.startObject("stored");
1009+
{
1010+
b.field("type", "nested").field("dynamic", false);
1011+
}
1012+
b.endObject();
1013+
}
1014+
b.endObject().endObject().endObject().endObject();
1015+
})).documentMapper();
1016+
var syntheticSource = syntheticSource(documentMapper, b -> {
1017+
b.startArray("path");
1018+
{
1019+
b.startObject();
1020+
{
1021+
b.startObject("to").startArray("stored");
1022+
{
1023+
b.startObject().field("leaf", 10).endObject();
1024+
}
1025+
b.endArray().endObject();
1026+
}
1027+
b.endObject();
1028+
b.startObject();
1029+
{
1030+
b.startObject("to").startObject("stored").field("leaf", 20).endObject().endObject();
1031+
}
1032+
b.endObject();
1033+
}
1034+
b.endArray();
1035+
});
1036+
assertEquals("""
1037+
{"path":{"to":{"stored":[{"leaf":10},{"leaf":20}]}}}""", syntheticSource);
1038+
}
1039+
9671040
public void testObjectArrayAndValueDisabledObject() throws IOException {
9681041
DocumentMapper documentMapper = createMapperService(syntheticSourceMapping(b -> {
9691042
b.startObject("path").field("type", "object").startObject("properties");

0 commit comments

Comments
 (0)