Browse Source

Add support for multipoint geoshape queries (#52133)

Currently multi-point queries are not supported when indexing your data using BKD-backed geoshape strategy. This commit removes this limitation.
Ignacio Vera 5 years ago
parent
commit
e2b410e15e

+ 3 - 9
docs/reference/mapping/types/geo-shape.asciidoc

@@ -134,16 +134,10 @@ and will be removed in a future version.
 
 *IMPORTANT NOTES*
 
-The following features are not yet supported with the new indexing approach:
+`CONTAINS` relation query - when using the new default vector indexing strategy, `geo_shape`
+queries with `relation` defined as `contains` are supported for indices created with
+ElasticSearch 7.5.0 or higher.
 
-* `geo_shape` query with `MultiPoint` geometry types - Elasticsearch currently prevents searching
-   geo_shape fields with a MultiPoint geometry type to avoid a brute force linear search
-   over each individual point. For now, if this is absolutely needed, this can be achieved
-   using a `bool` query with each individual point.
-
-* `CONTAINS` relation query - when using the new default vector indexing strategy, `geo_shape`
-   queries with `relation` defined as `contains` are supported for indices created with
-   ElasticSearch 7.5.0 or higher.
 
 [[prefix-trees]]
 [float]

+ 8 - 12
server/src/main/java/org/elasticsearch/index/query/VectorGeoShapeQueryProcessor.java

@@ -28,7 +28,6 @@ import org.apache.lucene.search.BooleanQuery;
 import org.apache.lucene.search.MatchNoDocsQuery;
 import org.apache.lucene.search.Query;
 import org.elasticsearch.Version;
-import org.elasticsearch.common.geo.GeoShapeType;
 import org.elasticsearch.common.geo.ShapeRelation;
 import org.elasticsearch.geometry.Circle;
 import org.elasticsearch.geometry.Geometry;
@@ -106,13 +105,7 @@ public class VectorGeoShapeQueryProcessor implements AbstractGeometryFieldMapper
                 occur = BooleanClause.Occur.SHOULD;
             }
             for (Geometry shape : collection) {
-                if (shape instanceof MultiPoint) {
-                    // Flatten multi-points
-                    // We do not support multi-point queries?
-                    visit(bqb, (GeometryCollection<?>) shape);
-                } else {
-                    bqb.add(shape.visit(this), occur);
-                }
+                bqb.add(shape.visit(this), occur);
             }
         }
 
@@ -139,8 +132,11 @@ public class VectorGeoShapeQueryProcessor implements AbstractGeometryFieldMapper
 
         @Override
         public Query visit(MultiPoint multiPoint) {
-            throw new QueryShardException(context, "Field [" + fieldName + "] does not support " + GeoShapeType.MULTIPOINT +
-                " queries");
+            double[][] points = new double[multiPoint.size()][2];
+            for (int i = 0; i < multiPoint.size(); i++) {
+                points[i] = new double[] {multiPoint.get(i).getLat(), multiPoint.get(i).getLon()};
+            }
+            return LatLonShape.newPointQuery(fieldName, relation.getLuceneRelation(), points);
         }
 
         @Override
@@ -161,8 +157,8 @@ public class VectorGeoShapeQueryProcessor implements AbstractGeometryFieldMapper
                 // intersects is more efficient.
                 luceneRelation = ShapeField.QueryRelation.INTERSECTS;
             }
-            return LatLonShape.newBoxQuery(fieldName, luceneRelation,
-                point.getY(), point.getY(), point.getX(), point.getX());
+            return LatLonShape.newPointQuery(fieldName, luceneRelation,
+                new double[] {point.getY(), point.getX()});
         }
 
         @Override

+ 14 - 6
server/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderTests.java

