Track source for objects and fields with [synthetic_source_keep:arrays] in arrays as ignored (#116065) (#116225)

* Track source for objects and fields with [synthetic_source_keep:arrays] in arrays as ignored

* Update TransportResumeFollowActionTests.java

* rest compat fixes

* rest compat fixes

* update test

(cherry picked from commit 6cf4536)

# Conflicts:
#	rest-api-spec/build.gradle
#	rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.create/21_synthetic_source_stored.yml
#	server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java

Author: kkrik-es

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

@@ -368,11 +368,8 @@ object param - nested object with stored array:
         sort: name
   - match:  { hits.total.value: 2 }
   - match:  { hits.hits.0._source.name: A }
-    # due to a workaround for #115261
-  - match: { hits.hits.0._source.nested_array_regular.0.b.0.c: 10 }
-  - match: { hits.hits.0._source.nested_array_regular.0.b.1.c: 100 }
-  - match: { hits.hits.0._source.nested_array_regular.1.b.0.c: 20 }
-  - match: { hits.hits.0._source.nested_array_regular.1.b.1.c: 200 }
+  - match: { hits.hits.0._source.nested_array_regular.0.b.c: [ 10, 100 ] }
+  - match: { hits.hits.0._source.nested_array_regular.1.b.c: [ 20, 200 ] }
   - match: { hits.hits.1._source.name: B }
   - match: { hits.hits.1._source.nested_array_stored.0.b.0.c: 10 }
   - match: { hits.hits.1._source.nested_array_stored.0.b.1.c: 100 }
@@ -428,55 +425,7 @@ index param - nested array within array:
 
 
 ---
-index param - nested array within array - disabled second pass:
-  - requires:
-      cluster_features: ["mapper.synthetic_source_keep"]
-      reason: requires tracking ignored source
-
-  - do:
-      indices.create:
-        index: test
-        body:
-          settings:
-            index:
-              synthetic_source:
-                enable_second_doc_parsing_pass: false
-              mapping.source.mode: synthetic
-
-          mappings:
-            properties:
-              name:
-                type: keyword
-              path:
-                properties:
-                  to:
-                    properties:
-                      some:
-                        synthetic_source_keep: arrays
-                        properties:
-                          id:
-                            type: integer
-
-  - do:
-      bulk:
-        index: test
-        refresh: true
-        body:
-          - '{ "create": { } }'
-          - '{ "name": "A", "path": [ { "to": [ { "some" : [ { "id": 10 }, { "id": [1, 3, 2] } ] }, { "some": { "id": 100 } } ] }, { "to": { "some": { "id": [1000, 2000] } } } ] }'
-  - match: { errors: false }
 
-  - do:
-      search:
-        index: test
-        sort: name
-  - match:  { hits.hits.0._source.name: A }
-  - length: { hits.hits.0._source.path.to.some: 2}
-  - match: { hits.hits.0._source.path.to.some.0.id: 10 }
-  - match: { hits.hits.0._source.path.to.some.1.id: [ 1, 3, 2] }
-
-
----
 # 112156
 stored field under object with store_array_source:
   - requires:
@@ -944,8 +893,10 @@ index param - root arrays:
   - match: { hits.hits.1._source.obj.1.span.id: "2" }
 
   - match: { hits.hits.2._source.id: 3 }
-  - match: { hits.hits.2._source.obj_default.trace.id: [aa, bb] }
-  - match: { hits.hits.2._source.obj_default.span.id: "2" }
+  - match: { hits.hits.2._source.obj_default.trace.0.id: bb }
+  - match: { hits.hits.2._source.obj_default.trace.1.id: aa }
+  - match: { hits.hits.2._source.obj_default.span.0.id: "2" }
+  - match: { hits.hits.2._source.obj_default.span.1.id: "2" }
 
 
 ---

server/src/main/java/org/elasticsearch/common/settings/IndexScopedSettings.java

@@ -188,7 +188,6 @@ public final class IndexScopedSettings extends AbstractScopedSettings {
         FieldMapper.SYNTHETIC_SOURCE_KEEP_INDEX_SETTING,
         IgnoredSourceFieldMapper.SKIP_IGNORED_SOURCE_WRITE_SETTING,
         IgnoredSourceFieldMapper.SKIP_IGNORED_SOURCE_READ_SETTING,
-        IndexSettings.SYNTHETIC_SOURCE_SECOND_DOC_PARSING_PASS_SETTING,
         SourceFieldMapper.INDEX_MAPPER_SOURCE_MODE_SETTING,
 
         // validate that built-in similarities don't get redefined

server/src/main/java/org/elasticsearch/index/IndexSettings.java

@@ -656,13 +656,6 @@ public Iterator<Setting<?>> settings() {
         Property.Final
     );
 
-    public static final Setting<Boolean> SYNTHETIC_SOURCE_SECOND_DOC_PARSING_PASS_SETTING = Setting.boolSetting(
-        "index.synthetic_source.enable_second_doc_parsing_pass",
-        true,
-        Property.IndexScope,
-        Property.Dynamic
-    );
-
     /**
      * Returns <code>true</code> if TSDB encoding is enabled. The default is <code>true</code>
      */
@@ -832,7 +825,6 @@ private void setRetentionLeaseMillis(final TimeValue retentionLease) {
     private volatile long mappingDimensionFieldsLimit;
     private volatile boolean skipIgnoredSourceWrite;
     private volatile boolean skipIgnoredSourceRead;
-    private volatile boolean syntheticSourceSecondDocParsingPassEnabled;
     private final SourceFieldMapper.Mode indexMappingSourceMode;
     private final boolean recoverySourceEnabled;
 
@@ -995,7 +987,6 @@ public IndexSettings(final IndexMetadata indexMetadata, final Settings nodeSetti
         es87TSDBCodecEnabled = scopedSettings.get(TIME_SERIES_ES87TSDB_CODEC_ENABLED_SETTING);
         skipIgnoredSourceWrite = scopedSettings.get(IgnoredSourceFieldMapper.SKIP_IGNORED_SOURCE_WRITE_SETTING);
         skipIgnoredSourceRead = scopedSettings.get(IgnoredSourceFieldMapper.SKIP_IGNORED_SOURCE_READ_SETTING);
-        syntheticSourceSecondDocParsingPassEnabled = scopedSettings.get(SYNTHETIC_SOURCE_SECOND_DOC_PARSING_PASS_SETTING);
         indexMappingSourceMode = scopedSettings.get(SourceFieldMapper.INDEX_MAPPER_SOURCE_MODE_SETTING);
         recoverySourceEnabled = RecoverySettings.INDICES_RECOVERY_SOURCE_ENABLED_SETTING.get(nodeSettings);
 
@@ -1085,10 +1076,6 @@ public IndexSettings(final IndexMetadata indexMetadata, final Settings nodeSetti
             this::setSkipIgnoredSourceWrite
         );
         scopedSettings.addSettingsUpdateConsumer(IgnoredSourceFieldMapper.SKIP_IGNORED_SOURCE_READ_SETTING, this::setSkipIgnoredSourceRead);
-        scopedSettings.addSettingsUpdateConsumer(
-            SYNTHETIC_SOURCE_SECOND_DOC_PARSING_PASS_SETTING,
-            this::setSyntheticSourceSecondDocParsingPassEnabled
-        );
     }
 
     private void setSearchIdleAfter(TimeValue searchIdleAfter) {
@@ -1681,14 +1668,6 @@ private void setSkipIgnoredSourceRead(boolean value) {
         this.skipIgnoredSourceRead = value;
     }
 
-    private void setSyntheticSourceSecondDocParsingPassEnabled(boolean syntheticSourceSecondDocParsingPassEnabled) {
-        this.syntheticSourceSecondDocParsingPassEnabled = syntheticSourceSecondDocParsingPassEnabled;
-    }
-
-    public boolean isSyntheticSourceSecondDocParsingPassEnabled() {
-        return syntheticSourceSecondDocParsingPassEnabled;
-    }
-
     public SourceFieldMapper.Mode getIndexMappingSourceMode() {
         return indexMappingSourceMode;
     }

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

@@ -35,16 +35,13 @@
 
 import java.io.IOException;
 import java.util.ArrayList;
-import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
-import java.util.Set;
 import java.util.function.Consumer;
 
 import static org.elasticsearch.index.mapper.vectors.DenseVectorFieldMapper.MAX_DIMS_COUNT;
@@ -148,9 +145,6 @@ private void internalParseDocument(MetadataFieldMapper[] metadataFieldsMappers,
 
             executeIndexTimeScripts(context);
 
-            // Record additional entries for {@link IgnoredSourceFieldMapper} before calling #postParse, so that they get stored.
-            addIgnoredSourceMissingValues(context);
-
             for (MetadataFieldMapper metadataMapper : metadataFieldsMappers) {
                 metadataMapper.postParse(context);
             }
@@ -159,128 +153,6 @@ private void internalParseDocument(MetadataFieldMapper[] metadataFieldsMappers,
         }
     }
 
-    private void addIgnoredSourceMissingValues(DocumentParserContext context) throws IOException {
-        Collection<IgnoredSourceFieldMapper.NameValue> ignoredFieldsMissingValues = context.getIgnoredFieldsMissingValues();
-        if (ignoredFieldsMissingValues.isEmpty()) {
-            return;
-        }
-
-        // Clean up any conflicting ignored values, to avoid double-printing them as array elements in synthetic source.
-        Map<String, IgnoredSourceFieldMapper.NameValue> fields = new HashMap<>(ignoredFieldsMissingValues.size());
-        for (var field : ignoredFieldsMissingValues) {
-            fields.put(field.name(), field);
-        }
-        context.deduplicateIgnoredFieldValues(fields.keySet());
-
-        assert context.mappingLookup().isSourceSynthetic();
-        try (
-            XContentParser parser = XContentHelper.createParser(
-                parserConfiguration,
-                context.sourceToParse().source(),
-                context.sourceToParse().getXContentType()
-            )
-        ) {
-            DocumentParserContext newContext = new RootDocumentParserContext(
-                context.mappingLookup(),
-                mappingParserContext,
-                context.sourceToParse(),
-                parser
-            );
-            var nameValues = parseDocForMissingValues(newContext, fields);
-            for (var nameValue : nameValues) {
-                context.addIgnoredField(nameValue);
-            }
-        }
-    }
-
-    /**
-     * Simplified parsing version for retrieving the source of a given set of fields.
-     */
-    private static List<IgnoredSourceFieldMapper.NameValue> parseDocForMissingValues(
-        DocumentParserContext context,
-        Map<String, IgnoredSourceFieldMapper.NameValue> fields
-    ) throws IOException {
-        // Generate all possible parent names for the given fields.
-        // This is used to skip processing objects that can't generate missing values.
-        Set<String> parentNames = getPossibleParentNames(fields.keySet());
-        List<IgnoredSourceFieldMapper.NameValue> result = new ArrayList<>();
-
-        XContentParser parser = context.parser();
-        XContentParser.Token currentToken = parser.nextToken();
-        List<String> path = new ArrayList<>();
-        List<Boolean> isObjectInPath = new ArrayList<>();  // Tracks if path components correspond to an object or an array.
-        String fieldName = null;
-        while (currentToken != null) {
-            while (currentToken != XContentParser.Token.FIELD_NAME) {
-                if (fieldName != null
-                    && (currentToken == XContentParser.Token.START_OBJECT || currentToken == XContentParser.Token.START_ARRAY)) {
-                    if (parentNames.contains(getCurrentPath(path, fieldName)) == false) {
-                        // No missing value under this parsing subtree, skip it.
-                        parser.skipChildren();
-                    } else {
-                        path.add(fieldName);
-                        isObjectInPath.add(currentToken == XContentParser.Token.START_OBJECT);
-                    }
-                    fieldName = null;
-                } else if (currentToken == XContentParser.Token.END_OBJECT || currentToken == XContentParser.Token.END_ARRAY) {
-                    // Remove the path, if the scope type matches the one when the path was added.
-                    if (isObjectInPath.isEmpty() == false
-                        && (isObjectInPath.get(isObjectInPath.size() - 1) && currentToken == XContentParser.Token.END_OBJECT
-                            || isObjectInPath.get(isObjectInPath.size() - 1) == false && currentToken == XContentParser.Token.END_ARRAY)) {
-                        path.remove(path.size() - 1);
-                        isObjectInPath.remove(isObjectInPath.size() - 1);
-                    }
-                    fieldName = null;
-                }
-                currentToken = parser.nextToken();
-                if (currentToken == null) {
-                    return result;
-                }
-            }
-            fieldName = parser.currentName();
-            String fullName = getCurrentPath(path, fieldName);
-            var leaf = fields.get(fullName);  // There may be multiple matches for array elements, don't use #remove.
-            if (leaf != null) {
-                parser.nextToken();  // Advance the parser to the value to be read.
-                result.add(leaf.cloneWithValue(context.encodeFlattenedToken()));
-                fieldName = null;
-            }
-            currentToken = parser.nextToken();
-        }
-        return result;
-    }
-
-    private static String getCurrentPath(List<String> path, String fieldName) {
-        assert fieldName != null;
-        return path.isEmpty() ? fieldName : String.join(".", path) + "." + fieldName;
-    }
-
-    /**
-     * Generates all possible parent object names for the given full names.
-     * For instance, for input ['path.to.foo', 'another.path.to.bar'], it returns:
-     * [ 'path', 'path.to', 'another', 'another.path', 'another.path.to' ]
-     */
-    private static Set<String> getPossibleParentNames(Set<String> fullPaths) {
-        if (fullPaths.isEmpty()) {
-            return Collections.emptySet();
-        }
-        Set<String> paths = new HashSet<>();
-        for (String fullPath : fullPaths) {
-            String[] split = fullPath.split("\\.");
-            if (split.length < 2) {
-                continue;
-            }
-            StringBuilder builder = new StringBuilder(split[0]);
-            paths.add(builder.toString());
-            for (int i = 1; i < split.length - 1; i++) {
-                builder.append(".");
-                builder.append(split[i]);
-                paths.add(builder.toString());
-            }
-        }
-        return paths;
-    }
-
     private static void executeIndexTimeScripts(DocumentParserContext context) {
         List<FieldMapper> indexTimeScriptMappers = context.mappingLookup().indexTimeScriptMappers();
         if (indexTimeScriptMappers.isEmpty()) {
@@ -426,7 +298,10 @@ static void parseObjectOrNested(DocumentParserContext context) throws IOExceptio
             throwOnConcreteValue(context.parent(), currentFieldName, context);
         }
 
-        if (context.canAddIgnoredField() && getSourceKeepMode(context, context.parent().sourceKeepMode()) == Mapper.SourceKeepMode.ALL) {
+        var sourceKeepMode = getSourceKeepMode(context, context.parent().sourceKeepMode());
+        if (context.canAddIgnoredField()
+            && (sourceKeepMode == Mapper.SourceKeepMode.ALL
+                || (sourceKeepMode == Mapper.SourceKeepMode.ARRAYS && context.inArrayScope()))) {
             context = context.addIgnoredFieldFromContext(
                 new IgnoredSourceFieldMapper.NameValue(
                     context.parent().fullPath(),
@@ -571,9 +446,11 @@ static void parseObjectOrField(DocumentParserContext context, Mapper mapper) thr
                 parseObjectOrNested(context.createFlattenContext(currentFieldName));
                 context.path().add(currentFieldName);
             } else {
+                var sourceKeepMode = getSourceKeepMode(context, fieldMapper.sourceKeepMode());
                 if (context.canAddIgnoredField()
                     && (fieldMapper.syntheticSourceMode() == FieldMapper.SyntheticSourceMode.FALLBACK
-                        || getSourceKeepMode(context, fieldMapper.sourceKeepMode()) == Mapper.SourceKeepMode.ALL
+                        || sourceKeepMode == Mapper.SourceKeepMode.ALL
+                        || (sourceKeepMode == Mapper.SourceKeepMode.ARRAYS && context.inArrayScope())
                         || (context.isWithinCopyTo() == false && context.isCopyToDestinationField(mapper.fullPath())))) {
                     context = context.addIgnoredFieldFromContext(
                         IgnoredSourceFieldMapper.NameValue.fromContext(context, fieldMapper.fullPath(), null)
@@ -811,9 +688,7 @@ private static void parseNonDynamicArray(
             if (mapper instanceof ObjectMapper objectMapper) {
                 mode = getSourceKeepMode(context, objectMapper.sourceKeepMode());
                 objectWithFallbackSyntheticSource = mode == Mapper.SourceKeepMode.ALL
-                    // Inside nested objects we always store object arrays as a workaround for #115261.
-                    || ((context.inNestedScope() || mode == Mapper.SourceKeepMode.ARRAYS)
-                        && objectMapper instanceof NestedObjectMapper == false);
+                    || (mode == Mapper.SourceKeepMode.ARRAYS && objectMapper instanceof NestedObjectMapper == false);
             }
             boolean fieldWithFallbackSyntheticSource = false;
             boolean fieldWithStoredArraySource = false;