Browse Source

Revert "Simplify adding dynamic sub-fields to their dynamic parent object (#87866)"

This reverts commit 73b0273f4d83797fd364405bd2c420c7aee323bd (#87866) as an unexpected percolator test failure has surfaced, which did not surface as part of the PR tests run.
Luca Cavanna 3 years ago
parent
commit
312ddd6b6b

+ 12 - 12
server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java

@@ -690,26 +690,26 @@ public final class DocumentParser {
     // find what the dynamic setting is given the current parse context and parent
     private static ObjectMapper.Dynamic dynamicOrDefault(ObjectMapper parentMapper, DocumentParserContext context) {
         ObjectMapper.Dynamic dynamic = parentMapper.dynamic();
-        String parentName = parentMapper.name();
         while (dynamic == null) {
-            int lastDotNdx = parentName.lastIndexOf('.');
+            int lastDotNdx = parentMapper.name().lastIndexOf('.');
             if (lastDotNdx == -1) {
-                // no dot means that the parent is the root
-                return context.root().dynamic() == null ? ObjectMapper.Dynamic.TRUE : context.root().dynamic();
+                // no dot means we the parent is the root, so just delegate to the default outside the loop
+                break;
             }
-            parentName = parentMapper.name().substring(0, lastDotNdx);
-            ObjectMapper parent = context.mappingLookup().objectMappers().get(parentName);
-            if (parent == null) {
+            String parentName = parentMapper.name().substring(0, lastDotNdx);
+            parentMapper = context.mappingLookup().objectMappers().get(parentName);
+            if (parentMapper == null) {
                 // If parentMapper is null, it means the parent of the current mapper is being dynamically created right now
-                ObjectMapper.Builder dynamicObjectMapperBuilder = context.getDynamicObjectMapperBuilder(parentName);
-                if (dynamicObjectMapperBuilder == null) {
+                parentMapper = context.getDynamicObjectMapper(parentName);
+                if (parentMapper == null) {
                     // it can still happen that the path is ambiguous and we are not able to locate the parent
                     break;
                 }
-                dynamic = dynamicObjectMapperBuilder.dynamic;
-            } else {
-                dynamic = parent.dynamic();
             }
+            dynamic = parentMapper.dynamic();
+        }
+        if (dynamic == null) {
+            return context.root().dynamic() == null ? ObjectMapper.Dynamic.TRUE : context.root().dynamic();
         }
         return dynamic;
     }

+ 5 - 5
server/src/main/java/org/elasticsearch/index/mapper/DocumentParserContext.java

@@ -86,7 +86,7 @@ public abstract class DocumentParserContext {
     private final Set<String> ignoredFields;
     private final 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 String id;
@@ -235,7 +235,7 @@ public abstract class DocumentParserContext {
             mappingLookup.checkFieldLimit(indexSettings().getMappingTotalFieldsLimit(), newFieldsSeen.size());
         }
         if (mapper instanceof ObjectMapper objectMapper) {
-            dynamicObjectMappers.put(objectMapper.name(), objectMapper.newBuilder(this.indexSettings.getIndexVersionCreated()));
+            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 submapper : objectMapper.mappers.values()) {
@@ -265,13 +265,13 @@ public abstract class DocumentParserContext {
     }
 
     /**
-     * Get a dynamic object mapper builder by name. Allows consumers to lookup objects that have been dynamically added as a result
+     * Get a dynamic object mapper by name. Allows consumers to lookup objects that have been dynamically added as a result
      * of parsing an incoming document. Used to find the parent object for new fields that are being dynamically mapped whose parent is
      * also not mapped yet. Such new fields will need to be dynamically added to their parent according to its dynamic behaviour.
-     * Holds a flat set of object builders, meaning that an object field named <code>foo.bar</code> can be looked up directly with its
+     * 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 getDynamicObjectMapperBuilder(String name) {
+    final ObjectMapper getDynamicObjectMapper(String name) {
         return dynamicObjectMappers.get(name);
     }
 

+ 4 - 4
server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java

@@ -136,11 +136,11 @@ public class ObjectMapper extends Mapper implements Cloneable {
                 return objectMapper.newBuilder(context.indexSettings().getIndexVersionCreated());
             }
             // has the object mapper been added as a dynamic update already?
-            Builder dynamicObjectMapperBuilder = context.getDynamicObjectMapperBuilder(fullName);
-            if (dynamicObjectMapperBuilder == null) {
-                throw new IllegalStateException("Missing intermediate object " + fullName);
+            objectMapper = context.getDynamicObjectMapper(fullName);
+            if (objectMapper != null) {
+                return objectMapper.newBuilder(context.indexSettings().getIndexVersionCreated());
             }
-            return dynamicObjectMapperBuilder;
+            throw new IllegalStateException("Missing intermediate object " + fullName);
         }
 
         protected final Map<String, Mapper> buildMappers(boolean root, MapperBuilderContext context) {