diff --git a/core/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java b/core/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java index 36c1cf106f4..8c8ded9b543 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java @@ -23,18 +23,14 @@ import java.io.Closeable; import java.io.IOException; import java.util.ArrayList; import java.util.Collections; -import java.util.HashSet; import java.util.Iterator; import java.util.List; -import java.util.Set; import org.apache.lucene.document.Field; -import org.apache.lucene.index.IndexOptions; import org.apache.lucene.index.IndexableField; import org.apache.lucene.util.CloseableThreadLocal; import org.elasticsearch.common.Strings; import org.elasticsearch.common.joda.FormatDateTimeFormatter; -import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.IndexSettings; @@ -240,28 +236,26 @@ final class DocumentParser implements Closeable { } previousMapper = newMapper; String[] nameParts = newMapper.name().split("\\."); - // find common elements with the previously processed dynamic mapper - int keepBefore = 1; - while (keepBefore < parentMappers.size() && - parentMappers.get(keepBefore).simpleName().equals(nameParts[keepBefore - 1])) { - ++keepBefore; - } - popMappers(parentMappers, keepBefore, true); - if (keepBefore < nameParts.length) { - String updateParentName = nameParts[keepBefore - 1]; - final ObjectMapper lastParent = parentMappers.get(parentMappers.size() - 1); - Mapper updateParent = lastParent.getMapper(updateParentName); - if (updateParent == null) { - // the parent we need is not on the stack, so look it up in the full mappings - if (keepBefore > 1) { - // only prefix with parent mapper if the parent mapper isn't the root (which has a fake name) - updateParentName = lastParent.name() + '.' + updateParentName; - } - updateParent = docMapper.objectMappers().get(updateParentName); - } - assert updateParent instanceof ObjectMapper; - newMapper = createUpdate((ObjectMapper)updateParent, nameParts, keepBefore, newMapper); + // We first need the stack to only contain mappers in common with the previously processed mapper + // For example, if the first mapper processed was a.b.c, and we now have a.d, the stack will contain + // a.b, and we want to merge b back into the stack so it just contains a + int i = removeUncommonMappers(parentMappers, nameParts); + + // Then we need to add back mappers that may already exist within the stack, but are not on it. + // For example, if we processed a.b, followed by an object mapper a.c.d, and now are adding a.c.d.e + // then the stack will only have a on it because we will have already merged a.c.d into the stack. + // So we need to pull a.c, followed by a.c.d, onto the stack so e can be added to the end. + i = expandCommonMappers(parentMappers, nameParts, i); + + // If there are still parents of the new mapper which are not on the stack, we need to pull them + // from the existing mappings. In order to maintain the invariant that the stack only contains + // fields which are updated, we cannot simply add the existing mappers to the stack, since they + // may have other subfields which will not be updated. Instead, we pull the mapper from the existing + // mappings, and build an update with only the new mapper and its parents. This then becomes our + // "new mapper", and can be added to the stack. + if (i < nameParts.length - 1) { + newMapper = createExistingMapperUpdate(parentMappers, nameParts, i, docMapper, newMapper); } if (newMapper instanceof ObjectMapper) { @@ -299,12 +293,56 @@ final class DocumentParser implements Closeable { parentMappers.set(lastIndex, withNewMapper); } + /** + * Removes mappers that exist on the stack, but are not part of the path of the current nameParts, + * Returns the next unprocessed index from nameParts. + */ + private static int removeUncommonMappers(List parentMappers, String[] nameParts) { + int keepBefore = 1; + while (keepBefore < parentMappers.size() && + parentMappers.get(keepBefore).simpleName().equals(nameParts[keepBefore - 1])) { + ++keepBefore; + } + popMappers(parentMappers, keepBefore, true); + return keepBefore - 1; + } + + /** + * Adds mappers from the end of the stack that exist as updates within those mappers. + * Returns the next unprocessed index from nameParts. + */ + private static int expandCommonMappers(List parentMappers, String[] nameParts, int i) { + ObjectMapper last = parentMappers.get(parentMappers.size() - 1); + while (i < nameParts.length - 1 && last.getMapper(nameParts[i]) != null) { + Mapper newLast = last.getMapper(nameParts[i]); + assert newLast instanceof ObjectMapper; + parentMappers.add((ObjectMapper)newLast); + ++i; + } + return i; + } + + /** Creates an update for intermediate object mappers that are not on the stack, but parents of newMapper. */ + private static ObjectMapper createExistingMapperUpdate(List parentMappers, String[] nameParts, int i, + DocumentMapper docMapper, Mapper newMapper) { + String updateParentName = nameParts[i]; + final ObjectMapper lastParent = parentMappers.get(parentMappers.size() - 1); + if (parentMappers.size() > 1) { + // only prefix with parent mapper if the parent mapper isn't the root (which has a fake name) + updateParentName = lastParent.name() + '.' + nameParts[i]; + } + ObjectMapper updateParent = docMapper.objectMappers().get(updateParentName); + assert updateParent != null : updateParentName + " doesn't exist"; + return createUpdate(updateParent, nameParts, i + 1, newMapper); + } + /** Build an update for the parent which will contain the given mapper and any intermediate fields. */ private static ObjectMapper createUpdate(ObjectMapper parent, String[] nameParts, int i, Mapper mapper) { List parentMappers = new ArrayList<>(); ObjectMapper previousIntermediate = parent; for (; i < nameParts.length - 1; ++i) { Mapper intermediate = previousIntermediate.getMapper(nameParts[i]); + assert intermediate != null : "Field " + previousIntermediate.name() + " does not have a subfield " + nameParts[i]; assert intermediate instanceof ObjectMapper; parentMappers.add((ObjectMapper)intermediate); previousIntermediate = (ObjectMapper)intermediate; diff --git a/core/src/test/java/org/elasticsearch/index/mapper/DocumentParserTests.java b/core/src/test/java/org/elasticsearch/index/mapper/DocumentParserTests.java index cbc858b642d..e4d1e306af3 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/DocumentParserTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/DocumentParserTests.java @@ -72,9 +72,10 @@ public class DocumentParserTests extends ESSingleNodeTestCase { DocumentMapper createDummyMapping(MapperService mapperService) throws Exception { String mapping = jsonBuilder().startObject().startObject("type").startObject("properties") - .startObject("a").startObject("properties") - .startObject("b").field("type", "object").startObject("properties") - .startObject("c").field("type", "object") + .startObject("y").field("type", "object").endObject() + .startObject("x").startObject("properties") + .startObject("subx").field("type", "object").startObject("properties") + .startObject("subsubx").field("type", "object") .endObject().endObject().endObject().endObject().endObject().endObject().endObject().endObject().string(); DocumentMapper defaultMapper = mapperService.documentMapperParser().parse("type", new CompressedXContent(mapping)); @@ -109,40 +110,55 @@ public class DocumentParserTests extends ESSingleNodeTestCase { public void testSubfieldMappingUpdate() throws Exception { DocumentMapper docMapper = createDummyMapping(createIndex("test").mapperService()); - List updates = Collections.singletonList(new MockFieldMapper("a.foo")); + List updates = Collections.singletonList(new MockFieldMapper("x.foo")); Mapping mapping = DocumentParser.createDynamicUpdate(docMapper.mapping(), docMapper, updates); - Mapper aMapper = mapping.root().getMapper("a"); - assertNotNull(aMapper); - assertTrue(aMapper instanceof ObjectMapper); - assertNotNull(((ObjectMapper)aMapper).getMapper("foo")); - assertNull(((ObjectMapper)aMapper).getMapper("b")); + Mapper xMapper = mapping.root().getMapper("x"); + assertNotNull(xMapper); + assertTrue(xMapper instanceof ObjectMapper); + assertNotNull(((ObjectMapper)xMapper).getMapper("foo")); + assertNull(((ObjectMapper)xMapper).getMapper("subx")); } public void testMultipleSubfieldMappingUpdate() throws Exception { DocumentMapper docMapper = createDummyMapping(createIndex("test").mapperService()); List updates = new ArrayList<>(); - updates.add(new MockFieldMapper("a.foo")); - updates.add(new MockFieldMapper("a.bar")); + updates.add(new MockFieldMapper("x.foo")); + updates.add(new MockFieldMapper("x.bar")); Mapping mapping = DocumentParser.createDynamicUpdate(docMapper.mapping(), docMapper, updates); - Mapper aMapper = mapping.root().getMapper("a"); - assertNotNull(aMapper); - assertTrue(aMapper instanceof ObjectMapper); - assertNotNull(((ObjectMapper)aMapper).getMapper("foo")); - assertNotNull(((ObjectMapper)aMapper).getMapper("bar")); - assertNull(((ObjectMapper)aMapper).getMapper("b")); + Mapper xMapper = mapping.root().getMapper("x"); + assertNotNull(xMapper); + assertTrue(xMapper instanceof ObjectMapper); + assertNotNull(((ObjectMapper)xMapper).getMapper("foo")); + assertNotNull(((ObjectMapper)xMapper).getMapper("bar")); + assertNull(((ObjectMapper)xMapper).getMapper("subx")); } public void testDeepSubfieldMappingUpdate() throws Exception { DocumentMapper docMapper = createDummyMapping(createIndex("test").mapperService()); - List updates = Collections.singletonList(new MockFieldMapper("a.b.foo")); + List updates = Collections.singletonList(new MockFieldMapper("x.subx.foo")); Mapping mapping = DocumentParser.createDynamicUpdate(docMapper.mapping(), docMapper, updates); - Mapper aMapper = mapping.root().getMapper("a"); - assertNotNull(aMapper); - assertTrue(aMapper instanceof ObjectMapper); - Mapper bMapper = ((ObjectMapper)aMapper).getMapper("b"); - assertTrue(bMapper instanceof ObjectMapper); - assertNotNull(((ObjectMapper)bMapper).getMapper("foo")); - assertNull(((ObjectMapper)bMapper).getMapper("c")); + Mapper xMapper = mapping.root().getMapper("x"); + assertNotNull(xMapper); + assertTrue(xMapper instanceof ObjectMapper); + Mapper subxMapper = ((ObjectMapper)xMapper).getMapper("subx"); + assertTrue(subxMapper instanceof ObjectMapper); + assertNotNull(((ObjectMapper)subxMapper).getMapper("foo")); + assertNull(((ObjectMapper)subxMapper).getMapper("subsubx")); + } + + public void testDeepSubfieldAfterSubfieldMappingUpdate() throws Exception { + DocumentMapper docMapper = createDummyMapping(createIndex("test").mapperService()); + List updates = new ArrayList<>(); + updates.add(new MockFieldMapper("x.a")); + updates.add(new MockFieldMapper("x.subx.b")); + Mapping mapping = DocumentParser.createDynamicUpdate(docMapper.mapping(), docMapper, updates); + Mapper xMapper = mapping.root().getMapper("x"); + assertNotNull(xMapper); + assertTrue(xMapper instanceof ObjectMapper); + assertNotNull(((ObjectMapper)xMapper).getMapper("a")); + Mapper subxMapper = ((ObjectMapper)xMapper).getMapper("subx"); + assertTrue(subxMapper instanceof ObjectMapper); + assertNotNull(((ObjectMapper)subxMapper).getMapper("b")); } public void testObjectMappingUpdate() throws Exception {