Handle pass-through subfields with deep nesting (#106767)

* Handle pass-through subfields with deep nesting

* Update docs/changelog/106767.yaml

* fix comment

* test fixes

* yaml tests

* refine test

docs/changelog/106767.yaml

@@ -0,0 +1,5 @@
+pr: 106767
+summary: Handle pass-through subfields with deep nesting
+area: Mapping
+type: bug
+issues: []

modules/data-streams/src/yamlRestTest/resources/rest-api-spec/test/data_stream/150_tsdb.yml

@@ -466,13 +466,13 @@ dynamic templates with nesting:
         refresh: true
         body:
           - '{ "create": { "dynamic_templates": { "data": "counter_metric" } } }'
-          - '{ "@timestamp": "2023-09-01T13:03:08.138Z","data": "10", "resource.attributes.dim1": "A", "resource.attributes.another.dim1": "1", "attributes.dim2": "C", "attributes.another.dim2": "10.5" }'
+          - '{ "@timestamp": "2023-09-01T13:03:08.138Z","data": "10", "resource.attributes.dim1": "A", "resource.attributes.another.dim1": "1", "attributes.dim2": "C", "attributes.another.dim2": "10.5",  "attributes.a.much.deeper.nested.dim": "AC" }'
           - '{ "create": { "dynamic_templates": { "data": "counter_metric" } } }'
-          - '{ "@timestamp": "2023-09-01T13:03:09.138Z","data": "20", "resource.attributes.dim1": "A", "resource.attributes.another.dim1": "1", "attributes.dim2": "C", "attributes.another.dim2": "10.5" }'
+          - '{ "@timestamp": "2023-09-01T13:03:09.138Z","data": "20", "resource.attributes.dim1": "A", "resource.attributes.another.dim1": "1", "attributes.dim2": "C", "attributes.another.dim2": "10.5",  "attributes.a.much.deeper.nested.dim": "AC" }'
           - '{ "create": { "dynamic_templates": { "data": "counter_metric" } } }'
-          - '{ "@timestamp": "2023-09-01T13:03:10.138Z","data": "30", "resource.attributes.dim1": "B", "resource.attributes.another.dim1": "2", "attributes.dim2": "D", "attributes.another.dim2": "20.5" }'
+          - '{ "@timestamp": "2023-09-01T13:03:10.138Z","data": "30", "resource.attributes.dim1": "B", "resource.attributes.another.dim1": "2", "attributes.dim2": "D", "attributes.another.dim2": "20.5",  "attributes.a.much.deeper.nested.dim": "BD" }'
           - '{ "create": { "dynamic_templates": { "data": "counter_metric" } } }'
-          - '{ "@timestamp": "2023-09-01T13:03:10.238Z","data": "40", "resource.attributes.dim1": "B", "resource.attributes.another.dim1": "2", "attributes.dim2": "D", "attributes.another.dim2": "20.5" }'
+          - '{ "@timestamp": "2023-09-01T13:03:10.238Z","data": "40", "resource.attributes.dim1": "B", "resource.attributes.another.dim1": "2", "attributes.dim2": "D", "attributes.another.dim2": "20.5",  "attributes.a.much.deeper.nested.dim": "BD" }'
 
   - do:
       search:
@@ -498,7 +498,7 @@ dynamic templates with nesting:
                     field: _tsid
 
   - length: { aggregations.filterA.tsids.buckets: 1 }
-  - match: { aggregations.filterA.tsids.buckets.0.key: "MK0AtuFZowY4QPzoYEAZNK6pWmkqIGKYiosO9O4X2dfFL8p_4TfsFAUUYYv9EqSmEQ" }
+  - match: { aggregations.filterA.tsids.buckets.0.key: "NNnsRFDTqKogyRBhOBQclM4BkssYqVppKiBimIqLDvTuF9nXxZWMD04YHQKL09tJYL5G4yo" }
   - match: { aggregations.filterA.tsids.buckets.0.doc_count: 2 }
 
   - do:
@@ -517,7 +517,7 @@ dynamic templates with nesting:
                     field: _tsid
 
   - length: { aggregations.filterA.tsids.buckets: 1 }
-  - match: { aggregations.filterA.tsids.buckets.0.key: "MK0AtuFZowY4QPzoYEAZNK6pWmkqIGKYiosO9O4X2dfFL8p_4TfsFAUUYYv9EqSmEQ" }
+  - match: { aggregations.filterA.tsids.buckets.0.key: "NNnsRFDTqKogyRBhOBQclM4BkssYqVppKiBimIqLDvTuF9nXxZWMD04YHQKL09tJYL5G4yo" }
   - match: { aggregations.filterA.tsids.buckets.0.doc_count: 2 }
 
   - do:
@@ -536,7 +536,7 @@ dynamic templates with nesting:
                     field: _tsid
 
   - length: { aggregations.filterA.tsids.buckets: 1 }
-  - match: { aggregations.filterA.tsids.buckets.0.key: "MK0AtuFZowY4QPzoYEAZNK6pWmkqIGKYiosO9O4X2dfFL8p_4TfsFAUUYYv9EqSmEQ" }
+  - match: { aggregations.filterA.tsids.buckets.0.key: "NNnsRFDTqKogyRBhOBQclM4BkssYqVppKiBimIqLDvTuF9nXxZWMD04YHQKL09tJYL5G4yo" }
   - match: { aggregations.filterA.tsids.buckets.0.doc_count: 2 }
 
   - do:
@@ -555,7 +555,220 @@ dynamic templates with nesting:
                     field: _tsid
 
   - length: { aggregations.filterA.tsids.buckets: 1 }
-  - match: { aggregations.filterA.tsids.buckets.0.key: "MK0AtuFZowY4QPzoYEAZNK6pWmkqIGKYiosO9O4X2dfFL8p_4TfsFAUUYYv9EqSmEQ" }
+  - match: { aggregations.filterA.tsids.buckets.0.key: "NNnsRFDTqKogyRBhOBQclM4BkssYqVppKiBimIqLDvTuF9nXxZWMD04YHQKL09tJYL5G4yo" }
+  - match: { aggregations.filterA.tsids.buckets.0.doc_count: 2 }
+
+  - do:
+      search:
+        index: k9s
+        body:
+          size: 0
+          aggs:
+            filterA:
+              filter:
+                term:
+                  a.much.deeper.nested.dim: AC
+              aggs:
+                tsids:
+                  terms:
+                    field: _tsid
+
+  - length: { aggregations.filterA.tsids.buckets: 1 }
+  - match: { aggregations.filterA.tsids.buckets.0.key: "NNnsRFDTqKogyRBhOBQclM4BkssYqVppKiBimIqLDvTuF9nXxZWMD04YHQKL09tJYL5G4yo" }
+  - match: { aggregations.filterA.tsids.buckets.0.doc_count: 2 }
+
+---
+dynamic templates with incremental indexing:
+  - skip:
+      version: " - 8.12.99"
+      reason: "Support for dynamic fields was added in 8.13"
+  - do:
+      allowed_warnings:
+        - "index template [my-dynamic-template] has index patterns [k9s*] matching patterns from existing older templates [global] with patterns (global => [*]); this template [my-dynamic-template] will take precedence during new index creation"
+      indices.put_index_template:
+        name: my-dynamic-template
+        body:
+          index_patterns: [k9s*]
+          data_stream: {}
+          template:
+            settings:
+              index:
+                number_of_shards: 1
+                mode: time_series
+                time_series:
+                  start_time: 2023-08-31T13:03:08.138Z
+
+            mappings:
+              properties:
+                attributes:
+                  type: passthrough
+                  dynamic: true
+                  time_series_dimension: true
+                resource:
+                  type: object
+                  properties:
+                    attributes:
+                      type: passthrough
+                      dynamic: true
+                      time_series_dimension: true
+              dynamic_templates:
+                - counter_metric:
+                    mapping:
+                      type: integer
+                      time_series_metric: counter
+
+  - do:
+      bulk:
+        index: k9s
+        refresh: true
+        body:
+          - '{ "create": { "dynamic_templates": { "data": "counter_metric" } } }'
+          - '{ "@timestamp": "2023-09-01T13:03:08.138Z","data": "10", "resource.attributes.dim1": "A", "attributes.dim2": "C" }'
+          - '{ "create": { "dynamic_templates": { "data": "counter_metric" } } }'
+          - '{ "@timestamp": "2023-09-01T13:03:09.138Z","data": "20", "resource.attributes.dim1": "A", "attributes.dim2": "C" }'
+          - '{ "create": { "dynamic_templates": { "data": "counter_metric" } } }'
+          - '{ "@timestamp": "2023-09-01T13:03:10.138Z","data": "30", "resource.attributes.dim1": "B", "attributes.dim2": "D" }'
+          - '{ "create": { "dynamic_templates": { "data": "counter_metric" } } }'
+          - '{ "@timestamp": "2023-09-01T13:03:10.238Z","data": "40", "resource.attributes.dim1": "B", "attributes.dim2": "D" }'
+
+  - do:
+      bulk:
+        index: k9s
+        refresh: true
+        body:
+          - '{ "create": { "dynamic_templates": { "data": "counter_metric" } } }'
+          - '{ "@timestamp": "2023-09-01T13:04:08.138Z","data": "110", "resource.attributes.another.dim1": "1", "attributes.another.dim2": "10.5" }'
+          - '{ "create": { "dynamic_templates": { "data": "counter_metric" } } }'
+          - '{ "@timestamp": "2023-09-01T13:04:09.138Z","data": "120", "resource.attributes.another.dim1": "1", "attributes.another.dim2": "10.5" }'
+          - '{ "create": { "dynamic_templates": { "data": "counter_metric" } } }'
+          - '{ "@timestamp": "2023-09-01T13:04:10.138Z","data": "130", "resource.attributes.another.dim1": "2", "attributes.another.dim2": "20.5" }'
+          - '{ "create": { "dynamic_templates": { "data": "counter_metric" } } }'
+          - '{ "@timestamp": "2023-09-01T13:04:10.238Z","data": "140", "resource.attributes.another.dim1": "2", "attributes.another.dim2": "20.5" }'
+
+  - do:
+      bulk:
+        index: k9s
+        refresh: true
+        body:
+          - '{ "create": { "dynamic_templates": { "data": "counter_metric" } } }'
+          - '{ "@timestamp": "2023-09-01T13:05:08.138Z","data": "210", "resource.attributes.another.deeper.dim1": "1", "attributes.another.deeper.dim2": "10.5" }'
+          - '{ "create": { "dynamic_templates": { "data": "counter_metric" } } }'
+          - '{ "@timestamp": "2023-09-01T13:05:09.138Z","data": "220", "resource.attributes.another.deeper.dim1": "1", "attributes.another.deeper.dim2": "10.5" }'
+          - '{ "create": { "dynamic_templates": { "data": "counter_metric" } } }'
+          - '{ "@timestamp": "2023-09-01T13:05:10.138Z","data": "230", "resource.attributes.another.deeper.dim1": "2", "attributes.another.deeper.dim2": "20.5" }'
+          - '{ "create": { "dynamic_templates": { "data": "counter_metric" } } }'
+          - '{ "@timestamp": "2023-09-01T13:05:10.238Z","data": "240", "resource.attributes.another.deeper.dim1": "2", "attributes.another.deeper.dim2": "20.5" }'
+
+  - do:
+      bulk:
+        index: k9s
+        refresh: true
+        body:
+          - '{ "create": { "dynamic_templates": { "data": "counter_metric" } } }'
+          - '{ "@timestamp": "2023-09-01T13:06:08.138Z","data": "310",  "attributes.a.much.deeper.nested.dim": "AC" }'
+          - '{ "create": { "dynamic_templates": { "data": "counter_metric" } } }'
+          - '{ "@timestamp": "2023-09-01T13:06:09.138Z","data": "320",  "attributes.a.much.deeper.nested.dim": "AC" }'
+          - '{ "create": { "dynamic_templates": { "data": "counter_metric" } } }'
+          - '{ "@timestamp": "2023-09-01T13:06:10.138Z","data": "330",  "attributes.a.much.deeper.nested.dim": "BD" }'
+          - '{ "create": { "dynamic_templates": { "data": "counter_metric" } } }'
+          - '{ "@timestamp": "2023-09-01T13:06:10.238Z","data": "340",  "attributes.a.much.deeper.nested.dim": "BD" }'
+
+  - do:
+      search:
+        index: k9s
+        body:
+          size: 0
+
+  - match: { hits.total.value: 16 }
+
+  - do:
+      search:
+        index: k9s
+        body:
+          size: 0
+          aggs:
+            filterA:
+              filter:
+                term:
+                  dim1: A
+              aggs:
+                tsids:
+                  terms:
+                    field: _tsid
+
+  - length: { aggregations.filterA.tsids.buckets: 1 }
+  - match: { aggregations.filterA.tsids.buckets.0.doc_count: 2 }
+
+  - do:
+      search:
+        index: k9s
+        body:
+          size: 0
+          aggs:
+            filterA:
+              filter:
+                term:
+                  dim2: C
+              aggs:
+                tsids:
+                  terms:
+                    field: _tsid
+
+  - length: { aggregations.filterA.tsids.buckets: 1 }
+  - match: { aggregations.filterA.tsids.buckets.0.doc_count: 2 }
+
+  - do:
+      search:
+        index: k9s
+        body:
+          size: 0
+          aggs:
+            filterA:
+              filter:
+                term:
+                  another.deeper.dim1: 1
+              aggs:
+                tsids:
+                  terms:
+                    field: _tsid
+
+  - length: { aggregations.filterA.tsids.buckets: 1 }
+  - match: { aggregations.filterA.tsids.buckets.0.doc_count: 2 }
+
+  - do:
+      search:
+        index: k9s
+        body:
+          size: 0
+          aggs:
+            filterA:
+              filter:
+                term:
+                  another.deeper.dim2: 10.5
+              aggs:
+                tsids:
+                  terms:
+                    field: _tsid
+
+  - length: { aggregations.filterA.tsids.buckets: 1 }
+  - match: { aggregations.filterA.tsids.buckets.0.doc_count: 2 }
+
+  - do:
+      search:
+        index: k9s
+        body:
+          size: 0
+          aggs:
+            filterA:
+              filter:
+                term:
+                  a.much.deeper.nested.dim: AC
+              aggs:
+                tsids:
+                  terms:
+                    field: _tsid
+
+  - length: { aggregations.filterA.tsids.buckets: 1 }
   - match: { aggregations.filterA.tsids.buckets.0.doc_count: 2 }
 
 ---

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

@@ -128,19 +128,22 @@ public class RootObjectMapper extends ObjectMapper {
         }
 
         Map<String, Mapper> getAliasMappers(Map<String, Mapper> mappers, MapperBuilderContext context) {
-            Map<String, Mapper> aliasMappers = new HashMap<>();
+            Map<String, Mapper> newMappers = new HashMap<>();
             Map<String, ObjectMapper.Builder> objectIntermediates = new HashMap<>(1);
-            getAliasMappers(mappers, aliasMappers, objectIntermediates, context, 0);
+            Map<String, ObjectMapper.Builder> objectIntermediatesFullName = new HashMap<>(1);
+            getAliasMappers(mappers, mappers, newMappers, objectIntermediates, objectIntermediatesFullName, context, 0);
             for (var entry : objectIntermediates.entrySet()) {
-                aliasMappers.put(entry.getKey(), entry.getValue().build(context));
+                newMappers.put(entry.getKey(), entry.getValue().build(context));
             }
-            return aliasMappers;
+            return newMappers;
         }
 
         void getAliasMappers(
             Map<String, Mapper> mappers,
+            Map<String, Mapper> topLevelMappers,
             Map<String, Mapper> aliasMappers,
             Map<String, ObjectMapper.Builder> objectIntermediates,
+            Map<String, ObjectMapper.Builder> objectIntermediatesFullName,
             MapperBuilderContext context,
             int level
         ) {
@@ -179,32 +182,76 @@ public class RootObjectMapper extends ObjectMapper {
                                     ).build(context);
                                     aliasMappers.put(aliasMapper.simpleName(), aliasMapper);
                                 } else {
+                                    conflict = topLevelMappers.get(fieldNameParts[0]);
+                                    if (conflict != null) {
+                                        if (isConflictingObject(conflict, fieldNameParts)) {
+                                            throw new IllegalArgumentException(
+                                                "Conflicting objects created during alias generation for pass-through field: ["
+                                                    + conflict.name()
+                                                    + "]"
+                                            );
+                                        }
+                                    }
+
                                     // Nest the alias within object(s).
                                     String realFieldName = fieldNameParts[fieldNameParts.length - 1];
                                     Mapper.Builder fieldBuilder = new FieldAliasMapper.Builder(realFieldName).path(
                                         fieldMapper.mappedFieldType.name()
                                     );
+                                    ObjectMapper.Builder intermediate = null;
                                     for (int i = fieldNameParts.length - 2; i >= 0; --i) {
                                         String intermediateObjectName = fieldNameParts[i];
-                                        ObjectMapper.Builder intermediate = objectIntermediates.computeIfAbsent(
-                                            intermediateObjectName,
+                                        intermediate = objectIntermediatesFullName.computeIfAbsent(
+                                            concatStrings(fieldNameParts, i),
                                             s -> new ObjectMapper.Builder(intermediateObjectName, ObjectMapper.Defaults.SUBOBJECTS)
                                         );
                                         intermediate.add(fieldBuilder);
                                         fieldBuilder = intermediate;
                                     }
+                                    objectIntermediates.putIfAbsent(fieldNameParts[0], intermediate);
                                 }
                             }
                         }
                     }
                 } else if (mapper instanceof ObjectMapper objectMapper) {
                     // Call recursively to check child fields. The level guards against long recursive call sequences.
-                    getAliasMappers(objectMapper.mappers, aliasMappers, objectIntermediates, context, level + 1);
+                    getAliasMappers(
+                        objectMapper.mappers,
+                        topLevelMappers,
+                        aliasMappers,
+                        objectIntermediates,
+                        objectIntermediatesFullName,
+                        context,
+                        level + 1
+                    );
                 }
             }
         }
     }
 
