From 050145f61b3bf6666712059bea07b54ca1076ed6 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Wed, 18 May 2016 16:13:52 +0200 Subject: [PATCH] parent/child: Allow adding additional child types that point to an existing parent type From 2.0 adding child types to existing types was forbidden because the`_parent` field stores the join between parent and child at index time. This is to protect from the fact that types that weren't a parent before become a parent while previously indexed documents would not have a join field. This would break the parent/child queries. The restriction was a bit too strict in the sense that also if a type was a parent type the restriction would forbid adding child types that point to a parent type (so child points already point to it). This change make sure that the restriction only applies if that type isn't a parent type already. Closes #17956 --- .../metadata/MetaDataMappingService.java | 7 +- .../metadata/MetaDataMappingServiceTests.java | 66 +++++++++++++++++++ .../search/child/ChildQuerySearchIT.java | 23 ------- 3 files changed, 71 insertions(+), 25 deletions(-) create mode 100644 core/src/test/java/org/elasticsearch/cluster/metadata/MetaDataMappingServiceTests.java diff --git a/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataMappingService.java b/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataMappingService.java index 311ffae71fd..d3b5e7ecbad 100644 --- a/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataMappingService.java +++ b/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataMappingService.java @@ -281,8 +281,11 @@ public class MetaDataMappingService extends AbstractComponent { // Also the order of the mappings may be backwards. if (newMapper.parentFieldMapper().active()) { for (ObjectCursor mapping : indexMetaData.getMappings().values()) { - if (newMapper.parentFieldMapper().type().equals(mapping.value.type())) { - throw new IllegalArgumentException("can't add a _parent field that points to an already existing type"); + String parentType = newMapper.parentFieldMapper().type(); + if (parentType.equals(mapping.value.type()) && + indexService.mapperService().getParentTypes().contains(parentType) == false) { + throw new IllegalArgumentException("can't add a _parent field that points to an " + + "already existing type, that isn't already a parent"); } } } diff --git a/core/src/test/java/org/elasticsearch/cluster/metadata/MetaDataMappingServiceTests.java b/core/src/test/java/org/elasticsearch/cluster/metadata/MetaDataMappingServiceTests.java new file mode 100644 index 00000000000..c6ce30e2a52 --- /dev/null +++ b/core/src/test/java/org/elasticsearch/cluster/metadata/MetaDataMappingServiceTests.java @@ -0,0 +1,66 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.elasticsearch.cluster.metadata; + +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.index.IndexService; +import org.elasticsearch.index.mapper.DocumentMapper; +import org.elasticsearch.test.ESSingleNodeTestCase; + +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; + +public class MetaDataMappingServiceTests extends ESSingleNodeTestCase { + + // Tests _parent meta field logic, because part of the validation is in MetaDataMappingService + public void testAddChildTypePointingToAlreadyExistingType() throws Exception { + createIndex("test", Settings.EMPTY, "type", "field", "type=keyword"); + + // Shouldn't be able the add the _parent field pointing to an already existing type, which isn't a parent type + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> client().admin() + .indices() + .preparePutMapping("test") + .setType("child") + .setSource("_parent", "type=type") + .get()); + assertThat(e.getMessage(), + equalTo("can't add a _parent field that points to an already existing type, that isn't already a parent")); + } + + // Tests _parent meta field logic, because part of the validation is in MetaDataMappingService + public void testAddExtraChildTypePointingToAlreadyParentExistingType() throws Exception { + IndexService indexService = createIndex("test", client().admin().indices().prepareCreate("test") + .addMapping("parent") + .addMapping("child1", "_parent", "type=parent") + ); + + // adding the extra child type that points to an already existing parent type is allowed: + client().admin() + .indices() + .preparePutMapping("test") + .setType("child2") + .setSource("_parent", "type=parent") + .get(); + + DocumentMapper documentMapper = indexService.mapperService().documentMapper("child2"); + assertThat(documentMapper.parentFieldMapper().type(), equalTo("parent")); + assertThat(documentMapper.parentFieldMapper().active(), is(true)); + } + +} diff --git a/core/src/test/java/org/elasticsearch/search/child/ChildQuerySearchIT.java b/core/src/test/java/org/elasticsearch/search/child/ChildQuerySearchIT.java index 1a258abc9cc..90f5c65c066 100644 --- a/core/src/test/java/org/elasticsearch/search/child/ChildQuerySearchIT.java +++ b/core/src/test/java/org/elasticsearch/search/child/ChildQuerySearchIT.java @@ -1321,29 +1321,6 @@ public class ChildQuerySearchIT extends ESIntegTestCase { } } - public void testAddParentFieldAfterIndexingParentDocButBeforeIndexingChildDoc() throws Exception { - assertAcked(prepareCreate("test") - .setSettings(Settings.builder() - .put(indexSettings()) - .put("index.refresh_interval", -1))); - ensureGreen(); - - String parentId = "p1"; - client().prepareIndex("test", "parent", parentId).setSource("p_field", "1").get(); - refresh(); - - try { - assertAcked(client().admin() - .indices() - .preparePutMapping("test") - .setType("child") - .setSource("_parent", "type=parent")); - fail("Shouldn't be able the add the _parent field pointing to an already existing parent type"); - } catch (IllegalArgumentException e) { - assertThat(e.getMessage(), equalTo("can't add a _parent field that points to an already existing type")); - } - } - public void testParentChildCaching() throws Exception { assertAcked(prepareCreate("test") .setSettings(