Reset array scope tracking for nested objects (elastic#114891)

* Reset array scope tracking for nested objects * update * update * update
jfreden · Nov 4, 2024 · e7c20fb · e7c20fb
1 parent 737e782
commit e7c20fb
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 12 deletions.
diff --git a/muted-tests.yml b/muted-tests.yml
@@ -376,9 +376,6 @@ tests:
   issue: https://github.com/elastic/elasticsearch/issues/114839
 - class: org.elasticsearch.license.LicensingTests
   issue: https://github.com/elastic/elasticsearch/issues/114865
-- class: org.elasticsearch.datastreams.logsdb.qa.LogsDbVersusLogsDbReindexedIntoStandardModeChallengeRestIT
-  method: testTermsQuery
-  issue: https://github.com/elastic/elasticsearch/issues/114873
 - class: org.elasticsearch.xpack.enrich.EnrichIT
   method: testDeleteIsCaseSensitive
   issue: https://github.com/elastic/elasticsearch/issues/114840

diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DocumentParserContext.java b/server/src/main/java/org/elasticsearch/index/mapper/DocumentParserContext.java
@@ -111,7 +111,7 @@ public int get() {
     private final Set<String> ignoredFields;
     private final List<IgnoredSourceFieldMapper.NameValue> ignoredFieldValues;
     private final List<IgnoredSourceFieldMapper.NameValue> ignoredFieldsMissingValues;
-    private final boolean inArrayScopeEnabled;
+    private boolean inArrayScopeEnabled;
     private boolean inArrayScope;
 
     private final Map<String, List<Mapper>> dynamicMappers;
@@ -376,13 +376,14 @@ public final Collection<IgnoredSourceFieldMapper.NameValue> getIgnoredFieldsMiss
      * Applies to synthetic source only.
      */
     public final DocumentParserContext maybeCloneForArray(Mapper mapper) throws IOException {
-        if (canAddIgnoredField() && mapper instanceof ObjectMapper && inArrayScopeEnabled) {
-            boolean isNested = mapper instanceof NestedObjectMapper;
-            if ((inArrayScope == false && isNested == false) || (inArrayScope && isNested)) {
-                DocumentParserContext subcontext = switchParser(parser());
-                subcontext.inArrayScope = inArrayScope == false;
-                return subcontext;
-            }
+        if (canAddIgnoredField()
+            && mapper instanceof ObjectMapper
+            && mapper instanceof NestedObjectMapper == false
+            && inArrayScope == false
+            && inArrayScopeEnabled) {
+            DocumentParserContext subcontext = switchParser(parser());
+            subcontext.inArrayScope = true;
+            return subcontext;
         }
         return this;
     }
@@ -709,12 +710,18 @@ public final DocumentParserContext createNestedContext(NestedObjectMapper nested
      * Return a new context that has the provided document as the current document.
      */
     public final DocumentParserContext switchDoc(final LuceneDocument document) {
-        return new Wrapper(this.parent, this) {
+        DocumentParserContext cloned = new Wrapper(this.parent, this) {
             @Override
             public LuceneDocument doc() {
                 return document;
             }
         };
+        // Disable tracking array scopes for ignored source, as it would be added to the parent doc.
+        // Nested documents are added to preserve object structure within arrays of objects, so the use
+        // of ignored source for arrays inside them should be mostly redundant.
+        cloned.inArrayScope = false;
+        cloned.inArrayScopeEnabled = false;
+        return cloned;
     }
 
     /**

diff --git a/server/src/test/java/org/elasticsearch/index/mapper/IgnoredSourceFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/IgnoredSourceFieldMapperTests.java
@@ -932,6 +932,36 @@ public void testConflictingFieldNameAfterArray() throws IOException {
             {"path":{"id":0.1,"to":{"id":[1,20,3,10]}}}""", syntheticSource);
     }
 
+    public void testArrayWithNestedObjects() throws IOException {
+        DocumentMapper documentMapper = createMapperService(syntheticSourceMapping(b -> {
+            b.startObject("path").startObject("properties");
+            {
+                b.startObject("to").field("type", "nested").startObject("properties");
+                {
+                    b.startObject("id").field("type", "integer").field("synthetic_source_keep", "arrays").endObject();
+                }
+                b.endObject().endObject();
+            }
+            b.endObject().endObject();
+        })).documentMapper();
+
+        var syntheticSource = syntheticSource(documentMapper, b -> {
+            b.startArray("path");
+            {
+                b.startObject().startArray("to");
+                {
+                    b.startObject().array("id", 1, 20, 3).endObject();
+                    b.startObject().field("id", 10).endObject();
+                }
+                b.endArray().endObject();
+                b.startObject().startObject("to").field("id", "0.1").endObject().endObject();
+            }
+            b.endArray();
+        });
+        assertEquals("""
+            {"path":{"to":[{"id":[1,20,3]},{"id":10},{"id":0}]}}""", syntheticSource);
+    }
+
     public void testArrayWithinArray() throws IOException {
         DocumentMapper documentMapper = createMapperService(syntheticSourceMapping(b -> {
             b.startObject("path");