Handle runtime subfields when shadowing dynamic mappings

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);
     }
 
     /**