Skip to content

Commit 0a5d2e7

Browse files
kkrik-eskkrik-es
authored
Merge field mappers when updating mappings with [subobjects:false] (#120370)
* Merge field mappers when updating mappings with [subobjects:false] * Update docs/changelog/120370.yaml * fix and add tests * node feature guarding
1 parent 5e8cce2 commit 0a5d2e7

File tree

5 files changed

+148
-6
lines changed

5 files changed

+148
-6
lines changed

‎docs/changelog/120370.yaml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
pr: 120370
2+
summary: "Merge field mappers when updating mappings with [subobjects:false]"
3+
area: Mapping
4+
type: bug
5+
issues:
6+
- 120216

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

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,3 +185,92 @@
185185
body:
186186
_source:
187187
mode: synthetic
188+
189+
---
190+
"modify field type with subobjects:false":
191+
- requires:
192+
cluster_features: [ "mapper.subobjects_false_mapping_update_fix" ]
193+
reason: requires fix for mapping updates when [subobjects:false] is set
194+
- do:
195+
indices.create:
196+
index: test_index
197+
body:
198+
mappings:
199+
subobjects: false
200+
properties:
201+
user.id:
202+
type: long
203+
user.name:
204+
type: text
205+
206+
- do:
207+
catch: bad_request
208+
indices.put_mapping:
209+
index: test_index
210+
body:
211+
properties:
212+
user.id:
213+
type: keyword
214+
215+
- match: { error.type: "illegal_argument_exception" }
216+
- match: { error.reason: "mapper [user.id] cannot be changed from type [long] to [keyword]" }
217+
218+
- do:
219+
indices.put_mapping:
220+
index: test_index
221+
body:
222+
properties:
223+
user.name:
224+
type: text
225+
fields:
226+
raw:
227+
type: keyword
228+
229+
- is_true: acknowledged
230+
231+
232+
---
233+
"modify nested field type with subobjects:false":
234+
- requires:
235+
cluster_features: [ "mapper.subobjects_false_mapping_update_fix" ]
236+
reason: requires fix for mapping updates when [subobjects:false] is set
237+
- do:
238+
indices.create:
239+
index: test_index
240+
body:
241+
mappings:
242+
properties:
243+
path:
244+
properties:
245+
to:
246+
subobjects: false
247+
properties:
248+
user.id:
249+
type: long
250+
user.name:
251+
type: text
252+
253+
- do:
254+
catch: bad_request
255+
indices.put_mapping:
256+
index: test_index
257+
body:
258+
properties:
259+
path.to.user.id:
260+
type: keyword
261+
262+
- match: { error.type: "illegal_argument_exception" }
263+
- match: { error.reason: "mapper [path.to.user.id] cannot be changed from type [long] to [keyword]" }
264+
265+
- do:
266+
indices.put_mapping:
267+
index: test_index
268+
body:
269+
properties:
270+
path.to.user.name:
271+
type: text
272+
fields:
273+
raw:
274+
type: keyword
275+
276+
- is_true: acknowledged

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,8 @@ public Set<NodeFeature> getTestFeatures() {
5454
META_FETCH_FIELDS_ERROR_CODE_CHANGED,
5555
SPARSE_VECTOR_STORE_SUPPORT,
5656
COUNTED_KEYWORD_SYNTHETIC_SOURCE_NATIVE_SUPPORT,
57-
SourceFieldMapper.SYNTHETIC_RECOVERY_SOURCE
57+
SourceFieldMapper.SYNTHETIC_RECOVERY_SOURCE,
58+
ObjectMapper.SUBOBJECTS_FALSE_MAPPING_UPDATE_FIX
5859
);
5960
}
6061
}

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

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import org.elasticsearch.common.util.FeatureFlag;
2121
import org.elasticsearch.common.xcontent.support.XContentMapValues;
2222
import org.elasticsearch.core.Nullable;
23+
import org.elasticsearch.features.NodeFeature;
2324
import org.elasticsearch.index.IndexVersion;
2425
import org.elasticsearch.index.IndexVersions;
2526
import org.elasticsearch.index.mapper.MapperService.MergeReason;
@@ -48,6 +49,7 @@ public class ObjectMapper extends Mapper {
4849
private static final Logger logger = LogManager.getLogger(ObjectMapper.class);
4950
private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(ObjectMapper.class);
5051
public static final FeatureFlag SUB_OBJECTS_AUTO_FEATURE_FLAG = new FeatureFlag("sub_objects_auto");
52+
static final NodeFeature SUBOBJECTS_FALSE_MAPPING_UPDATE_FIX = new NodeFeature("mapper.subobjects_false_mapping_update_fix");
5153

5254
public static final String CONTENT_TYPE = "object";
5355
static final String STORE_ARRAY_SOURCE_PARAM = "store_array_source";
@@ -659,11 +661,21 @@ private static Map<String, Mapper> buildMergedMappers(
659661
if (subobjects.isPresent()
660662
&& subobjects.get() == Subobjects.DISABLED
661663
&& mergeWithMapper instanceof ObjectMapper objectMapper) {
662-
// An existing mapping that has set `subobjects: false` is merged with a mapping with sub-objects
663-
objectMapper.asFlattenedFieldMappers(objectMergeContext.getMapperBuilderContext())
664-
.stream()
665-
.filter(m -> objectMergeContext.decrementFieldBudgetIfPossible(m.getTotalFieldsCount()))
666-
.forEach(m -> putMergedMapper(mergedMappers, m));
664+
// An existing mapping that has set `subobjects: false` is merged with a mapping with sub-objects.
665+
List<FieldMapper> flattenedMappers = objectMapper.asFlattenedFieldMappers(
666+
objectMergeContext.getMapperBuilderContext()
667+
);
668+
for (FieldMapper flattenedMapper : flattenedMappers) {
669+
if (objectMergeContext.decrementFieldBudgetIfPossible(flattenedMapper.getTotalFieldsCount())) {
670+
var conflict = mergedMappers.get(flattenedMapper.leafName());
671+
if (objectMergeContext.getMapperBuilderContext().getMergeReason() == MergeReason.INDEX_TEMPLATE
672+
|| conflict == null) {
673+
putMergedMapper(mergedMappers, flattenedMapper);
674+
} else {
675+
putMergedMapper(mergedMappers, conflict.merge(flattenedMapper, objectMergeContext));
676+
}
677+
}
678+
}
667679
} else if (objectMergeContext.decrementFieldBudgetIfPossible(mergeWithMapper.getTotalFieldsCount())) {
668680
putMergedMapper(mergedMappers, mergeWithMapper);
669681
} else if (mergeWithMapper instanceof ObjectMapper om) {

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

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -329,6 +329,40 @@ public void testDisallowFieldReplacementForIndexTemplates() throws IOException {
329329
assertThat(e.getMessage(), containsString("can't merge a non object mapping [object.field1] with an object mapping"));
330330
}
331331

332+
public void testFieldReplacementSubobjectsFalse() throws IOException {
333+
MapperService mapperService = createMapperService(mapping(b -> {
334+
b.startObject("obj").field("type", "object").field("subobjects", false).startObject("properties");
335+
{
336+
b.startObject("my.field").field("type", "keyword").endObject();
337+
}
338+
b.endObject().endObject();
339+
}));
340+
DocumentMapper mapper = mapperService.documentMapper();
341+
assertNull(mapper.mapping().getRoot().dynamic());
342+
Mapping mergeWith = mapperService.parseMapping(
343+
"_doc",
344+
MergeReason.INDEX_TEMPLATE,
345+
new CompressedXContent(BytesReference.bytes(topMapping(b -> {
346+
b.startObject("properties").startObject("obj").field("type", "object").field("subobjects", false).startObject("properties");
347+
{
348+
b.startObject("my.field").field("type", "long").endObject();
349+
}
350+
b.endObject().endObject().endObject();
351+
})))
352+
);
353+
354+
// Fails on mapping update.
355+
IllegalArgumentException exception = expectThrows(
356+
IllegalArgumentException.class,
357+
() -> mapper.mapping().merge(mergeWith, MergeReason.MAPPING_UPDATE, Long.MAX_VALUE)
358+
);
359+
assertEquals("mapper [obj.my.field] cannot be changed from type [keyword] to [long]", exception.getMessage());
360+
361+
// Passes on template merging.
362+
Mapping merged = mapper.mapping().merge(mergeWith, MergeReason.INDEX_TEMPLATE, Long.MAX_VALUE);
363+
assertThat(((ObjectMapper) merged.getRoot().getMapper("obj")).getMapper("my.field"), instanceOf(NumberFieldMapper.class));
364+
}
365+
332366
public void testUnknownLegacyFields() throws Exception {
333367
MapperService service = createMapperService(IndexVersion.fromId(5000099), Settings.EMPTY, () -> false, mapping(b -> {
334368
b.startObject("name");

0 commit comments

Comments
 (0)