Skip to content

Commit

Permalink
Mapping: fixes dynamic mapping of geo_point fields
Browse files Browse the repository at this point in the history 
If a dynamic mapping for a geo_point field is defined and the first document specifies the value of the field as a geo_point array, the dynamic mapping throws an error as the array is broken into individual number before consulting the dynamic mapping configuration.  This change adds a check of the dynamic mapping before the array is split into individual numbers.

Closes #6939
  • Loading branch information
@colings86 @areek
colings86 authored and areek committed Sep 8, 2014
1 parent d9c8d97 commit 4631615
Show file tree
Hide file tree
Showing 2 changed files with 110 additions and 43 deletions.
132 changes: 89 additions & 43 deletions src/main/java/org/elasticsearch/index/mapper/object/ObjectMapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -575,34 +575,7 @@ private void serializeObject(final ParseContext context, String currentFieldName
}
BuilderContext builderContext = new BuilderContext(context.indexSettings(), context.path());
objectMapper = builder.build(builderContext);
// ...now re add it
context.path().add(currentFieldName);
context.setMappingsModified();

if (context.isWithinNewMapper()) {
// within a new mapper, no need to traverse, just parse
objectMapper.parse(context);
} else {
// create a context of new mapper, so we batch aggregate all the changes within
// this object mapper once, and traverse all of them to add them in a single go
context.setWithinNewMapper();
try {
objectMapper.parse(context);
FieldMapperListener.Aggregator newFields = new FieldMapperListener.Aggregator();
ObjectMapperListener.Aggregator newObjects = new ObjectMapperListener.Aggregator();
objectMapper.traverse(newFields);
objectMapper.traverse(newObjects);
// callback on adding those fields!
context.docMapper().addFieldMappers(newFields.mappers);
context.docMapper().addObjectMappers(newObjects.mappers);
} finally {
context.clearWithinNewMapper();
}
}

// only put after we traversed and did the callbacks, so other parsing won't see it only after we
// properly traversed it and adding the mappers
putMapper(objectMapper);
putDynamicMapper(context, currentFieldName, objectMapper);
} else {
objectMapper.parse(context);
}
Expand All @@ -622,22 +595,95 @@ private void serializeArray(ParseContext context, String lastFieldName) throws I
if (mapper != null && mapper instanceof ArrayValueMapperParser) {
mapper.parse(context);
} else {
XContentParser parser = context.parser();
XContentParser.Token token;
while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) {
if (token == XContentParser.Token.START_OBJECT) {
serializeObject(context, lastFieldName);
} else if (token == XContentParser.Token.START_ARRAY) {
serializeArray(context, lastFieldName);
} else if (token == XContentParser.Token.FIELD_NAME) {
lastFieldName = parser.currentName();
} else if (token == XContentParser.Token.VALUE_NULL) {
serializeNullValue(context, lastFieldName);
} else if (token == null) {
throw new MapperParsingException("object mapping for [" + name + "] with array for [" + arrayFieldName + "] tried to parse as array, but got EOF, is there a mismatch in types for the same field?");
} else {
serializeValue(context, lastFieldName, token);

Dynamic dynamic = this.dynamic;
if (dynamic == null) {
dynamic = context.root().dynamic();
}
if (dynamic == Dynamic.STRICT) {
throw new StrictDynamicMappingException(fullPath, arrayFieldName);
} else if (dynamic == Dynamic.TRUE) {
// we sync here just so we won't add it twice. Its not the end of the world
// to sync here since next operations will get it before
synchronized (mutex) {
mapper = mappers.get(arrayFieldName);
if (mapper == null) {
Mapper.Builder builder = context.root().findTemplateBuilder(context, arrayFieldName, "object");
if (builder == null) {
serializeNonDynamicArray(context, lastFieldName, arrayFieldName);
return;
}
BuilderContext builderContext = new BuilderContext(context.indexSettings(), context.path());
mapper = builder.build(builderContext);
if (mapper != null && mapper instanceof ArrayValueMapperParser) {
putDynamicMapper(context, arrayFieldName, mapper);
} else {
serializeNonDynamicArray(context, lastFieldName, arrayFieldName);
}
} else {

serializeNonDynamicArray(context, lastFieldName, arrayFieldName);
}
}
} else {

serializeNonDynamicArray(context, lastFieldName, arrayFieldName);
}
}
}

private void putDynamicMapper(ParseContext context, String arrayFieldName, Mapper mapper) throws IOException {
// ...now re add it
context.path().add(arrayFieldName);
context.setMappingsModified();

if (context.isWithinNewMapper()) {
// within a new mapper, no need to traverse,
// just parse
mapper.parse(context);
} else {
// create a context of new mapper, so we batch
// aggregate all the changes within
// this object mapper once, and traverse all of
// them to add them in a single go
context.setWithinNewMapper();
try {
mapper.parse(context);
FieldMapperListener.Aggregator newFields = new FieldMapperListener.Aggregator();
ObjectMapperListener.Aggregator newObjects = new ObjectMapperListener.Aggregator();
mapper.traverse(newFields);
mapper.traverse(newObjects);
// callback on adding those fields!
context.docMapper().addFieldMappers(newFields.mappers);
context.docMapper().addObjectMappers(newObjects.mappers);
} finally {
context.clearWithinNewMapper();
}
}

// only put after we traversed and did the
// callbacks, so other parsing won't see it only
// after we
// properly traversed it and adding the mappers
putMapper(mapper);
}

private void serializeNonDynamicArray(ParseContext context, String lastFieldName, String arrayFieldName) throws IOException {
XContentParser parser = context.parser();
XContentParser.Token token;
while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) {
if (token == XContentParser.Token.START_OBJECT) {
serializeObject(context, lastFieldName);
} else if (token == XContentParser.Token.START_ARRAY) {
serializeArray(context, lastFieldName);
} else if (token == XContentParser.Token.FIELD_NAME) {
lastFieldName = parser.currentName();
} else if (token == XContentParser.Token.VALUE_NULL) {
serializeNullValue(context, lastFieldName);
} else if (token == null) {
throw new MapperParsingException("object mapping for [" + name + "] with array for [" + arrayFieldName + "] tried to parse as array, but got EOF, is there a mismatch in types for the same field?");
} else {
serializeValue(context, lastFieldName, token);
}
}
}
Expand Down
21 changes: 21 additions & 0 deletions src/test/java/org/elasticsearch/index/mapper/geo/LatLonMappingGeoPointTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,27 @@ public void testLonLatArray() throws Exception {
assertThat(doc.rootDoc().get("point"), equalTo("1.2,1.3"));
}

@Test
public void testLonLatArrayDynamic() throws Exception {
String mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
.startArray("dynamic_templates").startObject()
.startObject("point").field("match", "point*").startObject("mapping").field("type", "geo_point").field("lat_lon", true).endObject().endObject()
.endObject().endArray()
.endObject().endObject().string();

DocumentMapper defaultMapper = createIndex("test").mapperService().documentMapperParser().parse(mapping);

ParsedDocument doc = defaultMapper.parse("type", "1", XContentFactory.jsonBuilder()
.startObject()
.startArray("point").value(1.3).value(1.2).endArray()
.endObject()
.bytes());

assertThat(doc.rootDoc().getField("point.lat"), notNullValue());
assertThat(doc.rootDoc().getField("point.lon"), notNullValue());
assertThat(doc.rootDoc().get("point"), equalTo("1.2,1.3"));
}

@Test
public void testLonLatArrayStored() throws Exception {
String mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
Expand Down

0 comments on commit 4631615

Please sign in to comment.