Handle runtime subfields when shadowing dynamic mappings (#75595)

In #75454 we changed our dynamic shadowing logic to check that an unmapped
field was truly shadowed by a runtime field before returning no-op mappers. However,
this does not handle the case where the runtime field can have multiple subfields, as
will be true for the upcoming composite field type. We instead need to check that
the field in question would not be shadowed by any field type returned by any
runtime field.

This commit abstracts this logic into a new isShadowed() method on
DocumentParserContext, which uses a set of runtime field type names built from
the mapping lookup at construction time. It also simplifies the no-op mapper
slightly by making it a singleton object, as we don't need to preserve field names
here.

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

@@ -64,7 +64,8 @@ public final class DocumentParser {
 
     /**
      * Parse a document
-     * @param source the document to parse
+     *
+     * @param source        the document to parse
      * @param mappingLookup the mappings information needed to parse the document
      * @return the parsed document
      * @throws MapperParsingException whenever there's a problem parsing the document
@@ -160,6 +161,7 @@ private static void executeIndexTimeScripts(DocumentParserContext context) {
         Map<String, Consumer<LeafReaderContext>> fieldScripts = new HashMap<>();
         indexTimeScriptMappers.forEach(mapper -> fieldScripts.put(mapper.name(), new Consumer<>() {
             boolean executed = false;
+
             @Override
             public void accept(LeafReaderContext leafReaderContext) {
                 if (executed == false) {
@@ -233,22 +235,24 @@ private static String[] splitAndValidatePath(String fullFieldPath) {
                     // check if the field name contains only whitespace
                     if (Strings.isEmpty(part) == false) {
                         throw new IllegalArgumentException(
-                                "object field cannot contain only whitespace: ['" + fullFieldPath + "']");
+                            "object field cannot contain only whitespace: ['" + fullFieldPath + "']");
                     }
                     throw new IllegalArgumentException(
-                            "object field starting or ending with a [.] makes object resolution ambiguous: [" + fullFieldPath + "]");
+                        "object field starting or ending with a [.] makes object resolution ambiguous: [" + fullFieldPath + "]");
                 }
             }
             return parts;
         } else {
             if (Strings.isEmpty(fullFieldPath)) {
                 throw new IllegalArgumentException("field name cannot be an empty string");
             }
-            return new String[] {fullFieldPath};
+            return new String[]{fullFieldPath};
         }
     }
 
-    /** Creates a Mapping containing any dynamically added fields, or returns null if there were no dynamic mappings. */
+    /**
+     * Creates a Mapping containing any dynamically added fields, or returns null if there were no dynamic mappings.
+     */
     static Mapping createDynamicUpdate(MappingLookup mappingLookup,
                                        List<Mapper> dynamicMappers,
                                        List<RuntimeField> dynamicRuntimeFields) {
@@ -319,7 +323,7 @@ private static RootObjectMapper createDynamicUpdate(MappingLookup mappingLookup,
         }
         popMappers(parentMappers, 1, true);
         assert parentMappers.size() == 1;
-        return (RootObjectMapper)parentMappers.get(0);
+        return (RootObjectMapper) parentMappers.get(0);
     }
 
     private static void popMappers(List<ObjectMapper> parentMappers, int keepBefore, boolean merge) {
@@ -375,7 +379,9 @@ private static int expandCommonMappers(List<ObjectMapper> parentMappers, String[
         return i;
     }
 
-    /** Creates an update for intermediate object mappers that are not on the stack, but parents of newMapper. */
+    /**
+     * Creates an update for intermediate object mappers that are not on the stack, but parents of newMapper.
+     */
     private static ObjectMapper createExistingMapperUpdate(List<ObjectMapper> parentMappers, String[] nameParts, int i,
                                                            MappingLookup mappingLookup, Mapper newMapper) {
         String updateParentName = nameParts[i];
@@ -389,16 +395,18 @@ private static ObjectMapper createExistingMapperUpdate(List<ObjectMapper> parent
         return createUpdate(updateParent, nameParts, i + 1, newMapper);
     }
 
-    /** Build an update for the parent which will contain the given mapper and any intermediate fields. */
+    /**
+     * Build an update for the parent which will contain the given mapper and any intermediate fields.
+     */
     private static ObjectMapper createUpdate(ObjectMapper parent, String[] nameParts, int i, Mapper mapper) {
         List<ObjectMapper> parentMappers = new ArrayList<>();
         ObjectMapper previousIntermediate = parent;
         for (; i < nameParts.length - 1; ++i) {
             Mapper intermediate = previousIntermediate.getMapper(nameParts[i]);
             assert intermediate != null : "Field " + previousIntermediate.name() + " does not have a subfield " + nameParts[i];
             assert intermediate instanceof ObjectMapper;
-            parentMappers.add((ObjectMapper)intermediate);
-            previousIntermediate = (ObjectMapper)intermediate;
+            parentMappers.add((ObjectMapper) intermediate);
+            previousIntermediate = (ObjectMapper) intermediate;
         }
         if (parentMappers.isEmpty() == false) {
             // add the new mapper to the stack, and pop down to the original parent level
@@ -609,7 +617,7 @@ private static void parseArray(DocumentParserContext context, ObjectMapper paren
             ObjectMapper.Dynamic dynamic = dynamicOrDefault(parentMapper, context);
             if (dynamic == ObjectMapper.Dynamic.STRICT) {
                 throw new StrictDynamicMappingException(parentMapper.fullPath(), arrayFieldName);
-            } else if (dynamic == ObjectMapper.Dynamic.FALSE)  {
+            } else if (dynamic == ObjectMapper.Dynamic.FALSE) {
                 // TODO: shouldn't this skip, not parse?
                 parseNonDynamicArray(context, parentMapper, lastFieldName, arrayFieldName);
             } else {
@@ -705,7 +713,9 @@ private static void parseDynamicValue(final DocumentParserContext context, Objec
         dynamic.getDynamicFieldsBuilder().createDynamicFieldFromValue(context, token, currentFieldName);
     }
 
-    /** Creates instances of the fields that the current field should be copied to */
+    /**
+     * Creates instances of the fields that the current field should be copied to
+     */
     private static void parseCopyFields(DocumentParserContext context, List<String> copyToFields) throws IOException {
         context = context.createCopyToContext();
         for (String field : copyToFields) {
@@ -729,7 +739,9 @@ private static void parseCopyFields(DocumentParserContext context, List<String>
         }
     }
 
-    /** Creates an copy of the current field with given field name and boost */
+    /**
+     * Creates an copy of the current field with given field name and boost
+     */
     private static void parseCopy(String field, DocumentParserContext context) throws IOException {
         Mapper mapper = context.mappingLookup().getMapper(field);
         if (mapper != null) {
@@ -746,7 +758,7 @@ private static void parseCopy(String field, DocumentParserContext context) throw
             context = context.overridePath(new ContentPath(0));
 
             final String[] paths = splitAndValidatePath(field);
-            final String fieldName = paths[paths.length-1];
+            final String fieldName = paths[paths.length - 1];
             Tuple<Integer, ObjectMapper> parentMapperTuple = getDynamicParentMapper(context, paths, null);
             ObjectMapper objectMapper = parentMapperTuple.v2();
             parseDynamicValue(context, objectMapper, fieldName, context.parser().currentToken());
@@ -761,7 +773,7 @@ private static Tuple<Integer, ObjectMapper> getDynamicParentMapper(DocumentParse
         ObjectMapper mapper = currentParent == null ? context.root() : currentParent;
         int pathsAdded = 0;
         ObjectMapper parent = mapper;
-        for (int i = 0; i < paths.length-1; i++) {
+        for (int i = 0; i < paths.length - 1; i++) {
             String name = paths[i];
             String currentPath = context.path().pathAsText(name);
             Mapper existingFieldMapper = context.mappingLookup().getMapper(currentPath);
@@ -780,7 +792,7 @@ private static Tuple<Integer, ObjectMapper> getDynamicParentMapper(DocumentParse
                     // Should not dynamically create any more mappers so return the last mapper
                     return new Tuple<>(pathsAdded, parent);
                 } else if (dynamic == ObjectMapper.Dynamic.RUNTIME) {
-                        mapper = new NoOpObjectMapper(name, currentPath);
+                    mapper = new NoOpObjectMapper(name, currentPath);
                 } else {
                     final Mapper fieldMapper = dynamic.getDynamicFieldsBuilder().createDynamicObjectMapper(context, name);
                     if (fieldMapper instanceof ObjectMapper == false) {
@@ -852,11 +864,11 @@ private static Mapper getMapper(final DocumentParserContext context,
             if (mapper instanceof ObjectMapper == false) {
                 return null;
             }
-            objectMapper = (ObjectMapper)mapper;
+            objectMapper = (ObjectMapper) mapper;
             if (objectMapper.isNested()) {
                 throw new MapperParsingException("Cannot add a value for field ["
-                        + fieldName + "] since one of the intermediate objects is mapped as a nested object: ["
-                        + mapper.name() + "]");
+                    + fieldName + "] since one of the intermediate objects is mapped as a nested object: ["
+                    + mapper.name() + "]");
             }
         }
         String leafName = subfields[subfields.length - 1];
@@ -878,36 +890,33 @@ private static Mapper getLeafMapper(final DocumentParserContext context,
         // if a leaf field is not mapped, and is defined as a runtime field, then we
         // don't create a dynamic mapping for it and don't index it.
         String fieldPath = context.path().pathAsText(fieldName);
-        MappedFieldType fieldType = context.mappingLookup().getFieldType(fieldPath);
-        if (fieldType != null) {
-            RuntimeField runtimeField = context.root().getRuntimeField(fieldPath);
-            if (runtimeField != null) {
-                assert fieldType.hasDocValues() == false && fieldType.isAggregatable() && fieldType.isSearchable();
-                return new NoOpFieldMapper(subfields[subfields.length - 1], fieldType.name());
-            }
+        if (context.isShadowed(fieldPath)) {
+            return NO_OP_FIELDMAPPER;
         }
         return null;
     }
 
-    private static class NoOpFieldMapper extends FieldMapper {
-        NoOpFieldMapper(String simpleName, String fullName) {
-            super(simpleName, new MappedFieldType(fullName, false, false, false, TextSearchInfo.NONE, Collections.emptyMap()) {
-                @Override
-                public ValueFetcher valueFetcher(SearchExecutionContext context, String format) {
-                    throw new UnsupportedOperationException();
-                }
+    private static final FieldMapper NO_OP_FIELDMAPPER = new FieldMapper(
+        "no-op",
+        new MappedFieldType("no-op", false, false, false, TextSearchInfo.NONE, Collections.emptyMap()) {
+            @Override
+            public ValueFetcher valueFetcher(SearchExecutionContext context, String format) {
+                throw new UnsupportedOperationException();
+            }
 
-                @Override
-                public String typeName() {
-                    throw new UnsupportedOperationException();
-                }
+            @Override
+            public String typeName() {
+                throw new UnsupportedOperationException();
+            }
 
-                @Override
-                public Query termQuery(Object value, SearchExecutionContext context) {
-                    throw new UnsupportedOperationException();
-                }
-            }, MultiFields.empty(), CopyTo.empty());
-        }
+            @Override
+            public Query termQuery(Object value, SearchExecutionContext context) {
+                throw new UnsupportedOperationException();
+            }
+        },
+        FieldMapper.MultiFields.empty(),
+        FieldMapper.CopyTo.empty()
+    ) {
 
         @Override
         protected void parseCreateField(DocumentParserContext context) throws IOException {
@@ -963,7 +972,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
         protected String contentType() {
             throw new UnsupportedOperationException();
         }
-    }
+    };
 
     private static class NoOpObjectMapper extends ObjectMapper {
         NoOpObjectMapper(String name, String fullPath) {

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

@@ -92,6 +92,7 @@ protected void addDoc(LuceneDocument doc) {
     private final Set<String> newFieldsSeen;
     private final Map<String, ObjectMapper> dynamicObjectMappers;
     private final List<RuntimeField> dynamicRuntimeFields;
+    private final Set<String> shadowedFields;
     private Field version;
     private SeqNoFieldMapper.SequenceIDFields seqID;
 
@@ -107,6 +108,7 @@ private DocumentParserContext(DocumentParserContext in) {
         this.newFieldsSeen = in.newFieldsSeen;
         this.dynamicObjectMappers = in.dynamicObjectMappers;
         this.dynamicRuntimeFields = in.dynamicRuntimeFields;
+        this.shadowedFields = in.shadowedFields;
         this.version = in.version;
         this.seqID = in.seqID;
     }
@@ -127,6 +129,17 @@ protected DocumentParserContext(MappingLookup mappingLookup,
         this.newFieldsSeen = new HashSet<>();
         this.dynamicObjectMappers = new HashMap<>();
         this.dynamicRuntimeFields = new ArrayList<>();
+        this.shadowedFields = buildShadowedFields(mappingLookup);
+    }
+
+    private static Set<String> buildShadowedFields(MappingLookup lookup) {
+        Set<String> shadowedFields = new HashSet<>();
+        for (RuntimeField runtimeField : lookup.getMapping().getRoot().runtimeFields()) {
+            for (MappedFieldType mft : runtimeField.asMappedFieldTypes()) {
+                shadowedFields.add(mft.name());
+            }
+        }
+        return shadowedFields;
     }
 
     public final IndexSettings indexSettings() {
@@ -229,6 +242,10 @@ public final List<Mapper> getDynamicMappers() {
         return dynamicMappers;
     }
 
+    public final boolean isShadowed(String field) {
+        return shadowedFields.contains(field);
+    }
+
     public final ObjectMapper getObjectMapper(String name) {
         return dynamicObjectMappers.get(name);
     }

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

@@ -308,7 +308,7 @@ public void testDualingQueries() throws IOException {
             String source = "{\"foo\": " + values + "}";
             XContentParser parser = createParser(JsonXContent.jsonXContent, source);
             SourceToParse sourceToParse = new SourceToParse("test", "test", new BytesArray(source), XContentType.JSON);
-            DocumentParserContext ctx = new TestDocumentParserContext(null, null, null, null, sourceToParse) {
+            DocumentParserContext ctx = new TestDocumentParserContext(MappingLookup.EMPTY, null, null, null, sourceToParse) {
                 @Override
                 public XContentParser parser() {
                     return parser;

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

@@ -1928,6 +1928,35 @@ public void testParseWithFlattenedField() throws IOException {
         assertNull(doc.rootDoc().getField("field.second"));
     }
 
+    public void testDynamicShadowingOfRuntimeSubfields() throws IOException {
+
+        // Create mappings with a runtime field called 'obj' that produces two subfields,
+        // 'obj.foo' and 'obj.bar'
+
+        DocumentMapper mapper = createDocumentMapper(topMapping(b -> {
+            b.startObject("runtime");
+            b.startObject("obj").field("type", "test-composite").endObject();
+            b.endObject();
+        }));
+
+        // Incoming documents should not create mappings for 'obj.foo' fields, as they will
+        // be shadowed by the runtime fields; but other subfields are fine and should be
+        // indexed
+
+        ParsedDocument doc = mapper.parse(source(b -> {
+            b.startObject("obj");
+            b.field("foo", "ignored");
+            b.field("baz", "indexed");
+            b.field("bar", "ignored");
+            b.endObject();
+        }));
+
+        assertNull(doc.rootDoc().getField("obj.foo"));
+        assertNotNull(doc.rootDoc().getField("obj.baz"));
+        assertNull(doc.rootDoc().getField("obj.bar"));
+        assertNotNull(doc.dynamicMappingsUpdate());
+    }
+
     /**
      * Mapper plugin providing a mock metadata field mapper implementation that supports setting its value
      * as well as a mock runtime field parser.
@@ -1965,5 +1994,21 @@ protected String contentType() {
         public Map<String, MetadataFieldMapper.TypeParser> getMetadataMappers() {
             return Collections.singletonMap(MockMetadataMapper.CONTENT_TYPE, MockMetadataMapper.PARSER);
         }
+
+        @Override
+        public Map<String, RuntimeField.Parser> getRuntimeFields() {
+            return Collections.singletonMap(
+                "test-composite",
+                new RuntimeField.Parser(n -> new RuntimeField.Builder(n) {
+                    @Override
+                    protected RuntimeField createRuntimeField(MappingParserContext parserContext) {
+                        return new TestRuntimeField(n, List.of(
+                            new KeywordFieldMapper.KeywordFieldType(n + ".foo"),
+                            new KeywordFieldMapper.KeywordFieldType(n + ".bar")
+                        ));
+                    }
+                })
+            );
+        }
     }
 }

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

@@ -17,6 +17,9 @@
 import java.util.Collections;
 
 public final class TestRuntimeField implements RuntimeField {
+
+    public static final String CONTENT_TYPE = "test-composite";
+
     private final String name;
     private final Collection<MappedFieldType> subfields;
 
@@ -41,7 +44,10 @@ public Collection<MappedFieldType> asMappedFieldTypes() {
 
     @Override
     public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
-        throw new UnsupportedOperationException();
+        builder.startObject(name);
+        builder.field("type", CONTENT_TYPE);
+        builder.endObject();
+        return builder;
     }
 
     public static class TestRuntimeFieldType extends MappedFieldType {

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

@@ -31,7 +31,7 @@ public class TestDocumentParserContext extends DocumentParserContext {
      * Use with caution as it can cause {@link NullPointerException}s down the line.
      */
     public TestDocumentParserContext() {
-        super(null, null, null, null, null);
+        super(MappingLookup.EMPTY, null, null, null, null);
     }
 
     /**