From d7cded8d7a5b61fb8f7fe031838cd50ea97e15ce Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Thu, 16 Apr 2020 11:17:12 -0700 Subject: [PATCH] Fix updating include_in_parent/include_in_root of nested field. (#55326) The main changes are: 1. Throw an error when updating `include_in_parent` or `include_in_root` attribute of nested field dynamically by the PUT mapping API. 2. Add a test for the change. Closes #53792 Co-authored-by: bellengao --- docs/reference/migration/migrate_7_8.asciidoc | 9 ++++++ .../index/mapper/ObjectMapper.java | 20 +++++++++++-- .../index/mapper/NestedObjectMapperTests.java | 29 ++++++++++++++++++- 3 files changed, 54 insertions(+), 4 deletions(-) diff --git a/docs/reference/migration/migrate_7_8.asciidoc b/docs/reference/migration/migrate_7_8.asciidoc index 566caa5f065..8848f9d1d43 100644 --- a/docs/reference/migration/migrate_7_8.asciidoc +++ b/docs/reference/migration/migrate_7_8.asciidoc @@ -41,6 +41,15 @@ Previously, Elasticsearch accepted mapping updates that tried to change the request didn't throw an error. As of 7.8, requests that attempt to change `enabled` on the root mapping will fail. +[discrete] +[[prevent-include-in-root-change]] +==== `include_in_root` and `include_in_parent` cannot be changed + +Elasticsearch previously accepted mapping updates that changed the +`include_in_root` and `include_in_parent` settings. The update was not +applied, but the request didn't throw an error. As of 7.8, requests that +attempt to change these settings will fail. + [discrete] [[breaking_78_settings_changes]] === Settings changes diff --git a/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java index fd4f21b194b..3908efa50f0 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java @@ -459,9 +459,7 @@ public class ObjectMapper extends Mapper implements Cloneable { this.dynamic = mergeWith.dynamic; } - if (isEnabled() != mergeWith.isEnabled()) { - throw new MapperException("The [enabled] parameter can't be updated for the object mapping [" + name() + "]."); - } + checkObjectMapperParameters(mergeWith); for (Mapper mergeWithMapper : mergeWith) { Mapper mergeIntoMapper = mappers.get(mergeWithMapper.simpleName()); @@ -478,6 +476,22 @@ public class ObjectMapper extends Mapper implements Cloneable { } } + private void checkObjectMapperParameters(final ObjectMapper mergeWith) { + if (isEnabled() != mergeWith.isEnabled()) { + throw new MapperException("The [enabled] parameter can't be updated for the object mapping [" + name() + "]."); + } + + if (nested().isIncludeInParent() != mergeWith.nested().isIncludeInParent()) { + throw new MapperException("The [include_in_parent] parameter can't be updated for the nested object mapping [" + + name() + "]."); + } + + if (nested().isIncludeInRoot() != mergeWith.nested().isIncludeInRoot()) { + throw new MapperException("The [include_in_root] parameter can't be updated for the nested object mapping [" + + name() + "]."); + } + } + @Override public ObjectMapper updateFieldType(Map fullNameToFieldType) { List updatedMappers = null; diff --git a/server/src/test/java/org/elasticsearch/index/mapper/NestedObjectMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/NestedObjectMapperTests.java index fb4d6dbe2e4..744cfae4e96 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/NestedObjectMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/NestedObjectMapperTests.java @@ -19,7 +19,6 @@ package org.elasticsearch.index.mapper; -import java.util.HashSet; import org.apache.lucene.index.IndexableField; import org.elasticsearch.Version; import org.elasticsearch.cluster.metadata.IndexMetadata; @@ -41,6 +40,7 @@ import java.io.IOException; import java.io.UncheckedIOException; import java.util.Collection; import java.util.Collections; +import java.util.HashSet; import java.util.function.Function; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; @@ -749,4 +749,31 @@ public class NestedObjectMapperTests extends ESSingleNodeTestCase { } } } + + public void testMergeNestedMappings() throws IOException { + MapperService mapperService = createIndex("index1", Settings.EMPTY, MapperService.SINGLE_MAPPING_NAME, jsonBuilder().startObject() + .startObject("properties") + .startObject("nested1") + .field("type", "nested") + .endObject() + .endObject().endObject()).mapperService(); + + String mapping1 = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type").startObject("properties") + .startObject("nested1").field("type", "nested").field("include_in_parent", true) + .endObject().endObject().endObject().endObject()); + + // cannot update `include_in_parent` dynamically + MapperException e1 = expectThrows(MapperException.class, () -> mapperService.merge("type", + new CompressedXContent(mapping1), MergeReason.MAPPING_UPDATE)); + assertEquals("The [include_in_parent] parameter can't be updated for the nested object mapping [nested1].", e1.getMessage()); + + String mapping2 = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type").startObject("properties") + .startObject("nested1").field("type", "nested").field("include_in_root", true) + .endObject().endObject().endObject().endObject()); + + // cannot update `include_in_root` dynamically + MapperException e2 = expectThrows(MapperException.class, () -> mapperService.merge("type", + new CompressedXContent(mapping2), MergeReason.MAPPING_UPDATE)); + assertEquals("The [include_in_root] parameter can't be updated for the nested object mapping [nested1].", e2.getMessage()); + } }