Merge field mappers when updating mappings with [subobjects:false]

docs/changelog/120370.yaml

@@ -0,0 +1,6 @@
+pr: 120370
+summary: "Merge field mappers when updating mappings with [subobjects:false]"
+area: Mapping
+type: bug
+issues:
+ - 120216

rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.put_mapping/10_basic.yml

@@ -185,3 +185,92 @@
  body:
  _source:
  mode: synthetic
+
+---
+"modify field type with subobjects:false":
+ - requires:
+ cluster_features: [ "mapper.subobjects_false_mapping_update_fix" ]
+ reason: requires fix for mapping updates when [subobjects:false] is set
+ - do:
+ indices.create:
+ index: test_index
+ body:
+ mappings:
+ subobjects: false
+ properties:
+ user.id:
+ type: long
+ user.name:
+ type: text
+
+ - do:
+ catch: bad_request
+ indices.put_mapping:
+ index: test_index
+ body:
+ properties:
+ user.id:
+ type: keyword
+
+ - match: { error.type: "illegal_argument_exception" }
+ - match: { error.reason: "mapper [user.id] cannot be changed from type [long] to [keyword]" }
+
+ - do:
+ indices.put_mapping:
+ index: test_index
+ body:
+ properties:
+ user.name:
+ type: text
+ fields:
+ raw:
+ type: keyword
+
+ - is_true: acknowledged
+
+
+---
+"modify nested field type with subobjects:false":
+ - requires:
+ cluster_features: [ "mapper.subobjects_false_mapping_update_fix" ]
+ reason: requires fix for mapping updates when [subobjects:false] is set
+ - do:
+ indices.create:
+ index: test_index
+ body:
+ mappings:
+ properties:
+ path:
+ properties:
+ to:
+ subobjects: false
+ properties:
+ user.id:
+ type: long
+ user.name:
+ type: text
+
+ - do:
+ catch: bad_request
+ indices.put_mapping:
+ index: test_index
+ body:
+ properties:
+ path.to.user.id:
+ type: keyword
+
+ - match: { error.type: "illegal_argument_exception" }
+ - match: { error.reason: "mapper [path.to.user.id] cannot be changed from type [long] to [keyword]" }
+
+ - do:
+ indices.put_mapping:
+ index: test_index
+ body:
+ properties:
+ path.to.user.name:
+ type: text
+ fields:
+ raw:
+ type: keyword
+
+ - is_true: acknowledged

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

@@ -54,7 +54,8 @@ public Set<NodeFeature> getTestFeatures() {
  META_FETCH_FIELDS_ERROR_CODE_CHANGED,
  SPARSE_VECTOR_STORE_SUPPORT,
  COUNTED_KEYWORD_SYNTHETIC_SOURCE_NATIVE_SUPPORT,
- SourceFieldMapper.SYNTHETIC_RECOVERY_SOURCE
+ SourceFieldMapper.SYNTHETIC_RECOVERY_SOURCE,
+ ObjectMapper.SUBOBJECTS_FALSE_MAPPING_UPDATE_FIX
  );
  }
 }

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

@@ -20,6 +20,7 @@
 import org.elasticsearch.common.util.FeatureFlag;
 import org.elasticsearch.common.xcontent.support.XContentMapValues;
 import org.elasticsearch.core.Nullable;
+import org.elasticsearch.features.NodeFeature;
 import org.elasticsearch.index.IndexVersion;
 import org.elasticsearch.index.IndexVersions;
 import org.elasticsearch.index.mapper.MapperService.MergeReason;