@@ -82,9 +82,8 @@ public class GeoShapeQueryBuilderTests extends AbstractQueryTestCase<GeoShapeQue
     }
 
     protected GeoShapeQueryBuilder doCreateTestQueryBuilder(boolean indexedShape) {
-        // LatLonShape does not support MultiPoint queries
         RandomShapeGenerator.ShapeType shapeType =
-            randomFrom(ShapeType.POINT, ShapeType.LINESTRING, ShapeType.MULTILINESTRING, ShapeType.POLYGON);
+            randomFrom(ShapeType.POINT, ShapeType.MULTIPOINT, ShapeType.LINESTRING, ShapeType.MULTILINESTRING, ShapeType.POLYGON);
         ShapeBuilder<?, ?, ?> shape = RandomShapeGenerator.createShapeWithin(random(), null, shapeType);
         GeoShapeQueryBuilder builder;
         clearShapeFields();
@@ -108,11 +107,20 @@ public class GeoShapeQueryBuilderTests extends AbstractQueryTestCase<GeoShapeQue
             }
         }
         if (randomBoolean()) {
-            if (shapeType == ShapeType.LINESTRING || shapeType == ShapeType.MULTILINESTRING) {
-                builder.relation(randomFrom(ShapeRelation.DISJOINT, ShapeRelation.INTERSECTS));
+            QueryShardContext context = createShardContext();
+            if (context.indexVersionCreated().onOrAfter(Version.V_7_5_0)) { // CONTAINS is only supported from version 7.5
+                if (shapeType == ShapeType.LINESTRING || shapeType == ShapeType.MULTILINESTRING) {
+                    builder.relation(randomFrom(ShapeRelation.DISJOINT, ShapeRelation.INTERSECTS, ShapeRelation.CONTAINS));
+                } else {
+                    builder.relation(randomFrom(ShapeRelation.DISJOINT, ShapeRelation.INTERSECTS,
+                        ShapeRelation.WITHIN, ShapeRelation.CONTAINS));
+                }
             } else {
-                // LatLonShape does not support CONTAINS:
-                builder.relation(randomFrom(ShapeRelation.DISJOINT, ShapeRelation.INTERSECTS, ShapeRelation.WITHIN));
+                if (shapeType == ShapeType.LINESTRING || shapeType == ShapeType.MULTILINESTRING) {
+                    builder.relation(randomFrom(ShapeRelation.DISJOINT, ShapeRelation.INTERSECTS));
+                } else {
+                    builder.relation(randomFrom(ShapeRelation.DISJOINT, ShapeRelation.INTERSECTS, ShapeRelation.WITHIN));
+                }
             }
         }
 

+ 10 - 32
server/src/test/java/org/elasticsearch/search/geo/GeoShapeQueryTests.java

@@ -196,23 +196,9 @@ public class GeoShapeQueryTests extends GeoQueryTests {
     }
 
     public void testRandomGeoCollectionQuery() throws Exception {
-        boolean usePrefixTrees = randomBoolean();
         // Create a random geometry collection to index.
-        GeometryCollectionBuilder gcb;
-        if (usePrefixTrees) {
-            gcb = RandomShapeGenerator.createGeometryCollection(random());
-        } else {
-            // vector strategy does not yet support multipoint queries
-            gcb = new GeometryCollectionBuilder();
-            int numShapes = RandomNumbers.randomIntBetween(random(), 1, 4);
-            for (int i = 0; i < numShapes; ++i) {
-                ShapeBuilder shape;
-                do {
-                    shape = RandomShapeGenerator.createShape(random());
-                } while (shape instanceof MultiPointBuilder);
-                gcb.shape(shape);
-            }
-        }
+        GeometryCollectionBuilder gcb = RandomShapeGenerator.createGeometryCollection(random());;
+
         org.apache.lucene.geo.Polygon randomPoly = GeoTestUtil.nextPolygon();
 
         assumeTrue("Skipping the check for the polygon with a degenerated dimension",
@@ -224,10 +210,9 @@ public class GeoShapeQueryTests extends GeoQueryTests {
         }
         gcb.shape(new PolygonBuilder(cb));
 
-        logger.info("Created Random GeometryCollection containing {} shapes using {} tree", gcb.numShapes(),
-            usePrefixTrees ? "geohash" : "quadtree");
+        logger.info("Created Random GeometryCollection containing {} shapes", gcb.numShapes());
 
-        XContentBuilder mapping = createPrefixTreeMapping(usePrefixTrees ? "geohash" : "quadtree");
+        XContentBuilder mapping = createRandomMapping();
         Settings settings = Settings.builder().put("index.number_of_shards", 1).build();
         client().admin().indices().prepareCreate("test").setMapping(mapping).setSettings(settings).get();
         ensureGreen();
@@ -457,11 +442,8 @@ public class GeoShapeQueryTests extends GeoQueryTests {
         PointBuilder pb = new PointBuilder(pt[0], pt[1]);
         gcb.shape(pb);
 
-        // don't use random as permits quadtree
-        String mapping = Strings.toString(
-            randomBoolean() ?
-                createDefaultMapping() :
-                createPrefixTreeMapping(LegacyGeoShapeFieldMapper.DeprecatedParameters.PrefixTrees.QUADTREE));
+        // create mapping
+        String mapping = Strings.toString(createRandomMapping());
         client().admin().indices().prepareCreate("test").setMapping(mapping).get();
         ensureGreen();
 
@@ -527,10 +509,8 @@ public class GeoShapeQueryTests extends GeoQueryTests {
         GeometryCollectionBuilder gcb = RandomShapeGenerator.createGeometryCollection(random());
         logger.info("Created Random GeometryCollection containing {} shapes", gcb.numShapes());
 
-        String mapping = Strings.toString(
-            randomBoolean() ?
-                createDefaultMapping() :
-                createPrefixTreeMapping(LegacyGeoShapeFieldMapper.DeprecatedParameters.PrefixTrees.QUADTREE));
+        String mapping = Strings.toString(createRandomMapping());
+
         client().admin().indices().prepareCreate("test").setMapping(mapping).get();
         ensureGreen();
 
@@ -677,8 +657,7 @@ public class GeoShapeQueryTests extends GeoQueryTests {
 
     public void testQueryRandomGeoCollection() throws Exception {
         // Create a random geometry collection.
-        String mapping = Strings.toString(randomBoolean() ? createDefaultMapping() : createPrefixTreeMapping(
-            LegacyGeoShapeFieldMapper.DeprecatedParameters.PrefixTrees.QUADTREE));
+        String mapping = Strings.toString(createRandomMapping());
         GeometryCollectionBuilder gcb = RandomShapeGenerator.createGeometryCollection(random());
         org.apache.lucene.geo.Polygon randomPoly = GeoTestUtil.nextPolygon();
         CoordinatesBuilder cb = new CoordinatesBuilder();
@@ -708,8 +687,7 @@ public class GeoShapeQueryTests extends GeoQueryTests {
     }
 
     public void testShapeFilterWithDefinedGeoCollection() throws Exception {
-        String mapping = Strings.toString(
-            createPrefixTreeMapping(LegacyGeoShapeFieldMapper.DeprecatedParameters.PrefixTrees.QUADTREE));
+        String mapping = Strings.toString(createRandomMapping());
         client().admin().indices().prepareCreate("test").setMapping(mapping).get();
         ensureGreen();