Skip to content

Commit

Permalink
[GEO] Add merge conflicts to GeoShapeFieldMapper
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
nknize committed Apr 10, 2015
1 parent a8a35d7 commit 7548562
Show file tree
Hide file tree
Showing 2 changed files with 110 additions and 0 deletions.
Expand Up @@ -41,6 +41,8 @@
import org.elasticsearch.index.mapper.FieldMapper;
import org.elasticsearch.index.mapper.Mapper;
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.core.AbstractFieldMapper;

Expand Down Expand Up @@ -262,6 +264,50 @@ public void parse(ParseContext context) throws IOException {
}
}

@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
protected void parseCreateField(ParseContext context, List<Field> fields) throws IOException {
}
Expand Down
Expand Up @@ -19,6 +19,7 @@
package org.elasticsearch.index.mapper.geo;

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.QuadPrefixTree;
import org.elasticsearch.common.geo.GeoUtils;
Expand All @@ -31,9 +32,13 @@
import org.junit.Test;

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.instanceOf;
import static org.hamcrest.Matchers.isIn;

public class GeoShapeFieldMapperTests extends ElasticsearchSingleNodeTest {

Expand Down Expand Up @@ -291,4 +296,63 @@ public void testLevelDefaults() throws IOException {
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));
}
}

0 comments on commit 7548562

Please sign in to comment.