+    private static String concatStrings(String[] parts, int last) {
+        StringBuilder builder = new StringBuilder();
+        for (int i = 0; i <= last; i++) {
+            builder.append('.');
+            builder.append(parts[i]);
+        }
+        return builder.toString();
+    }
+
+    private static boolean isConflictingObject(Mapper mapper, String[] parts) {
+        for (int i = 0; i < parts.length - 1; i++) {
+            if (mapper == null) {
+                return true;
+            }
+            if (mapper instanceof ObjectMapper objectMapper) {
+                mapper = objectMapper.getMapper(parts[i + 1]);
+            } else {
+                return true;
+            }
+        }
+        return mapper == null;
+    }
+
     private final Explicit<DateFormatter[]> dynamicDateTimeFormatters;
     private final Explicit<Boolean> dateDetection;
     private final Explicit<Boolean> numericDetection;

server/src/test/java/org/elasticsearch/index/mapper/RootObjectMapperTests.java

@@ -391,6 +391,100 @@ public class RootObjectMapperTests extends MapperServiceTestCase {
         assertThat(mapperService.mappingLookup().getMapper("attributes.another.dim"), instanceOf(KeywordFieldMapper.class));
     }
 
+    public void testPassThroughObjectNestedWithDuplicateNames() throws IOException {
+        MapperService mapperService = createMapperService(mapping(b -> {
+            b.startObject("resource").field("type", "object");
+            {
+                b.startObject("properties");
+                {
+                    b.startObject("attributes").field("type", "passthrough");
+                    {
+                        b.startObject("properties");
+                        b.startObject("dim").field("type", "keyword").endObject();
+                        b.startObject("more.attributes.another.dimA").field("type", "keyword").endObject();
+                        b.startObject("more.attributes.another.dimB").field("type", "keyword").endObject();
+                        b.endObject();
+                    }
+                    b.endObject();
+                }
+                b.endObject();
+            }
+            b.endObject();
+            b.startObject("attributes").field("type", "passthrough");
+            {
+                b.startObject("properties");
+                b.startObject("another.dim").field("type", "keyword").endObject();
+                b.startObject("more.attributes.another.dimC").field("type", "keyword").endObject();
+                b.startObject("more.attributes.another.dimD").field("type", "keyword").endObject();
+                b.endObject();
+            }
+            b.endObject();
+        }));
+
+        assertThat(mapperService.mappingLookup().getMapper("dim"), instanceOf(FieldAliasMapper.class));
+        assertThat(mapperService.mappingLookup().getMapper("resource.attributes.dim"), instanceOf(KeywordFieldMapper.class));
+        assertThat(
+            mapperService.mappingLookup().objectMappers().get("more.attributes.another").getMapper("dimA"),
+            instanceOf(FieldAliasMapper.class)
+        );
+        assertThat(
+            mapperService.mappingLookup().getMapper("resource.attributes.more.attributes.another.dimA"),
+            instanceOf(KeywordFieldMapper.class)
+        );
+        assertThat(
+            mapperService.mappingLookup().objectMappers().get("more.attributes.another").getMapper("dimB"),
+            instanceOf(FieldAliasMapper.class)
+        );
+        assertThat(
+            mapperService.mappingLookup().getMapper("resource.attributes.more.attributes.another.dimB"),
+            instanceOf(KeywordFieldMapper.class)
+        );
+
+        assertThat(mapperService.mappingLookup().objectMappers().get("another").getMapper("dim"), instanceOf(FieldAliasMapper.class));
+        assertThat(mapperService.mappingLookup().getMapper("attributes.another.dim"), instanceOf(KeywordFieldMapper.class));
+        assertThat(
+            mapperService.mappingLookup().objectMappers().get("more.attributes.another").getMapper("dimC"),
+            instanceOf(FieldAliasMapper.class)
+        );
+        assertThat(
+            mapperService.mappingLookup().getMapper("attributes.more.attributes.another.dimC"),
+            instanceOf(KeywordFieldMapper.class)
+        );
+        assertThat(
+            mapperService.mappingLookup().objectMappers().get("more.attributes.another").getMapper("dimD"),
+            instanceOf(FieldAliasMapper.class)
+        );
+        assertThat(
+            mapperService.mappingLookup().getMapper("attributes.more.attributes.another.dimD"),
+            instanceOf(KeywordFieldMapper.class)
+        );
+    }
+
+    public void testPassThroughObjectNestedWithConflictingNames() throws IOException {
+        MapperParsingException e = expectThrows(MapperParsingException.class, () -> createMapperService(mapping(b -> {
+            b.startObject("resource").field("type", "object");
+            {
+                b.startObject("properties");
+                {
+                    b.startObject("attributes").field("type", "passthrough");
+                    {
+                        b.startObject("properties");
+                        b.startObject("dim").field("type", "keyword").endObject();
+                        b.startObject("resource.attributes.another.dim").field("type", "keyword").endObject();
+                        b.endObject();
+                    }
+                    b.endObject();
+                }
+                b.endObject();
+            }
+            b.endObject();
+        })));
+        assertEquals(
+            "Failed to parse mapping: Conflicting objects created during alias generation for pass-through field: [resource]",
+            e.getMessage()
+        );
+    }
+
     public void testAliasMappersCreatesAlias() throws Exception {
         var context = MapperBuilderContext.root(false, false);
         Map<String, Mapper> aliases = new RootObjectMapper.Builder("root", Explicit.EXPLICIT_FALSE).getAliasMappers(
@@ -445,6 +539,7 @@ public class RootObjectMapperTests extends MapperServiceTestCase {
         var context = MapperBuilderContext.root(false, false);
         Map<String, Mapper> aliases = new HashMap<>();
         var objectIntermediates = new HashMap<String, ObjectMapper.Builder>(1);
+        var objectIntermediatesFullPath = new HashMap<String, ObjectMapper.Builder>(1);
         new RootObjectMapper.Builder("root", Explicit.EXPLICIT_FALSE).getAliasMappers(
             Map.of(
                 "labels",
@@ -457,8 +552,10 @@ public class RootObjectMapperTests extends MapperServiceTestCase {
                     Explicit.EXPLICIT_FALSE
                 )
             ),
+            Map.of(),
             aliases,
             objectIntermediates,
+            objectIntermediatesFullPath,
             context,
             1_000_000
         );