elastic · elasticsearchmachine · Jan 4, 2024 · Jan 4, 2024
diff --git a/docs/changelog/103865.yaml b/docs/changelog/103865.yaml
@@ -0,0 +1,5 @@
+pr: 103865
+summary: Revert change
+area: Mapping
+type: bug
+issues: []
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java b/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java
@@ -251,8 +251,8 @@ static Mapping createDynamicUpdate(DocumentParserContext context) {
             return null;
         }
         RootObjectMapper.Builder rootBuilder = context.updateRoot();
-        context.getDynamicMappers()
-            .forEach((name, builders) -> builders.forEach(builder -> rootBuilder.addDynamic(name, null, builder, context)));
+        context.getDynamicMappers().forEach(mapper -> rootBuilder.addDynamic(mapper.name(), null, mapper, context));
+
         for (RuntimeField runtimeField : context.getDynamicRuntimeFields()) {
             rootBuilder.addRuntimeField(runtimeField);
         }
@@ -485,20 +485,13 @@ 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 {
-                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());
+                dynamicObjectMapper = DynamicFieldsBuilder.createDynamicObjectMapper(context, currentFieldName);
             }
             if (context.parent().subobjects() == false) {
                 if (dynamicObjectMapper instanceof NestedObjectMapper) {
@@ -520,8 +513,8 @@ private static void parseObjectDynamic(DocumentParserContext context, String cur
                 }
 
             }
-            if (context.dynamic() != ObjectMapper.Dynamic.RUNTIME && dynamicObjectBuilder != null) {
-                context.addDynamicMapper(dynamicObjectMapper.name(), dynamicObjectBuilder);
+            if (context.dynamic() != ObjectMapper.Dynamic.RUNTIME) {
+                context.addDynamicMapper(dynamicObjectMapper);
             }
             if (dynamicObjectMapper instanceof NestedObjectMapper && context.isWithinCopyTo()) {
                 throwOnCreateDynamicNestedViaCopyTo(dynamicObjectMapper, context);
@@ -558,13 +551,12 @@ private static void parseArrayDynamic(DocumentParserContext context, String curr
         if (context.dynamic() == ObjectMapper.Dynamic.FALSE) {
             context.parser().skipChildren();
         } else {
-            Mapper.Builder objectBuilderFromTemplate = DynamicFieldsBuilder.findTemplateBuilderForObject(context, currentFieldName);
-            if (objectBuilderFromTemplate == null) {
+            Mapper objectMapperFromTemplate = DynamicFieldsBuilder.createObjectMapperFromTemplate(context, currentFieldName);
+            if (objectMapperFromTemplate == null) {
                 parseNonDynamicArray(context, currentFieldName, currentFieldName);
             } else {
-                Mapper objectMapperFromTemplate = objectBuilderFromTemplate.build(context.createDynamicMapperBuilderContext());
                 if (parsesArrayValue(objectMapperFromTemplate)) {
-                    context.addDynamicMapper(objectMapperFromTemplate.name(), objectBuilderFromTemplate);
+                    context.addDynamicMapper(objectMapperFromTemplate);
                     context.path().add(currentFieldName);
                     parseObjectOrField(context, objectMapperFromTemplate);
                     context.path().remove();
@@ -607,7 +599,7 @@ 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.Builder> mappers = context.getDynamicMappers(fullFieldName);
+            final List<Mapper> mappers = context.getDynamicMappers(fullFieldName);
             if (mappers == null
                 || context.isFieldAppliedFromTemplate(fullFieldName)
                 || context.isCopyToField(fullFieldName)
@@ -616,8 +608,7 @@ private static void postProcessDynamicArrayMapping(DocumentParserContext context
                 // Anything that is NOT a number or anything that IS a number but not mapped to `float` should NOT be mapped to dense_vector
                 || mappers.stream()
                     .anyMatch(
-                        m -> m instanceof NumberFieldMapper.Builder == false
-                            || ((NumberFieldMapper.Builder) m).type != NumberFieldMapper.NumberType.FLOAT
+                        m -> m instanceof NumberFieldMapper == false || ((NumberFieldMapper) m).type() != NumberFieldMapper.NumberType.FLOAT
                     )) {
                 return;
             }
@@ -626,7 +617,8 @@ private static void postProcessDynamicArrayMapping(DocumentParserContext context
                 fieldName,
                 context.indexSettings().getIndexVersionCreated()
             );
-            context.updateDynamicMappers(fullFieldName, builder);
+            DenseVectorFieldMapper denseVectorFieldMapper = builder.build(builderContext);
+            context.updateDynamicMappers(fullFieldName, List.of(denseVectorFieldMapper));
         }
     }
 

diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DocumentParserContext.java b/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.Builder>> dynamicMappers;
+    private final Map<String, List<Mapper>> dynamicMappers;
     private final Set<String> newFieldsSeen;
-    private final Map<String, ObjectMapper.Builder> dynamicObjectMappers;
+    private final Map<String, ObjectMapper> 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.Builder>> dynamicMappers,
+        Map<String, List<Mapper>> dynamicMappers,
         Set<String> newFieldsSeen,
-        Map<String, ObjectMapper.Builder> dynamicObjectMappers,
+        Map<String, ObjectMapper> 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(String fullName, Mapper.Builder builder) {
+    public final void addDynamicMapper(Mapper mapper) {
         // eagerly check object depth limit here to avoid stack overflow errors
-        if (builder instanceof ObjectMapper.Builder) {
-            MappingLookup.checkObjectDepthLimit(indexSettings().getMappingDepthLimit(), fullName);
+        if (mapper instanceof ObjectMapper) {
+            MappingLookup.checkObjectDepthLimit(indexSettings().getMappingDepthLimit(), mapper.name());
         }
 
         // 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(fullName) == null
-            && mappingLookup.objectMappers().containsKey(fullName) == false
-            && newFieldsSeen.add(fullName)) {
+        if (mappingLookup.getMapper(mapper.name()) == null
+            && mappingLookup.objectMappers().containsKey(mapper.name()) == false
+            && newFieldsSeen.add(mapper.name())) {
             mappingLookup.checkFieldLimit(indexSettings().getMappingTotalFieldsLimit(), newFieldsSeen.size());
         }
-        if (builder instanceof ObjectMapper.Builder objectMapper) {
-            dynamicObjectMappers.put(fullName, objectMapper);
+        if (mapper instanceof ObjectMapper objectMapper) {
+            dynamicObjectMappers.put(objectMapper.name(), 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.Builder submapper : objectMapper.subBuilders()) {
+            for (Mapper submapper : objectMapper.mappers.values()) {
                 // 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(fullName + "." + submapper.name, submapper);
+                addDynamicMapper(submapper);
             }
         }
 
@@ -336,7 +336,7 @@ public final void addDynamicMapper(String fullName, Mapper.Builder builder) {
         // 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(fullName, k -> new ArrayList<>()).add(builder);
+        dynamicMappers.computeIfAbsent(mapper.name(), k -> new ArrayList<>()).add(mapper);
     }
 
     /**
@@ -345,8 +345,8 @@ public final void addDynamicMapper(String fullName, Mapper.Builder builder) {
      * 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 Map<String, List<Mapper.Builder>> getDynamicMappers() {
-        return dynamicMappers;
+    public final List<Mapper> getDynamicMappers() {
+        return dynamicMappers.values().stream().flatMap(List::stream).toList();
     }
 
     /**
@@ -355,13 +355,13 @@ public final Map<String, List<Mapper.Builder>> getDynamicMappers() {
      * @param fieldName Full field name with dot-notation.
      * @return List of Mappers or null
      */
-    public final List<Mapper.Builder> getDynamicMappers(String fieldName) {
+    public final List<Mapper> getDynamicMappers(String fieldName) {
         return dynamicMappers.get(fieldName);
     }
 
-    public void updateDynamicMappers(String name, Mapper.Builder mapper) {
+    public void updateDynamicMappers(String name, List<Mapper> mappers) {
         dynamicMappers.remove(name);
-        dynamicMappers.put(name, List.of(mapper));
+        mappers.forEach(this::addDynamicMapper);
     }
 
     /**
@@ -371,7 +371,7 @@ public void updateDynamicMappers(String name, Mapper.Builder mapper) {
      * 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.Builder getDynamicObjectMapper(String name) {
+    final ObjectMapper getDynamicObjectMapper(String name) {
         return dynamicObjectMappers.get(name);
     }
 

diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DynamicFieldsBuilder.java b/server/src/main/java/org/elasticsearch/index/mapper/DynamicFieldsBuilder.java
@@ -155,6 +155,25 @@ 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.
@@ -234,10 +253,7 @@ private static boolean applyMatchingTemplate(
         return true;
     }
 
-    /**
-     * Returns a dynamically created object builder, based exclusively on a matching dynamic template, null otherwise.
-     */
-    static Mapper.Builder findTemplateBuilderForObject(DocumentParserContext context, String name) {
+    private static Mapper.Builder findTemplateBuilderForObject(DocumentParserContext context, String name) {
         DynamicTemplate.XContentFieldType matchType = DynamicTemplate.XContentFieldType.OBJECT;
         DynamicTemplate dynamicTemplate = context.findDynamicTemplate(name, matchType);
         if (dynamicTemplate == null) {
@@ -293,7 +309,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.name(), builder);
+            context.addDynamicMapper(mapper);
             parseField.accept(context, mapper);
         }
 

diff --git a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java
@@ -118,7 +118,7 @@ public static final class Builder extends FieldMapper.Builder {
         private final Parameter<Map<String, String>> meta = Parameter.metaParam();
 
         private final ScriptCompiler scriptCompiler;
-        public final NumberType type;
+        private final NumberType type;
 
         private boolean allowMultipleValues = true;
         private final IndexVersion indexCreatedVersion;
@@ -1817,6 +1817,10 @@ public NumberFieldType fieldType() {
         return (NumberFieldType) super.fieldType();
     }
 
+    public NumberType type() {
+        return type;
+    }
+
     @Override
     protected String contentType() {
         return fieldType().type.typeName();

diff --git a/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java
@@ -26,12 +26,10 @@
 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 {
@@ -80,7 +78,6 @@ 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);
@@ -99,27 +96,31 @@ public Builder dynamic(Dynamic dynamic) {
 
         public Builder add(Mapper.Builder builder) {
             mappersBuilders.add(builder);
-            subMapperNames.add(builder.name);
             return this;
         }
 
-        public Collection<Mapper.Builder> subBuilders() {
-            return mappersBuilders;
+        private void add(String name, Mapper mapper) {
+            add(new Mapper.Builder(name) {
+                @Override
+                public Mapper build(MapperBuilderContext context) {
+                    return mapper;
+                }
+            });
         }
 
         /**
-         * Adds a dynamically created {@link Mapper.Builder} to this builder.
+         * Adds a dynamically created {@link Mapper} 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.Builder mapper, DocumentParserContext context) {
+        public final void addDynamic(String name, String prefix, Mapper 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(mapper);
+                add(name, 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,28 +130,22 @@ public final void addDynamic(String name, String prefix, Mapper.Builder mapper,
                 int firstDotIndex = name.indexOf(".");
                 String immediateChild = name.substring(0, firstDotIndex);
                 String immediateChildFullName = prefix == null ? immediateChild : prefix + "." + immediateChild;
-                ObjectMapper.Builder parentBuilder = findObjectBuilder(immediateChild, immediateChildFullName, context);
+                ObjectMapper.Builder parentBuilder = findObjectBuilder(immediateChildFullName, context);
                 parentBuilder.addDynamic(name.substring(firstDotIndex + 1), immediateChildFullName, mapper, context);
+                add(parentBuilder);
             }
         }
 
-        private ObjectMapper.Builder findObjectBuilder(String leafName, String fullName, DocumentParserContext context) {
+        private static ObjectMapper.Builder findObjectBuilder(String fullName, DocumentParserContext context) {
             // does the object mapper already exist? if so, use that
             ObjectMapper objectMapper = context.mappingLookup().objectMappers().get(fullName);
             if (objectMapper != null) {
-                ObjectMapper.Builder builder = objectMapper.newBuilder(context.indexSettings().getIndexVersionCreated());
-                add(builder);
-                return builder;
+                return objectMapper.newBuilder(context.indexSettings().getIndexVersionCreated());
             }
             // has the object mapper been added as a dynamic update already?
-            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;
+            objectMapper = context.getDynamicObjectMapper(fullName);
+            if (objectMapper != null) {
+                return objectMapper.newBuilder(context.indexSettings().getIndexVersionCreated());
             }
             throw new IllegalStateException("Missing intermediate object " + fullName);
         }