@@ -48,6 +49,7 @@ public class ObjectMapper extends Mapper {
  private static final Logger logger = LogManager.getLogger(ObjectMapper.class);
  private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(ObjectMapper.class);
  public static final FeatureFlag SUB_OBJECTS_AUTO_FEATURE_FLAG = new FeatureFlag("sub_objects_auto");
+ static final NodeFeature SUBOBJECTS_FALSE_MAPPING_UPDATE_FIX = new NodeFeature("mapper.subobjects_false_mapping_update_fix");
 
  public static final String CONTENT_TYPE = "object";
  static final String STORE_ARRAY_SOURCE_PARAM = "store_array_source";
@@ -659,11 +661,21 @@ private static Map<String, Mapper> buildMergedMappers(
  if (subobjects.isPresent()
  && subobjects.get() == Subobjects.DISABLED
  && mergeWithMapper instanceof ObjectMapper objectMapper) {
- // An existing mapping that has set `subobjects: false` is merged with a mapping with sub-objects
- objectMapper.asFlattenedFieldMappers(objectMergeContext.getMapperBuilderContext())
- .stream()
- .filter(m -> objectMergeContext.decrementFieldBudgetIfPossible(m.getTotalFieldsCount()))
- .forEach(m -> putMergedMapper(mergedMappers, m));
+ // An existing mapping that has set `subobjects: false` is merged with a mapping with sub-objects.
+ List<FieldMapper> flattenedMappers = objectMapper.asFlattenedFieldMappers(
+ objectMergeContext.getMapperBuilderContext()
+ );
+ for (FieldMapper flattenedMapper : flattenedMappers) {
+ if (objectMergeContext.decrementFieldBudgetIfPossible(flattenedMapper.getTotalFieldsCount())) {
+ var conflict = mergedMappers.get(flattenedMapper.leafName());
+ if (objectMergeContext.getMapperBuilderContext().getMergeReason() == MergeReason.INDEX_TEMPLATE
+ || conflict == null) {
+ putMergedMapper(mergedMappers, flattenedMapper);
+ } else {
+ putMergedMapper(mergedMappers, conflict.merge(flattenedMapper, objectMergeContext));
+ }
+ }
+ }
  } else if (objectMergeContext.decrementFieldBudgetIfPossible(mergeWithMapper.getTotalFieldsCount())) {
  putMergedMapper(mergedMappers, mergeWithMapper);
  } else if (mergeWithMapper instanceof ObjectMapper om) {

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

@@ -329,6 +329,40 @@ public void testDisallowFieldReplacementForIndexTemplates() throws IOException {
  assertThat(e.getMessage(), containsString("can't merge a non object mapping [object.field1] with an object mapping"));
  }
 
+ public void testFieldReplacementSubobjectsFalse() throws IOException {
+ MapperService mapperService = createMapperService(mapping(b -> {
+ b.startObject("obj").field("type", "object").field("subobjects", false).startObject("properties");
+ {
+ b.startObject("my.field").field("type", "keyword").endObject();
+ }
+ b.endObject().endObject();
+ }));
+ DocumentMapper mapper = mapperService.documentMapper();
+ assertNull(mapper.mapping().getRoot().dynamic());
+ Mapping mergeWith = mapperService.parseMapping(
+ "_doc",
+ MergeReason.INDEX_TEMPLATE,
+ new CompressedXContent(BytesReference.bytes(topMapping(b -> {
+ b.startObject("properties").startObject("obj").field("type", "object").field("subobjects", false).startObject("properties");
+ {
+ b.startObject("my.field").field("type", "long").endObject();
+ }
+ b.endObject().endObject().endObject();
+ })))
+ );
+
+ // Fails on mapping update.
+ IllegalArgumentException exception = expectThrows(
+ IllegalArgumentException.class,
+ () -> mapper.mapping().merge(mergeWith, MergeReason.MAPPING_UPDATE, Long.MAX_VALUE)
+ );
+ assertEquals("mapper [obj.my.field] cannot be changed from type [keyword] to [long]", exception.getMessage());
+
+ // Passes on template merging.
+ Mapping merged = mapper.mapping().merge(mergeWith, MergeReason.INDEX_TEMPLATE, Long.MAX_VALUE);
+ assertThat(((ObjectMapper) merged.getRoot().getMapper("obj")).getMapper("my.field"), instanceOf(NumberFieldMapper.class));
+ }
+
  public void testUnknownLegacyFields() throws Exception {
  MapperService service = createMapperService(IndexVersion.fromId(5000099), Settings.EMPTY, () -> false, mapping(b -> {
  b.startObject("name");