Revert change (elastic#90674) (elastic#103865)

Reverts elastic#90674

The revert is not perfectly clean as there are some minor adjustments to
account for later changes.

This is in contrast with:
elastic#103858

closes: elastic#103011

docs/changelog/103865.yaml

@@ -0,0 +1,5 @@
+pr: 103865
+summary: Revert change
+area: Mapping
+type: bug
+issues: []

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

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

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

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();

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