Merge pull request #17065 from rjernst/dynamic_mappers_are_hard
Fix dynamic mapper when its parent already has an update
This commit is contained in:
commit
efd59f531d
|
@ -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<ObjectMapper> 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<ObjectMapper> 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<ObjectMapper> 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<ObjectMapper> 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;
|
||||
|
|
|
@ -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<Mapper> updates = Collections.singletonList(new MockFieldMapper("a.foo"));
|
||||
List<Mapper> 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<Mapper> 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<Mapper> updates = Collections.singletonList(new MockFieldMapper("a.b.foo"));
|
||||
List<Mapper> 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<Mapper> 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 {
|
||||
|
|
Loading…
Reference in New Issue