From 77a7e2480b6745855d0bbfc6020ce70378b6267e Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Mon, 15 Jan 2018 18:34:10 +0100 Subject: [PATCH] Allow update of `eager_global_ordinals` on `_parent`. (#28014) A bug introduced in #24407 currently prevents `eager_global_ordinals` from being updated. This new approach should fix the issue while still allowing mapping updates to not specify the `_parent` field if it doesn't need updating, which was the goal of #24407. --- .../index/mapper/ParentFieldMapper.java | 11 +++++----- .../index/mapper/ParentFieldMapperTests.java | 20 +++++++++++++++++++ 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/ParentFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/ParentFieldMapper.java index 73109a3ecd8..34eaf569ca9 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/ParentFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/ParentFieldMapper.java @@ -303,15 +303,16 @@ public class ParentFieldMapper extends MetadataFieldMapper { @Override protected void doMerge(Mapper mergeWith, boolean updateAllTypes) { ParentFieldMapper fieldMergeWith = (ParentFieldMapper) mergeWith; - ParentFieldType currentFieldType = (ParentFieldType) fieldType.clone(); - super.doMerge(mergeWith, updateAllTypes); if (fieldMergeWith.parentType != null && Objects.equals(parentType, fieldMergeWith.parentType) == false) { throw new IllegalArgumentException("The _parent field's type option can't be changed: [" + parentType + "]->[" + fieldMergeWith.parentType + "]"); } - - if (active()) { - fieldType = currentFieldType; + // If fieldMergeWith is not active it means the user provided a mapping + // update that does not explicitly configure the _parent field, so we + // ignore it. + if (fieldMergeWith.active()) { + super.doMerge(mergeWith, updateAllTypes); } + } /** diff --git a/server/src/test/java/org/elasticsearch/index/mapper/ParentFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/ParentFieldMapperTests.java index d0e17b808c5..d21827ee18c 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/ParentFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/ParentFieldMapperTests.java @@ -41,6 +41,7 @@ import org.elasticsearch.test.ESSingleNodeTestCase; import org.elasticsearch.test.IndexSettingsModule; import org.elasticsearch.test.InternalSettingsPlugin; +import java.io.IOException; import java.util.Collection; import java.util.Collections; import java.util.HashSet; @@ -138,4 +139,23 @@ public class ParentFieldMapperTests extends ESSingleNodeTestCase { return numFieldWithParentPrefix; } + public void testUpdateEagerGlobalOrds() throws IOException { + String parentMapping = XContentFactory.jsonBuilder().startObject().startObject("parent_type") + .endObject().endObject().string(); + String childMapping = XContentFactory.jsonBuilder().startObject().startObject("child_type") + .startObject("_parent").field("type", "parent_type").endObject() + .endObject().endObject().string(); + IndexService indexService = createIndex("test", Settings.builder().put("index.version.created", Version.V_5_6_0).build()); + indexService.mapperService().merge("parent_type", new CompressedXContent(parentMapping), MergeReason.MAPPING_UPDATE, false); + indexService.mapperService().merge("child_type", new CompressedXContent(childMapping), MergeReason.MAPPING_UPDATE, false); + + assertTrue(indexService.mapperService().documentMapper("child_type").parentFieldMapper().fieldType().eagerGlobalOrdinals()); + + String childMappingUpdate = XContentFactory.jsonBuilder().startObject().startObject("child_type") + .startObject("_parent").field("type", "parent_type").field("eager_global_ordinals", false).endObject() + .endObject().endObject().string(); + indexService.mapperService().merge("child_type", new CompressedXContent(childMappingUpdate), MergeReason.MAPPING_UPDATE, false); + + assertFalse(indexService.mapperService().documentMapper("child_type").parentFieldMapper().fieldType().eagerGlobalOrdinals()); + } }