From 10f0454922afa5c9ee583a33bbc7663593e9537e Mon Sep 17 00:00:00 2001 From: bellengao Date: Fri, 17 Apr 2020 00:12:44 +0800 Subject: [PATCH 1/3] =?UTF-8?q?Fix=20updating=20include=5Fin=5Fparent/incl?= =?UTF-8?q?ude=5Fin=5Froot=20of=20nested=20field=20throws=E2=80=A6=20(#543?= =?UTF-8?q?86)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../index/mapper/ObjectMapper.java | 20 +++++++++++--- .../index/mapper/NestedObjectMapperTests.java | 27 +++++++++++++++++++ 2 files changed, 44 insertions(+), 3 deletions(-) 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 fd4f21b194b35..3908efa50f07c 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 @@ protected void doMerge(final ObjectMapper mergeWith) { 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 @@ protected void doMerge(final ObjectMapper mergeWith) { } } + 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 fb4d6dbe2e4c3..d45800bf1112b 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/NestedObjectMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/NestedObjectMapperTests.java @@ -749,4 +749,31 @@ public void testReorderParentBWC() throws IOException { } } } + + public void testMergeNestedMappings() throws IOException { + MapperService mapperService = createIndex("index1", Settings.EMPTY, 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()); + } } From c96b4ace1ed0d52501e03387696a1e61fb2dd692 Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Thu, 16 Apr 2020 09:29:07 -0700 Subject: [PATCH 2/3] Add a note to the migration docs. --- docs/reference/migration/migrate_7_8.asciidoc | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/docs/reference/migration/migrate_7_8.asciidoc b/docs/reference/migration/migrate_7_8.asciidoc index 566caa5f065c8..8848f9d1d4324 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 From 7ae2dfbfb8e4cde880df4f3884e20e2cb7960dd7 Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Thu, 16 Apr 2020 09:56:31 -0700 Subject: [PATCH 3/3] Adjust tests for backport. --- .../elasticsearch/index/mapper/NestedObjectMapperTests.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 d45800bf1112b..744cfae4e9662 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.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; @@ -751,7 +751,7 @@ public void testReorderParentBWC() throws IOException { } public void testMergeNestedMappings() throws IOException { - MapperService mapperService = createIndex("index1", Settings.EMPTY, jsonBuilder().startObject() + MapperService mapperService = createIndex("index1", Settings.EMPTY, MapperService.SINGLE_MAPPING_NAME, jsonBuilder().startObject() .startObject("properties") .startObject("nested1") .field("type", "nested")