Store dynamic mapping updates as builders (#90674)

Currently, newly created dynamic mappers as stored on the parser context as
instantiated mappers. This means that when we build the mapping update from
the collected dynamic mappers, we need to convert them back into builders
again, and immediately re-build them. This commit changes the context to
instead store just the builders, which results in a small simplification to the
dynamic update build logic and removes another method on ObjectMapper.Builder.

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

@@ -250,8 +250,8 @@ static Mapping createDynamicUpdate(DocumentParserContext context) {
             return null;
         }
         RootObjectMapper.Builder rootBuilder = context.updateRoot();
-        context.getDynamicMappers().forEach(mapper -> rootBuilder.addDynamic(mapper.name(), null, mapper, context));
-
+        context.getDynamicMappers()
+            .forEach((name, builders) -> builders.forEach(builder -> rootBuilder.addDynamic(name, null, builder, context)));
         for (RuntimeField runtimeField : context.getDynamicRuntimeFields()) {
             rootBuilder.addRuntimeField(runtimeField);
         }
@@ -484,13 +484,20 @@ private static void parseObjectDynamic(DocumentParserContext context, String cur
             // not dynamic, read everything up to end object
             context.parser().skipChildren();
         } else {
+            Mapper.Builder dynamicObjectBuilder = null;
             Mapper dynamicObjectMapper;
             if (context.dynamic() == ObjectMapper.Dynamic.RUNTIME) {
                 // with dynamic:runtime all leaf fields will be runtime fields unless explicitly mapped,
                 // hence we don't dynamically create empty objects under properties, but rather carry around an artificial object mapper
                 dynamicObjectMapper = new NoOpObjectMapper(currentFieldName, context.path().pathAsText(currentFieldName));
             } else {
-                dynamicObjectMapper = DynamicFieldsBuilder.createDynamicObjectMapper(context, currentFieldName);
+                dynamicObjectBuilder = DynamicFieldsBuilder.findTemplateBuilderForObject(context, currentFieldName);
+                if (dynamicObjectBuilder == null) {
+                    dynamicObjectBuilder = new ObjectMapper.Builder(currentFieldName, ObjectMapper.Defaults.SUBOBJECTS).enabled(
+                        ObjectMapper.Defaults.ENABLED
+                    );
+                }
+                dynamicObjectMapper = dynamicObjectBuilder.build(context.createDynamicMapperBuilderContext());
             }
             if (context.parent().subobjects() == false) {
                 if (dynamicObjectMapper instanceof NestedObjectMapper) {
@@ -512,8 +519,8 @@ private static void parseObjectDynamic(DocumentParserContext context, String cur
                 }
 
             }
-            if (context.dynamic() != ObjectMapper.Dynamic.RUNTIME) {
-                context.addDynamicMapper(dynamicObjectMapper);
+            if (context.dynamic() != ObjectMapper.Dynamic.RUNTIME && dynamicObjectBuilder != null) {
+                context.addDynamicMapper(dynamicObjectMapper.name(), dynamicObjectBuilder);
             }
             if (dynamicObjectMapper instanceof NestedObjectMapper && context.isWithinCopyTo()) {
                 throwOnCreateDynamicNestedViaCopyTo(dynamicObjectMapper, context);
@@ -554,12 +561,13 @@ private static void parseArray(DocumentParserContext context, String lastFieldNa
             } else if (context.dynamic() == ObjectMapper.Dynamic.FALSE) {
                 context.parser().skipChildren();
             } else {
-                Mapper objectMapperFromTemplate = DynamicFieldsBuilder.createObjectMapperFromTemplate(context, lastFieldName);
-                if (objectMapperFromTemplate == null) {
+                Mapper.Builder objectBuilderFromTemplate = DynamicFieldsBuilder.findTemplateBuilderForObject(context, lastFieldName);
+                if (objectBuilderFromTemplate == null) {
                     parseNonDynamicArray(context, lastFieldName, lastFieldName);
                 } else {
+                    Mapper objectMapperFromTemplate = objectBuilderFromTemplate.build(context.createDynamicMapperBuilderContext());
                     if (parsesArrayValue(objectMapperFromTemplate)) {
-                        context.addDynamicMapper(objectMapperFromTemplate);
+                        context.addDynamicMapper(objectMapperFromTemplate.name(), objectBuilderFromTemplate);
                         context.path().add(lastFieldName);
                         parseObjectOrField(context, objectMapperFromTemplate);
                         context.path().remove();
@@ -603,22 +611,22 @@ private static void postProcessDynamicArrayMapping(DocumentParserContext context
         if (context.indexSettings().getIndexVersionCreated().onOrAfter(DYNAMICALLY_MAP_DENSE_VECTORS_INDEX_VERSION)) {
             final MapperBuilderContext builderContext = context.createDynamicMapperBuilderContext();
             final String fullFieldName = builderContext.buildFullName(fieldName);
-            final List<Mapper> mappers = context.getDynamicMappers(fullFieldName);
+            final List<Mapper.Builder> mappers = context.getDynamicMappers(fullFieldName);
             if (mappers == null
                 || context.isFieldAppliedFromTemplate(fullFieldName)
                 || context.isCopyToField(fullFieldName)
                 || mappers.size() < MIN_DIMS_FOR_DYNAMIC_FLOAT_MAPPING
                 || mappers.size() > MAX_DIMS_COUNT
-                || mappers.stream().allMatch(m -> m instanceof NumberFieldMapper && "float".equals(m.typeName())) == false) {
+                || mappers.stream()
+                    .allMatch(m -> m instanceof NumberFieldMapper.Builder nb && nb.type != NumberFieldMapper.NumberType.FLOAT)) {
                 return;
             }
 
             DenseVectorFieldMapper.Builder builder = new DenseVectorFieldMapper.Builder(
                 fieldName,
                 context.indexSettings().getIndexVersionCreated()
             );
-            DenseVectorFieldMapper denseVectorFieldMapper = builder.build(builderContext);
-            context.updateDynamicMappers(fullFieldName, List.of(denseVectorFieldMapper));
+            context.updateDynamicMappers(fullFieldName, builder);
         }
     }
 

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

@@ -84,9 +84,9 @@ protected void addDoc(LuceneDocument doc) {
     private final MappingParserContext mappingParserContext;
     private final SourceToParse sourceToParse;
     private final Set<String> ignoredFields;
-    private final Map<String, List<Mapper>> dynamicMappers;
+    private final Map<String, List<Mapper.Builder>> dynamicMappers;
     private final Set<String> newFieldsSeen;
-    private final Map<String, ObjectMapper> dynamicObjectMappers;
+    private final Map<String, ObjectMapper.Builder> dynamicObjectMappers;
     private final List<RuntimeField> dynamicRuntimeFields;
     private final DocumentDimensions dimensions;
     private final ObjectMapper parent;
@@ -102,9 +102,9 @@ private DocumentParserContext(
         MappingParserContext mappingParserContext,
         SourceToParse sourceToParse,
         Set<String> ignoreFields,
-        Map<String, List<Mapper>> dynamicMappers,
+        Map<String, List<Mapper.Builder>> dynamicMappers,
         Set<String> newFieldsSeen,
-        Map<String, ObjectMapper> dynamicObjectMappers,
+        Map<String, ObjectMapper.Builder> dynamicObjectMappers,
         List<RuntimeField> dynamicRuntimeFields,
         String id,
         Field version,
@@ -304,29 +304,29 @@ public boolean isCopyToField(String name) {
     /**
      * Add a new mapper dynamically created while parsing.
      */
-    public final void addDynamicMapper(Mapper mapper) {
+    public final void addDynamicMapper(String fullName, Mapper.Builder builder) {
         // eagerly check object depth limit here to avoid stack overflow errors
-        if (mapper instanceof ObjectMapper) {
-            MappingLookup.checkObjectDepthLimit(indexSettings().getMappingDepthLimit(), mapper.name());
+        if (builder instanceof ObjectMapper.Builder) {
+            MappingLookup.checkObjectDepthLimit(indexSettings().getMappingDepthLimit(), fullName);
         }
 
         // eagerly check field name limit here to avoid OOM errors
         // only check fields that are not already mapped or tracked in order to avoid hitting field limit too early via double-counting
         // note that existing fields can also receive dynamic mapping updates (e.g. constant_keyword to fix the value)
-        if (mappingLookup.getMapper(mapper.name()) == null
-            && mappingLookup.objectMappers().containsKey(mapper.name()) == false
-            && newFieldsSeen.add(mapper.name())) {
+        if (mappingLookup.getMapper(fullName) == null
+            && mappingLookup.objectMappers().containsKey(fullName) == false
+            && newFieldsSeen.add(fullName)) {
             mappingLookup.checkFieldLimit(indexSettings().getMappingTotalFieldsLimit(), newFieldsSeen.size());
         }
-        if (mapper instanceof ObjectMapper objectMapper) {
-            dynamicObjectMappers.put(objectMapper.name(), objectMapper);
+        if (builder instanceof ObjectMapper.Builder objectMapper) {
+            dynamicObjectMappers.put(fullName, objectMapper);
             // dynamic object mappers may have been obtained from applying a dynamic template, in which case their definition may contain
             // sub-fields as well as sub-objects that need to be added to the mappings
-            for (Mapper submapper : objectMapper.mappers.values()) {
+            for (Mapper.Builder submapper : objectMapper.subBuilders()) {
                 // we could potentially skip the step of adding these to the dynamic mappers, because their parent is already added to
                 // that list, and what is important is that all of the intermediate objects are added to the dynamic object mappers so that
                 // they can be looked up once sub-fields need to be added to them. For simplicity, we treat these like any other object
-                addDynamicMapper(submapper);
+                addDynamicMapper(fullName + "." + submapper.name, submapper);
             }
         }
 
@@ -336,7 +336,7 @@ public final void addDynamicMapper(Mapper mapper) {
         // dynamically mapped objects when the incoming document defines no sub-fields in them:
         // 1) by default, they would be empty containers in the mappings, is it then important to map them?
         // 2) they can be the result of applying a dynamic template which may define sub-fields or set dynamic, enabled or subobjects.
-        dynamicMappers.computeIfAbsent(mapper.name(), k -> new ArrayList<>()).add(mapper);
+        dynamicMappers.computeIfAbsent(fullName, k -> new ArrayList<>()).add(builder);
     }
 
     /**
@@ -345,8 +345,8 @@ public final void addDynamicMapper(Mapper mapper) {
      * Consists of a all {@link Mapper}s that will need to be added to their respective parent {@link ObjectMapper}s in order
      * to become part of the resulting dynamic mapping update.
      */
-    public final List<Mapper> getDynamicMappers() {
-        return dynamicMappers.values().stream().flatMap(List::stream).toList();
+    public final Map<String, List<Mapper.Builder>> getDynamicMappers() {
+        return dynamicMappers;
     }
 
     /**
@@ -355,13 +355,13 @@ public final List<Mapper> getDynamicMappers() {
      * @param fieldName Full field name with dot-notation.
      * @return List of Mappers or null
      */
-    public final List<Mapper> getDynamicMappers(String fieldName) {
+    public final List<Mapper.Builder> getDynamicMappers(String fieldName) {
         return dynamicMappers.get(fieldName);
     }
 
-    public void updateDynamicMappers(String name, List<Mapper> mappers) {
+    public void updateDynamicMappers(String name, Mapper.Builder mapper) {
         dynamicMappers.remove(name);
-        mappers.forEach(this::addDynamicMapper);
+        dynamicMappers.put(name, List.of(mapper));
     }
 
     /**
@@ -371,7 +371,7 @@ public void updateDynamicMappers(String name, List<Mapper> mappers) {
      * Holds a flat set of object mappers, meaning that an object field named <code>foo.bar</code> can be looked up directly with its
      * dotted name.
      */
-    final ObjectMapper getDynamicObjectMapper(String name) {
+    final ObjectMapper.Builder getDynamicObjectMapper(String name) {
         return dynamicObjectMappers.get(name);
     }
 

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

@@ -155,25 +155,6 @@ void createDynamicFieldFromValue(final DocumentParserContext context, String nam
         }
     }
 
-    /**
-     * Returns a dynamically created object mapper, eventually based on a matching dynamic template.
-     */
-    static Mapper createDynamicObjectMapper(DocumentParserContext context, String name) {
-        Mapper mapper = createObjectMapperFromTemplate(context, name);
-        return mapper != null
-            ? mapper
-            : new ObjectMapper.Builder(name, ObjectMapper.Defaults.SUBOBJECTS).enabled(ObjectMapper.Defaults.ENABLED)
-                .build(context.createDynamicMapperBuilderContext());
-    }
-
-    /**
-     * Returns a dynamically created object mapper, based exclusively on a matching dynamic template, null otherwise.
-     */
-    static Mapper createObjectMapperFromTemplate(DocumentParserContext context, String name) {
-        Mapper.Builder templateBuilder = findTemplateBuilderForObject(context, name);
-        return templateBuilder == null ? null : templateBuilder.build(context.createDynamicMapperBuilderContext());
-    }
-
     /**
      * Creates a dynamic string field based on a matching dynamic template.
      * No field is created in case there is no matching dynamic template.
@@ -253,7 +234,10 @@ private static boolean applyMatchingTemplate(
         return true;
     }
 
-    private static Mapper.Builder findTemplateBuilderForObject(DocumentParserContext context, String name) {
+    /**
+     * Returns a dynamically created object builder, based exclusively on a matching dynamic template, null otherwise.
+     */
+    static Mapper.Builder findTemplateBuilderForObject(DocumentParserContext context, String name) {
         DynamicTemplate.XContentFieldType matchType = DynamicTemplate.XContentFieldType.OBJECT;
         DynamicTemplate dynamicTemplate = context.findDynamicTemplate(name, matchType);
         if (dynamicTemplate == null) {
@@ -309,7 +293,7 @@ private static final class Concrete implements Strategy {
 
         void createDynamicField(Mapper.Builder builder, DocumentParserContext context) throws IOException {
             Mapper mapper = builder.build(context.createDynamicMapperBuilderContext());
-            context.addDynamicMapper(mapper);
+            context.addDynamicMapper(mapper.name(), builder);
             parseField.accept(context, mapper);
         }
 

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

@@ -118,7 +118,7 @@ public static class Builder extends FieldMapper.Builder {
         private final Parameter<Map<String, String>> meta = Parameter.metaParam();
 
         private final ScriptCompiler scriptCompiler;
-        private final NumberType type;
+        public final NumberType type;
 
         private boolean allowMultipleValues = true;
         private final IndexVersion indexCreatedVersion;

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

@@ -25,10 +25,12 @@
 import java.util.Collection;
 import java.util.Comparator;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Set;
 import java.util.stream.Stream;
 
 public class ObjectMapper extends Mapper implements Cloneable {
@@ -77,6 +79,7 @@ public static class Builder extends Mapper.Builder {
         protected Explicit<Boolean> enabled = Explicit.IMPLICIT_TRUE;
         protected Dynamic dynamic;
         protected final List<Mapper.Builder> mappersBuilders = new ArrayList<>();
+        private final Set<String> subMapperNames = new HashSet<>(); // keeps track of dynamically added subfields
 
         public Builder(String name, Explicit<Boolean> subobjects) {
             super(name);
@@ -95,31 +98,27 @@ public Builder dynamic(Dynamic dynamic) {
 
         public Builder add(Mapper.Builder builder) {
             mappersBuilders.add(builder);
+            subMapperNames.add(builder.name);
             return this;
         }
 
-        private void add(String name, Mapper mapper) {
-            add(new Mapper.Builder(name) {
-                @Override
-                public Mapper build(MapperBuilderContext context) {
-                    return mapper;
-                }
-            });
+        public Collection<Mapper.Builder> subBuilders() {
+            return mappersBuilders;
         }
 
         /**
-         * Adds a dynamically created {@link Mapper} to this builder.
+         * Adds a dynamically created {@link Mapper.Builder} to this builder.
          *
          * @param name      the name of the Mapper, including object prefixes
          * @param prefix    the object prefix of this mapper
          * @param mapper    the mapper to add
          * @param context   the DocumentParserContext in which the mapper has been built
          */
-        public final void addDynamic(String name, String prefix, Mapper mapper, DocumentParserContext context) {
+        public final void addDynamic(String name, String prefix, Mapper.Builder mapper, DocumentParserContext context) {
             // If the mapper to add has no dots, or the current object mapper has subobjects set to false,
             // we just add it as it is for sure a leaf mapper
             if (name.contains(".") == false || subobjects.value() == false) {
-                add(name, mapper);
+                add(mapper);
             }
             // otherwise we strip off the first object path of the mapper name, load or create
             // the relevant object mapper, and then recurse down into it, passing the remainder
@@ -129,22 +128,28 @@ public final void addDynamic(String name, String prefix, Mapper mapper, Document
                 int firstDotIndex = name.indexOf(".");
                 String immediateChild = name.substring(0, firstDotIndex);
                 String immediateChildFullName = prefix == null ? immediateChild : prefix + "." + immediateChild;
-                ObjectMapper.Builder parentBuilder = findObjectBuilder(immediateChildFullName, context);
+                ObjectMapper.Builder parentBuilder = findObjectBuilder(immediateChild, immediateChildFullName, context);
                 parentBuilder.addDynamic(name.substring(firstDotIndex + 1), immediateChildFullName, mapper, context);
-                add(parentBuilder);
             }
         }
 
-        private static ObjectMapper.Builder findObjectBuilder(String fullName, DocumentParserContext context) {
+        private ObjectMapper.Builder findObjectBuilder(String leafName, String fullName, DocumentParserContext context) {
             // does the object mapper already exist? if so, use that
             ObjectMapper objectMapper = context.mappingLookup().objectMappers().get(fullName);
             if (objectMapper != null) {
-                return objectMapper.newBuilder(context.indexSettings().getIndexVersionCreated());
+                ObjectMapper.Builder builder = objectMapper.newBuilder(context.indexSettings().getIndexVersionCreated());
+                add(builder);
+                return builder;
             }
             // has the object mapper been added as a dynamic update already?
-            objectMapper = context.getDynamicObjectMapper(fullName);
-            if (objectMapper != null) {
-                return objectMapper.newBuilder(context.indexSettings().getIndexVersionCreated());
+            ObjectMapper.Builder builder = context.getDynamicObjectMapper(fullName);
+            if (builder != null) {
+                // we re-use builder instances so if the builder has already been
+                // added we don't need to do so again
+                if (subMapperNames.contains(leafName) == false) {
+                    add(builder);
+                }
+                return builder;
             }
             throw new IllegalStateException("Missing intermediate object " + fullName);
         }