Browse Source

add points_only option to GeoShapeFieldMapper for optimizing indexing performance on geo_shape indexes designed to store only points. Includes updated documentation and exception handling for ensuring index integrity on points only data.

Nicholas Knize 10 years ago
parent
commit
e4e71d8a9a

+ 10 - 0
core/src/main/java/org/elasticsearch/common/geo/XShapeCollection.java

@@ -36,10 +36,20 @@ import java.util.List;
  */
 public class XShapeCollection<S extends Shape> extends ShapeCollection<S> {
 
+  private boolean pointsOnly = false;
+
   public XShapeCollection(List<S> shapes, SpatialContext ctx) {
     super(shapes, ctx);
   }
 
+  public boolean pointsOnly() {
+    return this.pointsOnly;
+  }
+
+  public void setPointsOnly(boolean pointsOnly) {
+    this.pointsOnly = pointsOnly;
+  }
+
   @Override
   protected Rectangle computeBoundingBox(Collection<? extends Shape> shapes, SpatialContext ctx) {
     Rectangle retBox = shapes.iterator().next().getBoundingBox();

+ 3 - 1
core/src/main/java/org/elasticsearch/common/geo/builders/MultiPointBuilder.java

@@ -51,7 +51,9 @@ public class MultiPointBuilder extends PointCollection<MultiPointBuilder> {
         for (Coordinate coord : points) {
             shapes.add(SPATIAL_CONTEXT.makePoint(coord.x, coord.y));
         }
-        return new XShapeCollection<>(shapes, SPATIAL_CONTEXT);
+        XShapeCollection multiPoints = new XShapeCollection<>(shapes, SPATIAL_CONTEXT);
+        multiPoints.setPointsOnly(true);
+        return multiPoints;
     }
 
     @Override

+ 0 - 1
core/src/main/java/org/elasticsearch/common/geo/builders/ShapeBuilder.java

@@ -27,7 +27,6 @@ import com.vividsolutions.jts.geom.Coordinate;
 import com.vividsolutions.jts.geom.Geometry;
 import com.vividsolutions.jts.geom.GeometryFactory;
 import org.elasticsearch.ElasticsearchParseException;
-import org.elasticsearch.common.collect.Tuple;
 import org.elasticsearch.common.logging.ESLogger;
 import org.elasticsearch.common.logging.ESLoggerFactory;
 import org.elasticsearch.common.unit.DistanceUnit.Distance;

+ 34 - 2
core/src/main/java/org/elasticsearch/index/mapper/geo/GeoShapeFieldMapper.java

@@ -18,7 +18,9 @@
  */
 package org.elasticsearch.index.mapper.geo;
 
+import com.spatial4j.core.shape.Point;
 import com.spatial4j.core.shape.Shape;
+import com.spatial4j.core.shape.jts.JtsGeometry;
 import org.apache.lucene.document.Field;
 import org.apache.lucene.index.IndexOptions;
 import org.apache.lucene.spatial.prefix.PrefixTreeStrategy;
@@ -38,6 +40,7 @@ import org.elasticsearch.common.geo.builders.ShapeBuilder.Orientation;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.unit.DistanceUnit;
 import org.elasticsearch.common.xcontent.XContentBuilder;
+import org.elasticsearch.common.xcontent.support.XContentMapValues;
 import org.elasticsearch.index.mapper.FieldMapper;
 import org.elasticsearch.index.mapper.MappedFieldType;
 import org.elasticsearch.index.mapper.Mapper;
@@ -85,12 +88,14 @@ public class GeoShapeFieldMapper extends FieldMapper {
         public static final String DISTANCE_ERROR_PCT = "distance_error_pct";
         public static final String ORIENTATION = "orientation";
         public static final String STRATEGY = "strategy";
+        public static final String STRATEGY_POINTS_ONLY = "points_only";
         public static final String COERCE = "coerce";
     }
 
     public static class Defaults {
         public static final String TREE = Names.TREE_GEOHASH;
         public static final String STRATEGY = SpatialStrategy.RECURSIVE.getStrategyName();
+        public static final boolean POINTS_ONLY = false;
         public static final int GEOHASH_LEVELS = GeoUtils.geoHashLevelsForPrecision("50m");
         public static final int QUADTREE_LEVELS = GeoUtils.quadTreeLevelsForPrecision("50m");
         public static final double LEGACY_DISTANCE_ERROR_PCT = 0.025d;
@@ -143,7 +148,7 @@ public class GeoShapeFieldMapper extends FieldMapper {
         public GeoShapeFieldMapper build(BuilderContext context) {
             GeoShapeFieldType geoShapeFieldType = (GeoShapeFieldType)fieldType;
 
-            if (geoShapeFieldType.tree.equals("quadtree") && context.indexCreatedVersion().before(Version.V_2_0_0_beta1)) {
+            if (geoShapeFieldType.tree.equals(Names.TREE_QUADTREE) && context.indexCreatedVersion().before(Version.V_2_0_0_beta1)) {
                 geoShapeFieldType.setTree("legacyquadtree");
             }
 
@@ -188,6 +193,9 @@ public class GeoShapeFieldMapper extends FieldMapper {
                 } else if (Names.COERCE.equals(fieldName)) {
                     builder.coerce(nodeBooleanValue(fieldNode));
                     iterator.remove();
+                } else if (Names.STRATEGY_POINTS_ONLY.equals(fieldName)) {
+                    builder.fieldType().setPointsOnly(XContentMapValues.nodeBooleanValue(fieldNode));
+                    iterator.remove();
                 }
             }
             return builder;
@@ -198,6 +206,7 @@ public class GeoShapeFieldMapper extends FieldMapper {
 
         private String tree = Defaults.TREE;
         private String strategyName = Defaults.STRATEGY;
+        private boolean pointsOnly = Defaults.POINTS_ONLY;
         private int treeLevels = 0;
         private double precisionInMeters = -1;
         private Double distanceErrorPct;
@@ -215,6 +224,7 @@ public class GeoShapeFieldMapper extends FieldMapper {
             super(ref);
             this.tree = ref.tree;
             this.strategyName = ref.strategyName;
+            this.pointsOnly = ref.pointsOnly;
             this.treeLevels = ref.treeLevels;
             this.precisionInMeters = ref.precisionInMeters;
             this.distanceErrorPct = ref.distanceErrorPct;
@@ -236,13 +246,15 @@ public class GeoShapeFieldMapper extends FieldMapper {
                 defaultDistanceErrorPct == that.defaultDistanceErrorPct &&
                 Objects.equals(tree, that.tree) &&
                 Objects.equals(strategyName, that.strategyName) &&
+                pointsOnly == that.pointsOnly &&
                 Objects.equals(distanceErrorPct, that.distanceErrorPct) &&
                 orientation == that.orientation;
         }
 
         @Override
         public int hashCode() {
-            return Objects.hash(super.hashCode(), tree, strategyName, treeLevels, precisionInMeters, distanceErrorPct, defaultDistanceErrorPct, orientation);
+            return Objects.hash(super.hashCode(), tree, strategyName, pointsOnly, treeLevels, precisionInMeters, distanceErrorPct,
+                    defaultDistanceErrorPct, orientation);
         }
 
         @Override
@@ -288,6 +300,10 @@ public class GeoShapeFieldMapper extends FieldMapper {
                 conflicts.add("mapper [" + names().fullName() + "] has different [tree]");
             }
 
+            if ((pointsOnly() != other.pointsOnly())) {
+                conflicts.add("mapper [" + names().fullName() + "] has different points_only");
+            }
+
             // 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 (treeLevels() != other.treeLevels()) {
@@ -333,6 +349,14 @@ public class GeoShapeFieldMapper extends FieldMapper {
             this.strategyName = strategyName;
         }
 
+        public boolean pointsOnly() {
+            return pointsOnly;
+        }
+
+        public void setPointsOnly(boolean pointsOnly) {
+            checkIfFrozen();
+            this.pointsOnly = pointsOnly;
+        }
         public int treeLevels() {
             return treeLevels;
         }
@@ -378,6 +402,7 @@ public class GeoShapeFieldMapper extends FieldMapper {
 
         public PrefixTreeStrategy resolveStrategy(String strategyName) {
             if (SpatialStrategy.RECURSIVE.getStrategyName().equals(strategyName)) {
+                recursiveStrategy.setPointsOnly(pointsOnly());
                 return recursiveStrategy;
             }
             if (SpatialStrategy.TERM.getStrategyName().equals(strategyName)) {
@@ -417,6 +442,10 @@ public class GeoShapeFieldMapper extends FieldMapper {
                 }
                 shape = shapeBuilder.build();
             }
+            if (fieldType().defaultStrategy() instanceof RecursivePrefixTreeStrategy && fieldType().pointsOnly() && !(shape instanceof Point)) {
+                throw new MapperParsingException("[{" + fieldType().names().fullName() + "}] is configured for points only but a " +
+                        ((shape instanceof JtsGeometry) ? ((JtsGeometry)shape).getGeom().getGeometryType() : shape.getClass()) + " was found");
+            }
             Field[] fields = fieldType().defaultStrategy().createIndexableFields(shape);
             if (fields == null || fields.length == 0) {
                 return null;
@@ -474,6 +503,9 @@ public class GeoShapeFieldMapper extends FieldMapper {
         if (includeDefaults || fieldType().orientation() != Defaults.ORIENTATION) {
             builder.field(Names.ORIENTATION, fieldType().orientation());
         }
+        if (includeDefaults || fieldType().pointsOnly() != GeoShapeFieldMapper.Defaults.POINTS_ONLY) {
+            builder.field(Names.STRATEGY_POINTS_ONLY, fieldType().pointsOnly());
+        }
         if (includeDefaults || coerce.explicit()) {
             builder.field("coerce", coerce.value());
         }

+ 24 - 1
core/src/test/java/org/elasticsearch/index/mapper/geo/GeoShapeFieldMapperTests.java

@@ -168,6 +168,7 @@ public class GeoShapeFieldMapperTests extends ESSingleNodeTestCase {
                     .field("tree", "quadtree")
                     .field("tree_levels", "6")
                     .field("distance_error_pct", "0.5")
+                    .field("points_only", true)
                 .endObject().endObject()
                 .endObject().endObject().string();
 
@@ -181,6 +182,7 @@ public class GeoShapeFieldMapperTests extends ESSingleNodeTestCase {
         assertThat(strategy.getDistErrPct(), equalTo(0.5));
         assertThat(strategy.getGrid(), instanceOf(QuadPrefixTree.class));
         assertThat(strategy.getGrid().getMaxLevels(), equalTo(6));
+        assertThat(strategy.isPointsOnly(), equalTo(true));
     }
     
     @Test
@@ -308,7 +310,28 @@ public class GeoShapeFieldMapperTests extends ESSingleNodeTestCase {
             assertThat(strategy.getGrid().getMaxLevels(), equalTo(GeoUtils.quadTreeLevelsForPrecision(70d)+1)); 
         }
     }
-    
+
+    @Test
+    public void testPointsOnlyOption() throws IOException {
+        String mapping = XContentFactory.jsonBuilder().startObject().startObject("type1")
+                .startObject("properties").startObject("location")
+                .field("type", "geo_shape")
+                .field("tree", "geohash")
+                .field("points_only", true)
+                .endObject().endObject()
+                .endObject().endObject().string();
+
+        DocumentMapper defaultMapper = createIndex("test").mapperService().documentMapperParser().parse(mapping);
+        FieldMapper fieldMapper = defaultMapper.mappers().getMapper("location");
+        assertThat(fieldMapper, instanceOf(GeoShapeFieldMapper.class));
+
+        GeoShapeFieldMapper geoShapeFieldMapper = (GeoShapeFieldMapper) fieldMapper;
+        PrefixTreeStrategy strategy = geoShapeFieldMapper.fieldType().defaultStrategy();
+
+        assertThat(strategy.getGrid(), instanceOf(GeohashPrefixTree.class));
+        assertThat(strategy.isPointsOnly(), equalTo(true));
+    }
+
     @Test
     public void testLevelDefaults() throws IOException {
         DocumentMapperParser parser = createIndex("test").mapperService().documentMapperParser();

+ 35 - 4
core/src/test/java/org/elasticsearch/search/geo/GeoShapeIntegrationIT.java

@@ -45,15 +45,12 @@ import java.util.Locale;
 import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
 import static org.elasticsearch.index.query.QueryBuilders.filteredQuery;
 import static org.elasticsearch.index.query.QueryBuilders.geoIntersectionQuery;
-import static org.elasticsearch.index.query.QueryBuilders.geoIntersectionQuery;
 import static org.elasticsearch.index.query.QueryBuilders.geoShapeQuery;
 import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse;
-import static org.hamcrest.Matchers.equalTo;
-import static org.hamcrest.Matchers.instanceOf;
-import static org.hamcrest.Matchers.nullValue;
+import static org.hamcrest.Matchers.*;
 
 public class GeoShapeIntegrationIT extends ESIntegTestCase {
 
@@ -479,6 +476,40 @@ public class GeoShapeIntegrationIT extends ESIntegTestCase {
         assertThat(orientation, equalTo(ShapeBuilder.Orientation.CCW));
     }
 
+    @Test
+    public void testPointsOnly() throws Exception {
+        String mapping = XContentFactory.jsonBuilder().startObject().startObject("type1")
+                .startObject("properties").startObject("location")
+                .field("type", "geo_shape")
+                .field("tree", randomBoolean() ? "quadtree" : "geohash")
+                .field("tree_levels", "6")
+                .field("distance_error_pct", "0.01")
+                .field("points_only", true)
+                .endObject().endObject()
+                .endObject().endObject().string();
+
+        assertAcked(prepareCreate("geo_points_only").addMapping("type1", mapping));
+        ensureGreen();
+
+        ShapeBuilder shape = RandomShapeGenerator.createShape(random());
+        try {
+            indexRandom(true, client().prepareIndex("geo_points_only", "type1", "1").setSource(jsonBuilder().startObject()
+                    .field("location", shape).endObject()));
+        } catch (Throwable e) {
+            // RandomShapeGenerator created something other than a POINT type, verify the correct exception is thrown
+            assertThat(e.getMessage(), containsString("MapperParsingException"));
+            assertThat(e.getMessage(), containsString("is configured for points only"));
+            return;
+        }
+
+        // test that point was inserted
+        SearchResponse response = client().prepareSearch()
+                .setQuery(geoIntersectionQuery("location", shape))
+                .execute().actionGet();
+
+        assertEquals(1, response.getHits().getTotalHits());
+    }
+
     private String findNodeName(String index) {
         ClusterState state = client().admin().cluster().prepareState().get().getState();
         IndexShardRoutingTable shard = state.getRoutingTable().index(index).shard(0);

+ 10 - 1
core/src/test/java/org/elasticsearch/test/geo/RandomShapeGenerator.java

@@ -63,6 +63,14 @@ public class RandomShapeGenerator {
         }
     }
 
+    public static ShapeBuilder createShape(Random r) throws InvalidShapeException {
+        return createShapeNear(r, null);
+    }
+
+    public static ShapeBuilder createShape(Random r, ShapeType st) {
+        return createShapeNear(r, null, st);
+    }
+
     public static ShapeBuilder createShapeNear(Random r, Point nearPoint) throws InvalidShapeException {
         return createShape(r, nearPoint, null, null);
     }
@@ -211,7 +219,8 @@ public class RandomShapeGenerator {
                     try {
                         pgb.build();
                     } catch (InvalidShapeException e) {
-                        return null;
+                        // jts bug rarely results in an invalid shape, if it does happen we try again instead of returning null
+                        return createShape(r, nearPoint, within, st, validate);
                     }
                 }
                 return pgb;

+ 10 - 0
docs/reference/mapping/types/geo-shape.asciidoc

@@ -62,6 +62,16 @@ outer ring vertices in counterclockwise order with inner ring(s) vertices (holes
 in clockwise order. Setting this parameter in the geo_shape mapping explicitly
 sets vertex order for the coordinate list of a geo_shape field but can be
 overridden in each individual GeoJSON document.
+
+|`points_only` |Setting this option to `true` (defaults to `false`) configures
+the `geo_shape` field type for point shapes only (NOTE: Multi-Points are not
+yet supported). This optimizes index and search performance for the `geohash` and
+`quadtree` when it is known that only points will be indexed. At present geo_shape
+queries can not be executed on `geo_point` field types. This option bridges the gap
+by improving point performance on a `geo_shape` field so that `geo_shape` queries are
+optimal on a point only field.
+
+
 |=======================================================================
 
 [float]