Allow a passthrough mapper to merge with object under certain conditions

Author: gmarouli

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

@@ -14,6 +14,12 @@ static void throwObjectMappingConflictError(String fieldName) throws IllegalArgu
         throw new IllegalArgumentException("can't merge a non object mapping [" + fieldName + "] with an object mapping");
     }
 
+    static void throPassThroughMappingConflictError(String fieldName) throws IllegalArgumentException {
+        throw new IllegalArgumentException(
+            "can't merge a passthrough mapping [" + fieldName + "] with an object mapping that is either root or has subobjects enabled"
+        );
+    }
+
     static void throwNestedMappingConflictError(String fieldName) throws IllegalArgumentException {
         throw new IllegalArgumentException("can't merge a non-nested mapping [" + fieldName + "] with a nested mapping");
     }

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

@@ -148,29 +148,58 @@ public PassThroughObjectMapper.Builder newBuilder(IndexVersion indexVersionCreat
 
     @Override
     public PassThroughObjectMapper merge(Mapper mergeWith, MapperMergeContext parentBuilderContext) {
-        if (mergeWith instanceof PassThroughObjectMapper == false) {
+        if (mergeWith instanceof ObjectMapper == false) {
             MapperErrors.throwObjectMappingConflictError(mergeWith.fullPath());
         }
+        ObjectMapper mergeWithObjectMapper = (ObjectMapper) mergeWith;
+        if (mergeWithObjectMapper instanceof PassThroughObjectMapper mergeWithPassThrough) {
+            final var mergeResult = MergeResult.build(this, mergeWithPassThrough, parentBuilderContext);
+            final Explicit<Boolean> containsDimensions = (mergeWithPassThrough.timeSeriesDimensionSubFields.explicit())
+                ? mergeWithPassThrough.timeSeriesDimensionSubFields
+                : this.timeSeriesDimensionSubFields;
 
-        PassThroughObjectMapper mergeWithObject = (PassThroughObjectMapper) mergeWith;
-        final var mergeResult = MergeResult.build(this, mergeWithObject, parentBuilderContext);
-
-        final Explicit<Boolean> containsDimensions = (mergeWithObject.timeSeriesDimensionSubFields.explicit())
-            ? mergeWithObject.timeSeriesDimensionSubFields
-            : this.timeSeriesDimensionSubFields;
-
+            return new PassThroughObjectMapper(
+                leafName(),
+                fullPath(),
+                mergeResult.enabled(),
+                mergeResult.sourceKeepMode(),
+                mergeResult.dynamic(),
+                mergeResult.mappers(),
+                containsDimensions,
+                Math.max(priority, mergeWithPassThrough.priority)
+            );
+        }
+        if (mergeWithObjectMapper instanceof NestedObjectMapper nestedObjectMapper) {
+            MapperErrors.throwNestedMappingConflictError(fullPath());
+        }
+        if (isEligibleForMerge(mergeWithObjectMapper) == false) {
+            MapperErrors.throPassThroughMappingConflictError(fullPath());
+        }
+        MergeResult mergeResult = MergeResult.build(this, mergeWithObjectMapper, parentBuilderContext);
         return new PassThroughObjectMapper(
             leafName(),
             fullPath(),
             mergeResult.enabled(),
             mergeResult.sourceKeepMode(),
             mergeResult.dynamic(),
             mergeResult.mappers(),
-            containsDimensions,
-            Math.max(priority, mergeWithObject.priority)
+            timeSeriesDimensionSubFields,
+            priority
         );
     }
 
+    /**
+     * An object mapper is compatible to be merged with a passthrough mapper if
+     * - It is not a root mapper
+     * - If it does not have subobjects true
+     */
+    private boolean isEligibleForMerge(ObjectMapper objectMapper) {
+        return objectMapper.isRoot() == false
+            && (objectMapper.subobjects == null
+                || objectMapper.subobjects.explicit() == false
+                || objectMapper.subobjects.value().equals(Subobjects.DISABLED));
+    }
+
     @Override
     public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
         builder.startObject(leafName());

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

@@ -10,13 +10,16 @@
 package org.elasticsearch.index.mapper;
 
 import org.elasticsearch.common.Explicit;
+import org.elasticsearch.index.IndexVersion;
 
 import java.io.IOException;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 
+import static org.elasticsearch.index.mapper.MapperService.MergeReason.MAPPING_UPDATE;
 import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.instanceOf;
 
 public class PassThroughObjectMapperTests extends MapperServiceTestCase {
@@ -182,6 +185,39 @@ public void testCheckForDuplicatePrioritiesEmpty() throws IOException {
         PassThroughObjectMapper.checkForDuplicatePriorities(List.of());
     }
 
+    public void testMergingWithPassThrough() {
+        boolean isSourceSynthetic = randomBoolean();
+        var passThroughMapper = new RootObjectMapper.Builder("_doc").add(new PassThroughObjectMapper.Builder("metrics").setPriority(10))
+            .build(MapperBuilderContext.root(isSourceSynthetic, true));
+        var objectMapper = new RootObjectMapper.Builder("_doc").add(
+            new ObjectMapper.Builder("metrics").add(new KeywordFieldMapper.Builder("cpu_usage", IndexVersion.current()))
+        ).build(MapperBuilderContext.root(isSourceSynthetic, true));
+
+        RootObjectMapper merged = passThroughMapper.merge(
+            objectMapper,
+            MapperMergeContext.root(isSourceSynthetic, true, MAPPING_UPDATE, Long.MAX_VALUE)
+        );
+        assertThat(merged.getMapper("metrics"), instanceOf(PassThroughObjectMapper.class));
+
+        var objectMapperWithSubObjectTrue = new RootObjectMapper.Builder("_doc").add(
+            new ObjectMapper.Builder("metrics", Explicit.of(ObjectMapper.Subobjects.ENABLED)).add(
+                new KeywordFieldMapper.Builder("cpu_usage", IndexVersion.current())
+            )
+        ).build(MapperBuilderContext.root(isSourceSynthetic, true));
+
+        IllegalArgumentException error = expectThrows(
+            IllegalArgumentException.class,
+            () -> passThroughMapper.merge(
+                objectMapperWithSubObjectTrue,
+                MapperMergeContext.root(isSourceSynthetic, true, MAPPING_UPDATE, Long.MAX_VALUE)
+            )
+        );
+        assertThat(
+            error.getMessage(),
+            equalTo("can't merge a passthrough mapping [metrics] with an object mapping that is either root or has subobjects enabled")
+        );
+    }
+
     private PassThroughObjectMapper create(String name, int priority) {
         return new PassThroughObjectMapper(
             name,