Skip to content

Commit

Permalink
Mappings: Support dots in field names when mapping exists
Browse files Browse the repository at this point in the history 
In 2.0 we began restricting fields to not contains dots in their names.
This change adds back part of dots in fieldnames support. Specifically,
it allows indexing documents that contain dots in the field names, when
the correct corresponding mappers exist. For example, if mappings
contain an object field `foo`, and a subfield `bar`, then indexing a
document with `foo.bar` will work.

see elastic#15951
  • Loading branch information
@rjernst
rjernst committed Apr 14, 2016
1 parent f35cfc3 commit 125473d
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 38 deletions.
69 changes: 31 additions & 38 deletions core/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@
import org.elasticsearch.index.mapper.internal.UidFieldMapper;
import org.elasticsearch.index.mapper.object.ArrayValueMapperParser;
import org.elasticsearch.index.mapper.object.ObjectMapper;
import org.elasticsearch.index.mapper.object.RootObjectMapper;

/** A parser for documents, given mappings from a DocumentMapper */
final class DocumentParser implements Closeable {
Expand Down Expand Up @@ -474,7 +473,7 @@ private static void parseObjectOrField(ParseContext context, Mapper mapper) thro
context.addDynamicMapper(update);
}
if (fieldMapper.copyTo() != null) {
parseCopyFields(context, fieldMapper, fieldMapper.copyTo().copyToFields());
parseCopyFields(context, fieldMapper.copyTo().copyToFields());
}
}
}
Expand All @@ -484,14 +483,11 @@ private static ObjectMapper parseObject(final ParseContext context, ObjectMapper
context.path().add(currentFieldName);

ObjectMapper update = null;
Mapper objectMapper = mapper.getMapper(currentFieldName);
Mapper objectMapper = getMapper(mapper, currentFieldName);
if (objectMapper != null) {
parseObjectOrField(context, objectMapper);
} else {
ObjectMapper.Dynamic dynamic = mapper.dynamic();
if (dynamic == null) {
dynamic = dynamicOrDefault(context.root().dynamic());
}
ObjectMapper.Dynamic dynamic = dynamicOrDefault(mapper, context.root().dynamic());
if (dynamic == ObjectMapper.Dynamic.STRICT) {
throw new StrictDynamicMappingException(mapper.fullPath(), currentFieldName);
} else if (dynamic == ObjectMapper.Dynamic.TRUE) {
Expand All @@ -500,10 +496,6 @@ private static ObjectMapper parseObject(final ParseContext context, ObjectMapper
Mapper.Builder builder = context.root().findTemplateBuilder(context, currentFieldName, "object");
if (builder == null) {
builder = new ObjectMapper.Builder(currentFieldName).enabled(true);
// if this is a non root object, then explicitly set the dynamic behavior if set
if (!(mapper instanceof RootObjectMapper) && mapper.dynamic() != ObjectMapper.Defaults.DYNAMIC) {
((ObjectMapper.Builder) builder).dynamic(mapper.dynamic());
}
}
Mapper.BuilderContext builderContext = new Mapper.BuilderContext(context.indexSettings(), context.path());
objectMapper = builder.build(builderContext);
Expand All @@ -522,7 +514,7 @@ private static ObjectMapper parseObject(final ParseContext context, ObjectMapper

private static void parseArray(ParseContext context, ObjectMapper parentMapper, String lastFieldName) throws IOException {
String arrayFieldName = lastFieldName;
Mapper mapper = parentMapper.getMapper(lastFieldName);
Mapper mapper = getMapper(parentMapper, lastFieldName);
if (mapper != null) {
// There is a concrete mapper for this field already. Need to check if the mapper
// expects an array, if so we pass the context straight to the mapper and if not
Expand All @@ -534,10 +526,7 @@ private static void parseArray(ParseContext context, ObjectMapper parentMapper,
}
} else {

ObjectMapper.Dynamic dynamic = parentMapper.dynamic();
if (dynamic == null) {
dynamic = dynamicOrDefault(context.root().dynamic());
}
ObjectMapper.Dynamic dynamic = dynamicOrDefault(parentMapper, context.root().dynamic());
if (dynamic == ObjectMapper.Dynamic.STRICT) {
throw new StrictDynamicMappingException(parentMapper.fullPath(), arrayFieldName);
} else if (dynamic == ObjectMapper.Dynamic.TRUE) {
Expand Down Expand Up @@ -587,7 +576,7 @@ private static void parseValue(final ParseContext context, ObjectMapper parentMa
if (currentFieldName == null) {
throw new MapperParsingException("object mapping [" + parentMapper.name() + "] trying to serialize a value with no field associated with it, current value [" + context.parser().textOrNull() + "]");
}
Mapper mapper = parentMapper.getMapper(currentFieldName);
Mapper mapper = getMapper(parentMapper, currentFieldName);
if (mapper != null) {
parseObjectOrField(context, mapper);
} else {
Expand All @@ -597,7 +586,7 @@ private static void parseValue(final ParseContext context, ObjectMapper parentMa

private static void parseNullValue(ParseContext context, ObjectMapper parentMapper, String lastFieldName) throws IOException {
// we can only handle null values if we have mappings for them
Mapper mapper = parentMapper.getMapper(lastFieldName);
Mapper mapper = getMapper(parentMapper, lastFieldName);
if (mapper != null) {
// TODO: passing null to an object seems bogus?
parseObjectOrField(context, mapper);
Expand Down Expand Up @@ -811,10 +800,7 @@ private static Mapper.Builder<?,?> createBuilderFromDynamicValue(final ParseCont
}

private static void parseDynamicValue(final ParseContext context, ObjectMapper parentMapper, String currentFieldName, XContentParser.Token token) throws IOException {
ObjectMapper.Dynamic dynamic = parentMapper.dynamic();
if (dynamic == null) {
dynamic = dynamicOrDefault(context.root().dynamic());
}
ObjectMapper.Dynamic dynamic = dynamicOrDefault(parentMapper, context.root().dynamic());
if (dynamic == ObjectMapper.Dynamic.STRICT) {
throw new StrictDynamicMappingException(parentMapper.fullPath(), currentFieldName);
}
Expand All @@ -824,12 +810,11 @@ private static void parseDynamicValue(final ParseContext context, ObjectMapper p
final String path = context.path().pathAsText(currentFieldName);
final Mapper.BuilderContext builderContext = new Mapper.BuilderContext(context.indexSettings(), context.path());
final MappedFieldType existingFieldType = context.mapperService().fullName(path);
Mapper.Builder builder = null;
final Mapper.Builder builder;
if (existingFieldType != null) {
// create a builder of the same type
builder = createBuilderFromFieldType(context, existingFieldType, currentFieldName);
}
if (builder == null) {
} else {
builder = createBuilderFromDynamicValue(context, token, currentFieldName);
}
Mapper mapper = builder.build(builderContext);
Expand All @@ -843,7 +828,7 @@ private static void parseDynamicValue(final ParseContext context, ObjectMapper p
}

/** Creates instances of the fields that the current field should be copied to */
private static void parseCopyFields(ParseContext context, FieldMapper fieldMapper, List<String> copyToFields) throws IOException {
private static void parseCopyFields(ParseContext context, List<String> copyToFields) throws IOException {
if (!context.isWithinCopyTo() && copyToFields.isEmpty() == false) {
context = context.createCopyToContext();
for (String field : copyToFields) {
Expand Down Expand Up @@ -888,21 +873,14 @@ private static void parseCopy(String field, ParseContext context) throws IOExcep
mapper = context.docMapper().objectMappers().get(context.path().pathAsText(paths[i]));
if (mapper == null) {
// One mapping is missing, check if we are allowed to create a dynamic one.
ObjectMapper.Dynamic dynamic = parent.dynamic();
if (dynamic == null) {
dynamic = dynamicOrDefault(context.root().dynamic());
}
ObjectMapper.Dynamic dynamic = dynamicOrDefault(parent, context.root().dynamic());

switch (dynamic) {
case STRICT:
throw new StrictDynamicMappingException(parent.fullPath(), paths[i]);
case TRUE:
Mapper.Builder builder = context.root().findTemplateBuilder(context, paths[i], "object");
if (builder == null) {
// if this is a non root object, then explicitly set the dynamic behavior if set
if (!(parent instanceof RootObjectMapper) && parent.dynamic() != ObjectMapper.Defaults.DYNAMIC) {
((ObjectMapper.Builder) builder).dynamic(parent.dynamic());
}
builder = new ObjectMapper.Builder(paths[i]).enabled(true);
}
Mapper.BuilderContext builderContext = new Mapper.BuilderContext(context.indexSettings(), context.path());
Expand All @@ -915,8 +893,6 @@ private static void parseCopy(String field, ParseContext context) throws IOExcep
case FALSE:
// Maybe we should log something to tell the user that the copy_to is ignored in this case.
break;
default:
throw new AssertionError("Unexpected dynamic type " + dynamic);

}
}
Expand All @@ -929,8 +905,25 @@ private static void parseCopy(String field, ParseContext context) throws IOExcep
}
}

private static ObjectMapper.Dynamic dynamicOrDefault(ObjectMapper.Dynamic dynamic) {
return dynamic == null ? ObjectMapper.Dynamic.TRUE : dynamic;
private static ObjectMapper.Dynamic dynamicOrDefault(ObjectMapper parentMapper, ObjectMapper.Dynamic dynamicDefault) {
ObjectMapper.Dynamic dynamic = parentMapper.dynamic();
if (dynamic == null) {
return dynamicDefault == null ? ObjectMapper.Dynamic.TRUE : dynamicDefault;
}
return dynamic;
}

// looks up a child mapper, but takes into account field names that expand to objects
static Mapper getMapper(ObjectMapper objectMapper, String fieldName) {
String[] subfields = fieldName.split("\\.");
for (int i = 0; i < subfields.length - 1; ++i) {
Mapper mapper = objectMapper.getMapper(subfields[i]);
if (mapper == null || (mapper instanceof ObjectMapper) == false) {
return null;
}
objectMapper = (ObjectMapper)mapper;
}
return objectMapper.getMapper(subfields[subfields.length - 1]);
}

@Override
Expand Down
27 changes: 27 additions & 0 deletions core/src/test/java/org/elasticsearch/index/mapper/DocumentParserTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,33 @@ public void testFieldDisabled() throws Exception {
assertNotNull(doc.rootDoc().getField(UidFieldMapper.NAME));
}

public void testDotsWithExistingMapper() throws Exception {
DocumentMapperParser mapperParser = createIndex("test").mapperService().documentMapperParser();
String mapping = XContentFactory.jsonBuilder().startObject().startObject("type").startObject("properties")
.startObject("foo").startObject("properties")
.startObject("bar").startObject("properties")
.startObject("baz").field("type", "integer")
.endObject().endObject().endObject().endObject().endObject().endObject().endObject().endObject().string();
DocumentMapper mapper = mapperParser.parse("type", new CompressedXContent(mapping));

BytesReference bytes = XContentFactory.jsonBuilder()
.startObject()
.field("foo.bar.baz", 123)
.startObject("foo")
.field("bar.baz", 456)
.endObject()
.startObject("foo.bar")
.field("baz", 789)
.endObject()
.endObject().bytes();
ParsedDocument doc = mapper.parse("test", "type", "1", bytes);
String[] values = doc.rootDoc().getValues("foo.bar.baz");
assertEquals(3, values.length);
assertEquals("123", values[0]);
assertEquals("456", values[1]);
assertEquals("789", values[2]);
}

DocumentMapper createDummyMapping(MapperService mapperService) throws Exception {
String mapping = jsonBuilder().startObject().startObject("type").startObject("properties")
.startObject("y").field("type", "object").endObject()
Expand Down

0 comments on commit 125473d

Please sign in to comment.