Explorar o código

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 5531e5dcd2a3632685b4d27253c431fa11294cf8)
Kostas Krikellas hai 1 ano
pai
achega
cfbe148d90

+ 2 - 2
server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java

@@ -824,8 +824,8 @@ public final class DocumentParser {
 
         // In synthetic source, if any array element requires storing its source as-is, it takes precedence over
         // elements from regular source loading that are then skipped from the synthesized array source.
-        // To prevent this, we track each array name, to check if it contains any sub-arrays in its elements.
-        context = context.cloneForArray(fullPath);
+        // To prevent this, we track that parsing sub-context is within array scope.
+        context = context.maybeCloneForArray(mapper);
 
         XContentParser parser = context.parser();
         XContentParser.Token token;

+ 15 - 15
server/src/main/java/org/elasticsearch/index/mapper/DocumentParserContext.java

@@ -111,7 +111,7 @@ public abstract class DocumentParserContext {
     private final Set<String> ignoredFields;
     private final List<IgnoredSourceFieldMapper.NameValue> ignoredFieldValues;
     private final List<IgnoredSourceFieldMapper.NameValue> ignoredFieldsMissingValues;
-    private String parentArrayField;
+    private boolean inArrayScope;
 
     private final Map<String, List<Mapper>> dynamicMappers;
     private final DynamicMapperSize dynamicMappersSize;
@@ -143,7 +143,7 @@ public abstract class DocumentParserContext {
         Set<String> ignoreFields,
         List<IgnoredSourceFieldMapper.NameValue> ignoredFieldValues,
         List<IgnoredSourceFieldMapper.NameValue> ignoredFieldsWithNoSource,
-        String parentArrayField,
+        boolean inArrayScope,
         Map<String, List<Mapper>> dynamicMappers,
         Map<String, ObjectMapper> dynamicObjectMappers,
         Map<String, List<RuntimeField>> dynamicRuntimeFields,
@@ -164,7 +164,7 @@ public abstract class DocumentParserContext {
         this.ignoredFields = ignoreFields;
         this.ignoredFieldValues = ignoredFieldValues;
         this.ignoredFieldsMissingValues = ignoredFieldsWithNoSource;
-        this.parentArrayField = parentArrayField;
+        this.inArrayScope = inArrayScope;
         this.dynamicMappers = dynamicMappers;
         this.dynamicObjectMappers = dynamicObjectMappers;
         this.dynamicRuntimeFields = dynamicRuntimeFields;
@@ -188,7 +188,7 @@ public abstract class DocumentParserContext {
             in.ignoredFields,
             in.ignoredFieldValues,
             in.ignoredFieldsMissingValues,
-            in.parentArrayField,
+            in.inArrayScope,
             in.dynamicMappers,
             in.dynamicObjectMappers,
             in.dynamicRuntimeFields,
@@ -219,7 +219,7 @@ public abstract class DocumentParserContext {
             new HashSet<>(),
             new ArrayList<>(),
             new ArrayList<>(),
-            null,
+            false,
             new HashMap<>(),
             new HashMap<>(),
             new HashMap<>(),
@@ -324,10 +324,7 @@ public abstract class DocumentParserContext {
     public final DocumentParserContext addIgnoredFieldFromContext(IgnoredSourceFieldMapper.NameValue ignoredFieldWithNoSource)
         throws IOException {
         if (canAddIgnoredField()) {
-            if (parentArrayField != null
-                && parent != null
-                && parentArrayField.equals(parent.fullPath())
-                && parent instanceof NestedObjectMapper == false) {
+            if (inArrayScope) {
                 // The field is an array within an array, store all sub-array elements.
                 ignoredFieldsMissingValues.add(ignoredFieldWithNoSource);
                 return cloneWithRecordedSource();
@@ -364,14 +361,17 @@ public abstract class DocumentParserContext {
     }
 
     /**
-     * Clones the current context to mark it as an array. Records the full name of the array field, to check for sub-arrays.
+     * 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.
      * Applies to synthetic source only.
      */
-    public final DocumentParserContext cloneForArray(String fullName) throws IOException {
-        if (canAddIgnoredField()) {
-            DocumentParserContext subcontext = switchParser(parser());
-            subcontext.parentArrayField = fullName;
-            return subcontext;
+    public final DocumentParserContext maybeCloneForArray(Mapper mapper) throws IOException {
+        if (canAddIgnoredField() && mapper instanceof ObjectMapper) {
+            boolean isNested = mapper instanceof NestedObjectMapper;
+            if ((inArrayScope == false && isNested == false) || (inArrayScope && isNested)) {
+                DocumentParserContext subcontext = switchParser(parser());
+                subcontext.inArrayScope = inArrayScope == false;
+                return subcontext;
+            }
         }
         return this;
     }

+ 73 - 0
server/src/test/java/org/elasticsearch/index/mapper/IgnoredSourceFieldMapperTests.java

@@ -964,6 +964,79 @@ public class IgnoredSourceFieldMapperTests extends MapperServiceTestCase {
             {"path":{"stored":[{"leaf":10},{"leaf":20}]}}""", syntheticSource);
     }
 
+    public void testDeeplyNestedObjectArrayAndValue() throws IOException {
+        DocumentMapper documentMapper = createMapperService(syntheticSourceMapping(b -> {
+            b.startObject("path").startObject("properties").startObject("to").startObject("properties");
+            {
+                b.startObject("stored");
+                {
+                    b.field("type", "object").field("store_array_source", true);
+                    b.startObject("properties").startObject("leaf").field("type", "integer").endObject().endObject();
+                }
+                b.endObject();
+            }
+            b.endObject().endObject().endObject().endObject();
+        })).documentMapper();
+        var syntheticSource = syntheticSource(documentMapper, b -> {
+            b.startArray("path");
+            {
+                b.startObject();
+                {
+                    b.startObject("to").startArray("stored");
+                    {
+                        b.startObject().field("leaf", 10).endObject();
+                    }
+                    b.endArray().endObject();
+                }
+                b.endObject();
+                b.startObject();
+                {
+                    b.startObject("to").startObject("stored").field("leaf", 20).endObject().endObject();
+                }
+                b.endObject();
+            }
+            b.endArray();
+        });
+        assertEquals("""
+            {"path":{"to":{"stored":[{"leaf":10},{"leaf":20}]}}}""", syntheticSource);
+    }
+
+    public void testObjectArrayAndValueInNestedObject() throws IOException {
+        DocumentMapper documentMapper = createMapperService(syntheticSourceMapping(b -> {
+            b.startObject("path").startObject("properties").startObject("to").startObject("properties");
+            {
+                b.startObject("stored");
+                {
+                    b.field("type", "nested").field("dynamic", false);
+                }
+                b.endObject();
+            }
+            b.endObject().endObject().endObject().endObject();
+        })).documentMapper();
+        var syntheticSource = syntheticSource(documentMapper, b -> {
+            b.startArray("path");
+            {
+                b.startObject();
+                {
+                    b.startObject("to").startArray("stored");
+                    {
+                        b.startObject().field("leaf", 10).endObject();
+                    }
+                    b.endArray().endObject();
+                }
+                b.endObject();
+                b.startObject();
+                {
+                    b.startObject("to").startObject("stored").field("leaf", 20).endObject().endObject();
+                }
+                b.endObject();
+            }
+            b.endArray();
+        });
+        assertEquals("""
+            {"path":{"to":{"stored":[{"leaf":10},{"leaf":20}]}}}""", syntheticSource);
+    }
+
     public void testObjectArrayAndValueDisabledObject() throws IOException {
         DocumentMapper documentMapper = createMapperService(syntheticSourceMapping(b -> {
             b.startObject("path").field("type", "object").startObject("properties");