[GEO] Add merge conflicts to GeoShapeFieldMapper
Prevents the user from changing strategies, tree, tree_level or precision. distance_error_pct changes are allowed as they do not compromise the integrity of the index. A separate issue is open for allowing users to change tree_level or precision.
This commit is contained in:
parent
a8a35d7c29
commit
754856289e
|
@ -41,6 +41,8 @@ import org.elasticsearch.index.fielddata.FieldDataType;
|
||||||
import org.elasticsearch.index.mapper.FieldMapper;
|
import org.elasticsearch.index.mapper.FieldMapper;
|
||||||
import org.elasticsearch.index.mapper.Mapper;
|
import org.elasticsearch.index.mapper.Mapper;
|
||||||
import org.elasticsearch.index.mapper.MapperParsingException;
|
import org.elasticsearch.index.mapper.MapperParsingException;
|
||||||
|
import org.elasticsearch.index.mapper.MergeContext;
|
||||||
|
import org.elasticsearch.index.mapper.MergeMappingException;
|
||||||
import org.elasticsearch.index.mapper.ParseContext;
|
import org.elasticsearch.index.mapper.ParseContext;
|
||||||
import org.elasticsearch.index.mapper.core.AbstractFieldMapper;
|
import org.elasticsearch.index.mapper.core.AbstractFieldMapper;
|
||||||
|
|
||||||
|
@ -262,6 +264,50 @@ public class GeoShapeFieldMapper extends AbstractFieldMapper<String> {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Override
|
||||||
|
public void merge(Mapper mergeWith, MergeContext mergeContext) throws MergeMappingException {
|
||||||
|
super.merge(mergeWith, mergeContext);
|
||||||
|
if (!this.getClass().equals(mergeWith.getClass())) {
|
||||||
|
mergeContext.addConflict("mapper [" + names.fullName() + "] has different field type");
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
final GeoShapeFieldMapper fieldMergeWith = (GeoShapeFieldMapper) mergeWith;
|
||||||
|
if (!mergeContext.mergeFlags().simulate()) {
|
||||||
|
final PrefixTreeStrategy mergeWithStrategy = fieldMergeWith.defaultStrategy;
|
||||||
|
|
||||||
|
// prevent user from changing strategies
|
||||||
|
if (!(this.defaultStrategy.getClass().equals(mergeWithStrategy.getClass()))) {
|
||||||
|
mergeContext.addConflict("mapper [" + names.fullName() + "] has different strategy");
|
||||||
|
}
|
||||||
|
|
||||||
|
final SpatialPrefixTree grid = this.defaultStrategy.getGrid();
|
||||||
|
final SpatialPrefixTree mergeGrid = mergeWithStrategy.getGrid();
|
||||||
|
|
||||||
|
// prevent user from changing trees (changes encoding)
|
||||||
|
if (!grid.getClass().equals(mergeGrid.getClass())) {
|
||||||
|
mergeContext.addConflict("mapper [" + names.fullName() + "] has different tree");
|
||||||
|
}
|
||||||
|
|
||||||
|
// TODO we should allow this, but at the moment levels is used to build bookkeeping variables
|
||||||
|
// in lucene's SpatialPrefixTree implementations, need a patch to correct that first
|
||||||
|
if (grid.getMaxLevels() != mergeGrid.getMaxLevels()) {
|
||||||
|
mergeContext.addConflict("mapper [" + names.fullName() + "] has different tree_levels or precision");
|
||||||
|
}
|
||||||
|
|
||||||
|
// bail if there were merge conflicts
|
||||||
|
if (mergeContext.hasConflicts()) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
// change distance error percent
|
||||||
|
this.defaultStrategy.setDistErrPct(mergeWithStrategy.getDistErrPct());
|
||||||
|
|
||||||
|
// change orientation - this is allowed because existing dateline spanning shapes
|
||||||
|
// have already been unwound and segmented
|
||||||
|
this.shapeOrientation = fieldMergeWith.shapeOrientation;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
protected void parseCreateField(ParseContext context, List<Field> fields) throws IOException {
|
protected void parseCreateField(ParseContext context, List<Field> fields) throws IOException {
|
||||||
}
|
}
|
||||||
|
|
|
@ -19,6 +19,7 @@
|
||||||
package org.elasticsearch.index.mapper.geo;
|
package org.elasticsearch.index.mapper.geo;
|
||||||
|
|
||||||
import org.apache.lucene.spatial.prefix.PrefixTreeStrategy;
|
import org.apache.lucene.spatial.prefix.PrefixTreeStrategy;
|
||||||
|
import org.apache.lucene.spatial.prefix.RecursivePrefixTreeStrategy;
|
||||||
import org.apache.lucene.spatial.prefix.tree.GeohashPrefixTree;
|
import org.apache.lucene.spatial.prefix.tree.GeohashPrefixTree;
|
||||||
import org.apache.lucene.spatial.prefix.tree.QuadPrefixTree;
|
import org.apache.lucene.spatial.prefix.tree.QuadPrefixTree;
|
||||||
import org.elasticsearch.common.geo.GeoUtils;
|
import org.elasticsearch.common.geo.GeoUtils;
|
||||||
|
@ -31,9 +32,13 @@ import org.elasticsearch.test.ElasticsearchSingleNodeTest;
|
||||||
import org.junit.Test;
|
import org.junit.Test;
|
||||||
|
|
||||||
import java.io.IOException;
|
import java.io.IOException;
|
||||||
|
import java.util.ArrayList;
|
||||||
|
import java.util.Arrays;
|
||||||
|
|
||||||
|
import static org.elasticsearch.index.mapper.DocumentMapper.MergeFlags.mergeFlags;
|
||||||
import static org.hamcrest.Matchers.equalTo;
|
import static org.hamcrest.Matchers.equalTo;
|
||||||
import static org.hamcrest.Matchers.instanceOf;
|
import static org.hamcrest.Matchers.instanceOf;
|
||||||
|
import static org.hamcrest.Matchers.isIn;
|
||||||
|
|
||||||
public class GeoShapeFieldMapperTests extends ElasticsearchSingleNodeTest {
|
public class GeoShapeFieldMapperTests extends ElasticsearchSingleNodeTest {
|
||||||
|
|
||||||
|
@ -291,4 +296,63 @@ public class GeoShapeFieldMapperTests extends ElasticsearchSingleNodeTest {
|
||||||
assertThat(strategy.getGrid().getMaxLevels(), equalTo(GeoUtils.geoHashLevelsForPrecision(50d)));
|
assertThat(strategy.getGrid().getMaxLevels(), equalTo(GeoUtils.geoHashLevelsForPrecision(50d)));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testGeoShapeMapperMerge() throws Exception {
|
||||||
|
String stage1Mapping = XContentFactory.jsonBuilder().startObject().startObject("type").startObject("properties")
|
||||||
|
.startObject("shape").field("type", "geo_shape").field("tree", "geohash").field("strategy", "recursive")
|
||||||
|
.field("precision", "1m").field("tree_levels", 8).field("distance_error_pct", 0.01).field("orientation", "ccw")
|
||||||
|
.endObject().endObject().endObject().endObject().string();
|
||||||
|
DocumentMapperParser parser = createIndex("test").mapperService().documentMapperParser();
|
||||||
|
DocumentMapper stage1 = parser.parse(stage1Mapping);
|
||||||
|
String stage2Mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
|
||||||
|
.startObject("properties").startObject("shape").field("type", "geo_shape").field("tree", "quadtree")
|
||||||
|
.field("strategy", "term").field("precision", "1km").field("tree_levels", 26).field("distance_error_pct", 26)
|
||||||
|
.field("orientation", "cw").endObject().endObject().endObject().endObject().string();
|
||||||
|
DocumentMapper stage2 = parser.parse(stage2Mapping);
|
||||||
|
|
||||||
|
DocumentMapper.MergeResult mergeResult = stage1.merge(stage2, mergeFlags().simulate(false));
|
||||||
|
// check correct conflicts
|
||||||
|
assertThat(mergeResult.hasConflicts(), equalTo(true));
|
||||||
|
assertThat(mergeResult.conflicts().length, equalTo(3));
|
||||||
|
ArrayList conflicts = new ArrayList<>(Arrays.asList(mergeResult.conflicts()));
|
||||||
|
assertThat("mapper [shape] has different strategy", isIn(conflicts));
|
||||||
|
assertThat("mapper [shape] has different tree", isIn(conflicts));
|
||||||
|
assertThat("mapper [shape] has different tree_levels or precision", isIn(conflicts));
|
||||||
|
|
||||||
|
// verify nothing changed
|
||||||
|
FieldMapper fieldMapper = stage1.mappers().name("shape").mapper();
|
||||||
|
assertThat(fieldMapper, instanceOf(GeoShapeFieldMapper.class));
|
||||||
|
|
||||||
|
GeoShapeFieldMapper geoShapeFieldMapper = (GeoShapeFieldMapper) fieldMapper;
|
||||||
|
PrefixTreeStrategy strategy = geoShapeFieldMapper.defaultStrategy();
|
||||||
|
|
||||||
|
assertThat(strategy, instanceOf(RecursivePrefixTreeStrategy.class));
|
||||||
|
assertThat(strategy.getGrid(), instanceOf(GeohashPrefixTree.class));
|
||||||
|
assertThat(strategy.getDistErrPct(), equalTo(0.01));
|
||||||
|
assertThat(strategy.getGrid().getMaxLevels(), equalTo(GeoUtils.geoHashLevelsForPrecision(1d)));
|
||||||
|
assertThat(geoShapeFieldMapper.orientation(), equalTo(ShapeBuilder.Orientation.CCW));
|
||||||
|
|
||||||
|
// correct mapping
|
||||||
|
stage2Mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
|
||||||
|
.startObject("properties").startObject("shape").field("type", "geo_shape").field("precision", "1m")
|
||||||
|
.field("distance_error_pct", 0.001).field("orientation", "cw").endObject().endObject().endObject().endObject().string();
|
||||||
|
stage2 = parser.parse(stage2Mapping);
|
||||||
|
mergeResult = stage1.merge(stage2, mergeFlags().simulate(false));
|
||||||
|
|
||||||
|
// verify mapping changes, and ensure no failures
|
||||||
|
assertThat(mergeResult.hasConflicts(), equalTo(false));
|
||||||
|
|
||||||
|
fieldMapper = stage1.mappers().name("shape").mapper();
|
||||||
|
assertThat(fieldMapper, instanceOf(GeoShapeFieldMapper.class));
|
||||||
|
|
||||||
|
geoShapeFieldMapper = (GeoShapeFieldMapper) fieldMapper;
|
||||||
|
strategy = geoShapeFieldMapper.defaultStrategy();
|
||||||
|
|
||||||
|
assertThat(strategy, instanceOf(RecursivePrefixTreeStrategy.class));
|
||||||
|
assertThat(strategy.getGrid(), instanceOf(GeohashPrefixTree.class));
|
||||||
|
assertThat(strategy.getDistErrPct(), equalTo(0.001));
|
||||||
|
assertThat(strategy.getGrid().getMaxLevels(), equalTo(GeoUtils.geoHashLevelsForPrecision(1d)));
|
||||||
|
assertThat(geoShapeFieldMapper.orientation(), equalTo(ShapeBuilder.Orientation.CW));
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in New Issue