Skip storing ignored source for single-element leaf arrays (#113937)

* Minimize storing array source

* restrict to fields

* revert changes for `addIgnoredFieldFromContext`

* fix test

* spotless

* count nulls

Author: kkrik-es

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

@@ -799,17 +799,30 @@ private static void parseNonDynamicArray(
         String fullPath = context.path().pathAsText(arrayFieldName);
 
         // Check if we need to record the array source. This only applies to synthetic source.
+        boolean canRemoveSingleLeafElement = false;
         if (context.canAddIgnoredField()) {
-            boolean objectRequiresStoringSource = mapper instanceof ObjectMapper objectMapper
-                && (getSourceKeepMode(context, objectMapper.sourceKeepMode()) == Mapper.SourceKeepMode.ALL
-                    || (getSourceKeepMode(context, objectMapper.sourceKeepMode()) == Mapper.SourceKeepMode.ARRAYS
-                        && 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;
+            Mapper.SourceKeepMode mode = Mapper.SourceKeepMode.NONE;
+            boolean objectWithFallbackSyntheticSource = false;
+            if (mapper instanceof ObjectMapper objectMapper) {
+                mode = getSourceKeepMode(context, objectMapper.sourceKeepMode());
+                objectWithFallbackSyntheticSource = (mode == Mapper.SourceKeepMode.ALL
+                    || (mode == Mapper.SourceKeepMode.ARRAYS && objectMapper instanceof NestedObjectMapper == false));
+            }
+            boolean fieldWithFallbackSyntheticSource = false;
+            boolean fieldWithStoredArraySource = false;
+            if (mapper instanceof FieldMapper fieldMapper) {
+                mode = getSourceKeepMode(context, fieldMapper.sourceKeepMode());
+                fieldWithFallbackSyntheticSource = fieldMapper.syntheticSourceMode() == FieldMapper.SyntheticSourceMode.FALLBACK;
+                fieldWithStoredArraySource = mode != Mapper.SourceKeepMode.NONE;
+            }
             boolean copyToFieldHasValuesInDocument = context.isWithinCopyTo() == false && context.isCopyToDestinationField(fullPath);
-            if (objectRequiresStoringSource
+
+            canRemoveSingleLeafElement = mapper instanceof FieldMapper
+                && mode == Mapper.SourceKeepMode.ARRAYS
+                && fieldWithFallbackSyntheticSource == false
+                && copyToFieldHasValuesInDocument == false;
+
+            if (objectWithFallbackSyntheticSource
                 || fieldWithFallbackSyntheticSource
                 || fieldWithStoredArraySource
                 || copyToFieldHasValuesInDocument) {
@@ -829,20 +842,28 @@ private static void parseNonDynamicArray(
 
         XContentParser parser = context.parser();
         XContentParser.Token token;
+        int elements = 0;
         while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) {
             if (token == XContentParser.Token.START_OBJECT) {
+                elements = Integer.MAX_VALUE;
                 parseObject(context, lastFieldName);
             } else if (token == XContentParser.Token.START_ARRAY) {
+                elements = Integer.MAX_VALUE;
                 parseArray(context, lastFieldName);
             } else if (token == XContentParser.Token.VALUE_NULL) {
+                elements++;
                 parseNullValue(context, lastFieldName);
             } else if (token == null) {
                 throwEOFOnParseArray(arrayFieldName, context);
             } else {
                 assert token.isValue();
+                elements++;
                 parseValue(context, lastFieldName);
             }
         }
+        if (elements <= 1 && canRemoveSingleLeafElement) {
+            context.removeLastIgnoredField(fullPath);
+        }
         postProcessDynamicArrayMapping(context, lastFieldName);
     }
 

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

@@ -296,6 +296,12 @@ public final void addIgnoredField(IgnoredSourceFieldMapper.NameValue values) {
         }
     }
 
+    final void removeLastIgnoredField(String name) {
+        if (ignoredFieldValues.isEmpty() == false && ignoredFieldValues.getLast().name().equals(name)) {
+            ignoredFieldValues.removeLast();
+        }
+    }
+
     /**
      * Return the collection of values for fields that have been ignored so far.
      */

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

@@ -532,6 +532,38 @@ public void testIndexStoredArraySourceRootValueArrayDisabled() throws IOExceptio
             {"bool_value":true,"int_value":[10,20,30]}""", syntheticSource);
     }
 
+    public void testIndexStoredArraySourceSingleLeafElement() throws IOException {
+        DocumentMapper documentMapper = createMapperServiceWithStoredArraySource(syntheticSourceMapping(b -> {
+            b.startObject("int_value").field("type", "integer").endObject();
+        })).documentMapper();
+        var syntheticSource = syntheticSource(documentMapper, b -> b.array("int_value", new int[] { 10 }));
+        assertEquals("{\"int_value\":10}", syntheticSource);
+        ParsedDocument doc = documentMapper.parse(source(syntheticSource));
+        assertNull(doc.rootDoc().getField("_ignored_source"));
+    }
+
+    public void testIndexStoredArraySourceSingleLeafElementAndNull() throws IOException {
+        DocumentMapper documentMapper = createMapperServiceWithStoredArraySource(syntheticSourceMapping(b -> {
+            b.startObject("value").field("type", "keyword").endObject();
+        })).documentMapper();
+        var syntheticSource = syntheticSource(documentMapper, b -> b.array("value", new String[] { "foo", null }));
+        assertEquals("{\"value\":[\"foo\",null]}", syntheticSource);
+    }
+
+    public void testIndexStoredArraySourceSingleObjectElement() throws IOException {
+        DocumentMapper documentMapper = createMapperServiceWithStoredArraySource(syntheticSourceMapping(b -> {
+            b.startObject("path").startObject("properties");
+            {
+                b.startObject("int_value").field("type", "integer").endObject();
+            }
+            b.endObject().endObject();
+        })).documentMapper();
+        var syntheticSource = syntheticSource(documentMapper, b -> {
+            b.startArray("path").startObject().field("int_value", 10).endObject().endArray();
+        });
+        assertEquals("{\"path\":[{\"int_value\":10}]}", syntheticSource);
+    }
+
     public void testFieldStoredArraySourceRootValueArray() throws IOException {
         DocumentMapper documentMapper = createMapperService(syntheticSourceMapping(b -> {
             b.startObject("int_value").field("type", "integer").field(Mapper.SYNTHETIC_SOURCE_KEEP_PARAM, "arrays").endObject();

test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java

@@ -1580,7 +1580,11 @@ public void testSyntheticSourceKeepArrays() throws IOException {
         buildInput.accept(builder);
         builder.endObject();
         String expected = Strings.toString(builder);
-        assertThat(syntheticSource(mapperAll, buildInput), equalTo(expected));
+        String actual = syntheticSource(mapperAll, buildInput);
+        // Check for single-element array, the array source is not stored in this case.
+        if (expected.replace("[", "").replace("]", "").equals(actual) == false) {
+            assertThat(actual, equalTo(expected));
+        }
     }
 
     @Override