From 14913fdc37130edc58f33216ef300f6832d28ef1 Mon Sep 17 00:00:00 2001 From: Yu Date: Wed, 7 Jun 2017 10:51:21 +0200 Subject: [PATCH] keep _parent field while updating child type mapping (#24407) parent/child: Allow updating mapping without specifying `_parent` field on each update. Prior to this change when a mapping has a `_parent` field then any update (also updates that didn't modify the `_parent` field) to the mapping involved specifying the `_parent` field again. With this change specifying the `_parent` field on each mapping update is no longer required. Closes #23381 --- .../index/mapper/ParentFieldMapper.java | 5 +- .../mapper/DocumentMapperMergeTests.java | 75 +++++++++++++++++-- 2 files changed, 71 insertions(+), 9 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/index/mapper/ParentFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/ParentFieldMapper.java index c7a6b7db32e..49f5b820f8e 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/ParentFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/ParentFieldMapper.java @@ -296,9 +296,10 @@ public class ParentFieldMapper extends MetadataFieldMapper { @Override protected void doMerge(Mapper mergeWith, boolean updateAllTypes) { + ParentFieldType currentFieldType = (ParentFieldType) fieldType.clone(); super.doMerge(mergeWith, updateAllTypes); ParentFieldMapper fieldMergeWith = (ParentFieldMapper) mergeWith; - if (Objects.equals(parentType, fieldMergeWith.parentType) == false) { + 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 + "]"); } @@ -309,7 +310,7 @@ public class ParentFieldMapper extends MetadataFieldMapper { } if (active()) { - fieldType = fieldMergeWith.fieldType.clone(); + fieldType = currentFieldType; } } diff --git a/core/src/test/java/org/elasticsearch/index/mapper/DocumentMapperMergeTests.java b/core/src/test/java/org/elasticsearch/index/mapper/DocumentMapperMergeTests.java index e2fbbe7ebfe..198992b41a0 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/DocumentMapperMergeTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/DocumentMapperMergeTests.java @@ -168,9 +168,9 @@ public class DocumentMapperMergeTests extends ESSingleNodeTestCase { barrier.await(); for (int i = 0; i < 200 && stopped.get() == false; i++) { final String fieldName = Integer.toString(i); - ParsedDocument doc = documentMapper.parse(SourceToParse.source("test", - "test", - fieldName, + ParsedDocument doc = documentMapper.parse(SourceToParse.source("test", + "test", + fieldName, new BytesArray("{ \"" + fieldName + "\" : \"test\" }"), XContentType.JSON)); Mapping update = doc.dynamicMappingsUpdate(); @@ -191,10 +191,10 @@ public class DocumentMapperMergeTests extends ESSingleNodeTestCase { while(stopped.get() == false) { final String fieldName = lastIntroducedFieldName.get(); final BytesReference source = new BytesArray("{ \"" + fieldName + "\" : \"test\" }"); - ParsedDocument parsedDoc = documentMapper.parse(SourceToParse.source("test", - "test", - "random", - source, + ParsedDocument parsedDoc = documentMapper.parse(SourceToParse.source("test", + "test", + "random", + source, XContentType.JSON)); if (parsedDoc.dynamicMappingsUpdate() != null) { // not in the mapping yet, try again @@ -235,4 +235,65 @@ public class DocumentMapperMergeTests extends ESSingleNodeTestCase { assertNotNull(mapper.mappers().getMapper("foo")); assertFalse(mapper.sourceMapper().enabled()); } + + public void testMergeChildType() throws IOException { + DocumentMapperParser parser = createIndex("test").mapperService().documentMapperParser(); + + String initMapping = XContentFactory.jsonBuilder().startObject().startObject("child") + .startObject("_parent").field("type", "parent").endObject() + .endObject().endObject().string(); + DocumentMapper initMapper = parser.parse("child", new CompressedXContent(initMapping)); + + assertThat(initMapper.mappers().getMapper("_parent#parent"), notNullValue()); + + String updatedMapping1 = XContentFactory.jsonBuilder().startObject().startObject("child") + .startObject("properties") + .startObject("name").field("type", "text").endObject() + .endObject().endObject().endObject().string(); + DocumentMapper updatedMapper1 = parser.parse("child", new CompressedXContent(updatedMapping1)); + DocumentMapper mergedMapper1 = initMapper.merge(updatedMapper1.mapping(), false); + + assertThat(mergedMapper1.mappers().getMapper("_parent#parent"), notNullValue()); + assertThat(mergedMapper1.mappers().getMapper("name"), notNullValue()); + + String updatedMapping2 = XContentFactory.jsonBuilder().startObject().startObject("child") + .startObject("_parent").field("type", "parent").endObject() + .startObject("properties") + .startObject("age").field("type", "byte").endObject() + .endObject().endObject().endObject().string(); + DocumentMapper updatedMapper2 = parser.parse("child", new CompressedXContent(updatedMapping2)); + DocumentMapper mergedMapper2 = mergedMapper1.merge(updatedMapper2.mapping(), false); + + assertThat(mergedMapper2.mappers().getMapper("_parent#parent"), notNullValue()); + assertThat(mergedMapper2.mappers().getMapper("name"), notNullValue()); + assertThat(mergedMapper2.mappers().getMapper("age"), notNullValue()); + + String modParentMapping = XContentFactory.jsonBuilder().startObject().startObject("child") + .startObject("_parent").field("type", "new_parent").endObject() + .endObject().endObject().string(); + DocumentMapper modParentMapper = parser.parse("child", new CompressedXContent(modParentMapping)); + Exception e = expectThrows(IllegalArgumentException.class, () -> initMapper.merge(modParentMapper.mapping(), false)); + assertThat(e.getMessage(), containsString("The _parent field's type option can't be changed: [parent]->[new_parent]")); + } + + public void testMergeAddingParent() throws IOException { + DocumentMapperParser parser = createIndex("test").mapperService().documentMapperParser(); + + String initMapping = XContentFactory.jsonBuilder().startObject().startObject("cowboy") + .startObject("properties") + .startObject("name").field("type", "text").endObject() + .endObject().endObject().endObject().string(); + DocumentMapper initMapper = parser.parse("cowboy", new CompressedXContent(initMapping)); + + assertThat(initMapper.mappers().getMapper("name"), notNullValue()); + + String updatedMapping = XContentFactory.jsonBuilder().startObject().startObject("cowboy") + .startObject("_parent").field("type", "parent").endObject() + .startObject("properties") + .startObject("age").field("type", "byte").endObject() + .endObject().endObject().endObject().string(); + DocumentMapper updatedMapper = parser.parse("cowboy", new CompressedXContent(updatedMapping)); + Exception e = expectThrows(IllegalArgumentException.class, () -> initMapper.merge(updatedMapper.mapping(), false)); + assertThat(e.getMessage(), containsString("The _parent field's type option can't be changed: [null]->[parent]")); + } }