From 346889a93bb18cda93736a8076aef198b8f86fe1 Mon Sep 17 00:00:00 2001 From: PnPie Date: Sat, 29 Apr 2017 21:06:53 +0200 Subject: [PATCH 1/2] Keep _parent field mapping while updating child mapping (#23381) be able to update child type mapping without specifying _parent field instead of trying to update it to "null" as previously. --- .../index/mapper/ParentFieldMapper.java | 2 +- .../mapper/DocumentMapperMergeTests.java | 43 ++++++++++++++++--- 2 files changed, 37 insertions(+), 8 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 62877567c3d4d..7c16fcc292917 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/ParentFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/ParentFieldMapper.java @@ -297,7 +297,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws protected void doMerge(Mapper mergeWith, boolean updateAllTypes) { 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 + "]"); } 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 e2fbbe7ebfe78..d1468db905a8c 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 void run() { 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 void run() { 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,33 @@ public void testDoNotRepeatOriginalMapping() throws IOException { 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)); + + String updatedMapping = XContentFactory.jsonBuilder().startObject().startObject("child") + .startObject("properties") + .startObject("name").field("type", "text").endObject() + .endObject().endObject().endObject().string(); + DocumentMapper updatedMapper = parser.parse("child", new CompressedXContent(updatedMapping)); + DocumentMapper mergedMapper = initMapper.merge(updatedMapper.mapping(), false); + + assertThat(mergedMapper.mappers().getMapper("_parent"), notNullValue()); + assertThat(mergedMapper.mappers().getMapper("name"), 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)); + try { + initMapper.merge(modParentMapper.mapping(), false); + } catch (IllegalArgumentException e) { + assertThat(e.getMessage(), containsString("The _parent field's type option can't be changed:")); + } + } } From 5bed8f56ae64e77bb2e56a1370f1bf4e0a6557cf Mon Sep 17 00:00:00 2001 From: PnPie Date: Thu, 1 Jun 2017 15:42:11 +0200 Subject: [PATCH 2/2] Fix _parent field's name changing while merging The parent field should not be updatable, but the it's fieldType is changed and the new one is applied. So this patch make it keep the original one while merging. --- .../index/mapper/ParentFieldMapper.java | 3 +- .../mapper/DocumentMapperMergeTests.java | 52 +++++++++++++++---- 2 files changed, 44 insertions(+), 11 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 7c16fcc292917..c0da57730cfe9 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/ParentFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/ParentFieldMapper.java @@ -295,6 +295,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws @Override protected void doMerge(Mapper mergeWith, boolean updateAllTypes) { + ParentFieldType currentFieldType = (ParentFieldType) fieldType.clone(); super.doMerge(mergeWith, updateAllTypes); ParentFieldMapper fieldMergeWith = (ParentFieldMapper) mergeWith; if (fieldMergeWith.parentType != null && Objects.equals(parentType, fieldMergeWith.parentType) == false) { @@ -308,7 +309,7 @@ protected void doMerge(Mapper mergeWith, boolean updateAllTypes) { } 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 d1468db905a8c..198992b41a063 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/DocumentMapperMergeTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/DocumentMapperMergeTests.java @@ -244,24 +244,56 @@ public void testMergeChildType() throws IOException { .endObject().endObject().string(); DocumentMapper initMapper = parser.parse("child", new CompressedXContent(initMapping)); - String updatedMapping = XContentFactory.jsonBuilder().startObject().startObject("child") + 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 updatedMapper = parser.parse("child", new CompressedXContent(updatedMapping)); - DocumentMapper mergedMapper = initMapper.merge(updatedMapper.mapping(), false); + 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(mergedMapper.mappers().getMapper("_parent"), notNullValue()); - assertThat(mergedMapper.mappers().getMapper("name"), notNullValue()); + 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)); - try { - initMapper.merge(modParentMapper.mapping(), false); - } catch (IllegalArgumentException e) { - assertThat(e.getMessage(), containsString("The _parent field's type option can't be changed:")); - } + 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]")); } }