diff --git a/docs/reference/mapping/types/geo-shape-type.asciidoc b/docs/reference/mapping/types/geo-shape-type.asciidoc index 4f342c62763..914bbbd3b13 100644 --- a/docs/reference/mapping/types/geo-shape-type.asciidoc +++ b/docs/reference/mapping/types/geo-shape-type.asciidoc @@ -46,7 +46,13 @@ via the mapping API even if you use the precision parameter. |`distance_error_pct` |Used as a hint to the PrefixTree about how precise it should be. Defaults to 0.025 (2.5%) with 0.5 as the maximum -supported value. +supported value. PERFORMANCE NOTE: This value will be default to 0 if a `precision` or +`tree_level` definition is explicitly defined. This guarantees spatial precision +at the level defined in the mapping. This can lead to significant memory usage +for high resolution shapes with low error (e.g., large shapes at 1m with < 0.001 error). +To improve indexing performance (at the cost of query accuracy) explicitly define +`tree_level` or `precision` along with a reasonable `distance_error_pct`, noting +that large shapes will have greater false positives. |`orientation` |Optionally define how to interpret vertex order for polygons / multipolygons. This parameter defines one of two coordinate diff --git a/src/main/java/org/elasticsearch/index/mapper/geo/GeoShapeFieldMapper.java b/src/main/java/org/elasticsearch/index/mapper/geo/GeoShapeFieldMapper.java index 5aba9ed54ad..979346767db 100644 --- a/src/main/java/org/elasticsearch/index/mapper/geo/GeoShapeFieldMapper.java +++ b/src/main/java/org/elasticsearch/index/mapper/geo/GeoShapeFieldMapper.java @@ -114,6 +114,7 @@ public class GeoShapeFieldMapper extends AbstractFieldMapper { private int treeLevels = 0; private double precisionInMeters = -1; private double distanceErrorPct = Defaults.DISTANCE_ERROR_PCT; + private boolean distErrPctDefined; private Orientation orientation = Defaults.ORIENTATION; private SpatialPrefixTree prefixTree; @@ -173,23 +174,27 @@ public class GeoShapeFieldMapper extends AbstractFieldMapper { return new GeoShapeFieldMapper(names, prefixTree, strategyName, distanceErrorPct, orientation, fieldType, context.indexSettings(), multiFieldsBuilder.build(this, context), copyTo); } - } - private static final int getLevels(int treeLevels, double precisionInMeters, int defaultLevels, boolean geoHash) { - if (treeLevels > 0 || precisionInMeters >= 0) { - return Math.max(treeLevels, precisionInMeters >= 0 ? (geoHash ? GeoUtils.geoHashLevelsForPrecision(precisionInMeters) - : GeoUtils.quadTreeLevelsForPrecision(precisionInMeters)) : 0); + private final int getLevels(int treeLevels, double precisionInMeters, int defaultLevels, boolean geoHash) { + if (treeLevels > 0 || precisionInMeters >= 0) { + // if the user specified a precision but not a distance error percent then zero out the distance err pct + // this is done to guarantee precision specified by the user without doing something unexpected under the covers + if (!distErrPctDefined) distanceErrorPct = 0; + return Math.max(treeLevels, precisionInMeters >= 0 ? (geoHash ? GeoUtils.geoHashLevelsForPrecision(precisionInMeters) + : GeoUtils.quadTreeLevelsForPrecision(precisionInMeters)) : 0); + } + return defaultLevels; } - return defaultLevels; } - public static class TypeParser implements Mapper.TypeParser { @Override public Mapper.Builder parse(String name, Map node, ParserContext parserContext) throws MapperParsingException { Builder builder = geoShapeField(name); - + // if index was created before 1.6, this conditional should be true (this forces any index created on/or after 1.6 to use 0 for + // the default distanceErrorPct parameter). + builder.distErrPctDefined = parserContext.indexVersionCreated().before(Version.V_1_6_0); for (Iterator> iterator = node.entrySet().iterator(); iterator.hasNext();) { Map.Entry entry = iterator.next(); String fieldName = Strings.toUnderscoreCase(entry.getKey()); @@ -205,6 +210,7 @@ public class GeoShapeFieldMapper extends AbstractFieldMapper { iterator.remove(); } else if (Names.DISTANCE_ERROR_PCT.equals(fieldName)) { builder.distanceErrorPct(Double.parseDouble(fieldNode.toString())); + builder.distErrPctDefined = true; iterator.remove(); } else if (Names.ORIENTATION.equals(fieldName)) { builder.orientation(ShapeBuilder.orientationFromString(fieldNode.toString())); @@ -282,40 +288,38 @@ public class GeoShapeFieldMapper extends AbstractFieldMapper { return; } final GeoShapeFieldMapper fieldMergeWith = (GeoShapeFieldMapper) mergeWith; - if (!mergeContext.mergeFlags().simulate()) { - final PrefixTreeStrategy mergeWithStrategy = fieldMergeWith.defaultStrategy; + 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; + // 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() || mergeContext.mergeFlags().simulate()) { + 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 diff --git a/src/test/java/org/elasticsearch/index/mapper/geo/GeoShapeFieldMapperTests.java b/src/test/java/org/elasticsearch/index/mapper/geo/GeoShapeFieldMapperTests.java index 8bb837906ad..3161fcb1b3a 100644 --- a/src/test/java/org/elasticsearch/index/mapper/geo/GeoShapeFieldMapperTests.java +++ b/src/test/java/org/elasticsearch/index/mapper/geo/GeoShapeFieldMapperTests.java @@ -173,9 +173,35 @@ public class GeoShapeFieldMapperTests extends ElasticsearchSingleNodeTest { assertThat(strategy.getDistErrPct(), equalTo(0.5)); assertThat(strategy.getGrid(), instanceOf(QuadPrefixTree.class)); - /* 70m is more precise so it wins */ + // 70m is more precise so it wins assertThat(strategy.getGrid().getMaxLevels(), equalTo(GeoUtils.quadTreeLevelsForPrecision(70d))); } + + { + String mapping = XContentFactory.jsonBuilder().startObject().startObject("type1") + .startObject("properties").startObject("location") + .field("type", "geo_shape") + .field("tree", "quadtree") + .field("tree_levels", "26") + .field("precision", "70m") + .endObject().endObject() + .endObject().endObject().string(); + + + DocumentMapper defaultMapper = parser.parse(mapping); + FieldMapper fieldMapper = defaultMapper.mappers().name("location").mapper(); + assertThat(fieldMapper, instanceOf(GeoShapeFieldMapper.class)); + + GeoShapeFieldMapper geoShapeFieldMapper = (GeoShapeFieldMapper) fieldMapper; + PrefixTreeStrategy strategy = geoShapeFieldMapper.defaultStrategy(); + + // distance_error_pct was not specified so we expect the mapper to take the highest precision between "precision" and + // "tree_levels" setting distErrPct to 0 to guarantee desired precision + assertThat(strategy.getDistErrPct(), equalTo(0.0)); + assertThat(strategy.getGrid(), instanceOf(QuadPrefixTree.class)); + // 70m is less precise so it loses + assertThat(strategy.getGrid().getMaxLevels(), equalTo(26)); + } { String mapping = XContentFactory.jsonBuilder().startObject().startObject("type1") @@ -197,7 +223,7 @@ public class GeoShapeFieldMapperTests extends ElasticsearchSingleNodeTest { assertThat(strategy.getDistErrPct(), equalTo(0.5)); assertThat(strategy.getGrid(), instanceOf(GeohashPrefixTree.class)); - /* 70m is more precise so it wins */ + // 70m is more precise so it wins assertThat(strategy.getGrid().getMaxLevels(), equalTo(GeoUtils.geoHashLevelsForPrecision(70d))); }