浏览代码

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

DocumentParserContext holds all the dynamic mappers created while parsing a document, as well as dynamic object mappers on a separate map so they can be looked up by name quickly when parsing their sub-fields and looking for their parent field. For every sub-field, so far we looked up the corresponding object mapper and then call .newBuilder on it. Calling newBuilder can be done already when the object mapper is added to the map, which simplifies the logic down the line as instead of creating a new builde for each sub-field, we re-use the same builder and add sub-fields to it. This has no effect on the resulting dynamic update, but it simplifies the code and it should have a positive impact on performance.
Luca Cavanna 3 年之前
父节点
当前提交
73b0273f4d

+ 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
     // find what the dynamic setting is given the current parse context and parent
     private static ObjectMapper.Dynamic dynamicOrDefault(ObjectMapper parentMapper, DocumentParserContext context) {
     private static ObjectMapper.Dynamic dynamicOrDefault(ObjectMapper parentMapper, DocumentParserContext context) {
         ObjectMapper.Dynamic dynamic = parentMapper.dynamic();
         ObjectMapper.Dynamic dynamic = parentMapper.dynamic();
+        String parentName = parentMapper.name();
         while (dynamic == null) {
         while (dynamic == null) {
-            int lastDotNdx = parentMapper.name().lastIndexOf('.');
+            int lastDotNdx = parentName.lastIndexOf('.');
             if (lastDotNdx == -1) {
             if (lastDotNdx == -1) {
-                // no dot means we the parent is the root, so just delegate to the default outside the loop
-                break;
+                // no dot means that the parent is the root
+                return context.root().dynamic() == null ? ObjectMapper.Dynamic.TRUE : context.root().dynamic();
             }
             }
-            String parentName = parentMapper.name().substring(0, lastDotNdx);
-            parentMapper = context.mappingLookup().objectMappers().get(parentName);
-            if (parentMapper == null) {
+            parentName = parentMapper.name().substring(0, lastDotNdx);
+            ObjectMapper parent = context.mappingLookup().objectMappers().get(parentName);
+            if (parent == null) {
                 // If parentMapper is null, it means the parent of the current mapper is being dynamically created right now
                 // If parentMapper is null, it means the parent of the current mapper is being dynamically created right now
-                parentMapper = context.getDynamicObjectMapper(parentName);
-                if (parentMapper == null) {
+                ObjectMapper.Builder dynamicObjectMapperBuilder = context.getDynamicObjectMapperBuilder(parentName);
+                if (dynamicObjectMapperBuilder == null) {
                     // it can still happen that the path is ambiguous and we are not able to locate the parent
                     // it can still happen that the path is ambiguous and we are not able to locate the parent
                     break;
                     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;
         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 Set<String> ignoredFields;
     private final List<Mapper> dynamicMappers;
     private final List<Mapper> dynamicMappers;
     private final Set<String> newFieldsSeen;
     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 List<RuntimeField> dynamicRuntimeFields;
     private final DocumentDimensions dimensions;
     private final DocumentDimensions dimensions;
     private String id;
     private String id;
@@ -235,7 +235,7 @@ public abstract class DocumentParserContext {
             mappingLookup.checkFieldLimit(indexSettings().getMappingTotalFieldsLimit(), newFieldsSeen.size());
             mappingLookup.checkFieldLimit(indexSettings().getMappingTotalFieldsLimit(), newFieldsSeen.size());
         }
         }
         if (mapper instanceof ObjectMapper objectMapper) {
         if (mapper instanceof ObjectMapper objectMapper) {
-            dynamicObjectMappers.put(objectMapper.name(), objectMapper);
+            dynamicObjectMappers.put(objectMapper.name(), objectMapper.newBuilder(this.indexSettings.getIndexVersionCreated()));
             // dynamic object mappers may have been obtained from applying a dynamic template, in which case their definition may contain
             // 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
             // sub-fields as well as sub-objects that need to be added to the mappings
             for (Mapper submapper : objectMapper.mappers.values()) {
             for (Mapper submapper : objectMapper.mappers.values()) {
@@ -265,13 +265,13 @@ public abstract class DocumentParserContext {
     }
     }
 
 
     /**
     /**
-     * Get a dynamic object mapper by name. Allows consumers to lookup objects that have been dynamically added as a result
+     * Get a dynamic object mapper builder 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
      * 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.
      * 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 mappers, meaning that an object field named <code>foo.bar</code> can be looked up directly with its
+     * Holds a flat set of object builders, meaning that an object field named <code>foo.bar</code> can be looked up directly with its
      * dotted name.
      * dotted name.
      */
      */
-    final ObjectMapper getDynamicObjectMapper(String name) {
+    final ObjectMapper.Builder getDynamicObjectMapperBuilder(String name) {
         return dynamicObjectMappers.get(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());
                 return objectMapper.newBuilder(context.indexSettings().getIndexVersionCreated());
             }
             }
             // has the object mapper been added as a dynamic update already?
             // has the object mapper been added as a dynamic update already?
-            objectMapper = context.getDynamicObjectMapper(fullName);
-            if (objectMapper != null) {
-                return objectMapper.newBuilder(context.indexSettings().getIndexVersionCreated());
+            Builder dynamicObjectMapperBuilder = context.getDynamicObjectMapperBuilder(fullName);
+            if (dynamicObjectMapperBuilder == null) {
+                throw new IllegalStateException("Missing intermediate object " + fullName);
             }
             }
-            throw new IllegalStateException("Missing intermediate object " + fullName);
+            return dynamicObjectMapperBuilder;
         }
         }
 
 
         protected final Map<String, Mapper> buildMappers(boolean root, MapperBuilderContext context) {
         protected final Map<String, Mapper> buildMappers(boolean root, MapperBuilderContext context) {