From bd2d9350b705c8a02b3eb4cadde90d094824ec7d Mon Sep 17 00:00:00 2001 From: Nick Knize Date: Mon, 21 Mar 2022 22:51:51 -0500 Subject: [PATCH] Support for geo_bounding_box queries on geo_shape fields (#2506) Adds support for using geo_bounding_box queries on geo_shape field types by lifting the geo_point restriction in the QueryBuilder. Bounding Box query integration tests are abstracted to test both geo_point and geo_shape types. Signed-off-by: Nicholas Walter Knize --- ...ava => AbstractGeoBoundingBoxQueryIT.java} | 117 ++++-------------- .../geo/GeoBoundingBoxQueryGeoPointsIT.java | 21 ++++ .../geo/GeoBoundingBoxQueryGeoShapesIT.java | 25 ++++ .../query/GeoBoundingBoxQueryBuilder.java | 50 ++++---- .../GeoBoundingBoxQueryBuilderTests.java | 15 ++- 5 files changed, 108 insertions(+), 120 deletions(-) rename server/src/internalClusterTest/java/org/opensearch/search/geo/{GeoBoundingBoxQueryIT.java => AbstractGeoBoundingBoxQueryIT.java} (73%) create mode 100644 server/src/internalClusterTest/java/org/opensearch/search/geo/GeoBoundingBoxQueryGeoPointsIT.java create mode 100644 server/src/internalClusterTest/java/org/opensearch/search/geo/GeoBoundingBoxQueryGeoShapesIT.java diff --git a/server/src/internalClusterTest/java/org/opensearch/search/geo/GeoBoundingBoxQueryIT.java b/server/src/internalClusterTest/java/org/opensearch/search/geo/AbstractGeoBoundingBoxQueryIT.java similarity index 73% rename from server/src/internalClusterTest/java/org/opensearch/search/geo/GeoBoundingBoxQueryIT.java rename to server/src/internalClusterTest/java/org/opensearch/search/geo/AbstractGeoBoundingBoxQueryIT.java index 90e71633425..3e63987af20 100644 --- a/server/src/internalClusterTest/java/org/opensearch/search/geo/GeoBoundingBoxQueryIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/search/geo/AbstractGeoBoundingBoxQueryIT.java @@ -43,6 +43,8 @@ import org.opensearch.search.SearchHit; import org.opensearch.test.OpenSearchIntegTestCase; import org.opensearch.test.VersionUtils; +import java.io.IOException; + import static org.opensearch.action.support.WriteRequest.RefreshPolicy.IMMEDIATE; import static org.opensearch.common.xcontent.XContentFactory.jsonBuilder; import static org.opensearch.index.query.QueryBuilders.boolQuery; @@ -52,7 +54,15 @@ import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked; import static org.hamcrest.Matchers.anyOf; import static org.hamcrest.Matchers.equalTo; -public class GeoBoundingBoxQueryIT extends OpenSearchIntegTestCase { +abstract class AbstractGeoBoundingBoxQueryIT extends OpenSearchIntegTestCase { + + public abstract XContentBuilder addGeoMapping(XContentBuilder parentMapping) throws IOException; + + public XContentBuilder getMapping() throws IOException { + XContentBuilder mapping = XContentFactory.jsonBuilder().startObject().startObject("properties"); + mapping = addGeoMapping(mapping); + return mapping.endObject().endObject(); + } @Override protected boolean forbidPrivateIndexSettings() { @@ -62,110 +72,55 @@ public class GeoBoundingBoxQueryIT extends OpenSearchIntegTestCase { public void testSimpleBoundingBoxTest() throws Exception { Version version = VersionUtils.randomIndexCompatibleVersion(random()); Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, version).build(); - XContentBuilder xContentBuilder = XContentFactory.jsonBuilder() - .startObject() - .startObject("properties") - .startObject("location") - .field("type", "geo_point"); - xContentBuilder.endObject().endObject().endObject(); + XContentBuilder xContentBuilder = getMapping(); assertAcked(prepareCreate("test").setSettings(settings).setMapping(xContentBuilder)); ensureGreen(); client().prepareIndex("test") .setId("1") - .setSource( - jsonBuilder().startObject() - .field("name", "New York") - .startObject("location") - .field("lat", 40.7143528) - .field("lon", -74.0059731) - .endObject() - .endObject() - ) + .setSource(jsonBuilder().startObject().field("name", "New York").field("location", "POINT(-74.0059731 40.7143528)").endObject()) .get(); // to NY: 5.286 km client().prepareIndex("test") .setId("2") .setSource( - jsonBuilder().startObject() - .field("name", "Times Square") - .startObject("location") - .field("lat", 40.759011) - .field("lon", -73.9844722) - .endObject() - .endObject() + jsonBuilder().startObject().field("name", "Times Square").field("location", "POINT(-73.9844722 40.759011)").endObject() ) .get(); // to NY: 0.4621 km client().prepareIndex("test") .setId("3") - .setSource( - jsonBuilder().startObject() - .field("name", "Tribeca") - .startObject("location") - .field("lat", 40.718266) - .field("lon", -74.007819) - .endObject() - .endObject() - ) + .setSource(jsonBuilder().startObject().field("name", "Tribeca").field("location", "POINT(-74.007819 40.718266)").endObject()) .get(); // to NY: 1.055 km client().prepareIndex("test") .setId("4") .setSource( - jsonBuilder().startObject() - .field("name", "Wall Street") - .startObject("location") - .field("lat", 40.7051157) - .field("lon", -74.0088305) - .endObject() - .endObject() + jsonBuilder().startObject().field("name", "Wall Street").field("location", "POINT(-74.0088305 40.7051157)").endObject() ) .get(); // to NY: 1.258 km client().prepareIndex("test") .setId("5") - .setSource( - jsonBuilder().startObject() - .field("name", "Soho") - .startObject("location") - .field("lat", 40.7247222) - .field("lon", -74) - .endObject() - .endObject() - ) + .setSource(jsonBuilder().startObject().field("name", "Soho").field("location", "POINT(-74 40.7247222)").endObject()) .get(); // to NY: 2.029 km client().prepareIndex("test") .setId("6") .setSource( - jsonBuilder().startObject() - .field("name", "Greenwich Village") - .startObject("location") - .field("lat", 40.731033) - .field("lon", -73.9962255) - .endObject() - .endObject() + jsonBuilder().startObject().field("name", "Greenwich Village").field("location", "POINT(-73.9962255 40.731033)").endObject() ) .get(); // to NY: 8.572 km client().prepareIndex("test") .setId("7") - .setSource( - jsonBuilder().startObject() - .field("name", "Brooklyn") - .startObject("location") - .field("lat", 40.65) - .field("lon", -73.95) - .endObject() - .endObject() - ) + .setSource(jsonBuilder().startObject().field("name", "Brooklyn").field("location", "POINT(-73.95 40.65)").endObject()) .get(); client().admin().indices().prepareRefresh().get(); @@ -192,12 +147,7 @@ public class GeoBoundingBoxQueryIT extends OpenSearchIntegTestCase { public void testLimit2BoundingBox() throws Exception { Version version = VersionUtils.randomIndexCompatibleVersion(random()); Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, version).build(); - XContentBuilder xContentBuilder = XContentFactory.jsonBuilder() - .startObject() - .startObject("properties") - .startObject("location") - .field("type", "geo_point"); - xContentBuilder.endObject().endObject().endObject(); + XContentBuilder xContentBuilder = getMapping(); assertAcked(prepareCreate("test").setSettings(settings).setMapping(xContentBuilder)); ensureGreen(); @@ -207,10 +157,7 @@ public class GeoBoundingBoxQueryIT extends OpenSearchIntegTestCase { jsonBuilder().startObject() .field("userid", 880) .field("title", "Place in Stockholm") - .startObject("location") - .field("lat", 59.328355000000002) - .field("lon", 18.036842) - .endObject() + .field("location", "POINT(59.328355000000002 18.036842)") .endObject() ) .setRefreshPolicy(IMMEDIATE) @@ -222,10 +169,7 @@ public class GeoBoundingBoxQueryIT extends OpenSearchIntegTestCase { jsonBuilder().startObject() .field("userid", 534) .field("title", "Place in Montreal") - .startObject("location") - .field("lat", 45.509526999999999) - .field("lon", -73.570986000000005) - .endObject() + .field("location", "POINT(-73.570986000000005 45.509526999999999)") .endObject() ) .setRefreshPolicy(IMMEDIATE) @@ -271,12 +215,7 @@ public class GeoBoundingBoxQueryIT extends OpenSearchIntegTestCase { public void testCompleteLonRange() throws Exception { Version version = VersionUtils.randomIndexCompatibleVersion(random()); Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, version).build(); - XContentBuilder xContentBuilder = XContentFactory.jsonBuilder() - .startObject() - .startObject("properties") - .startObject("location") - .field("type", "geo_point"); - xContentBuilder.endObject().endObject().endObject(); + XContentBuilder xContentBuilder = getMapping(); assertAcked(prepareCreate("test").setSettings(settings).setMapping(xContentBuilder)); ensureGreen(); @@ -286,10 +225,7 @@ public class GeoBoundingBoxQueryIT extends OpenSearchIntegTestCase { jsonBuilder().startObject() .field("userid", 880) .field("title", "Place in Stockholm") - .startObject("location") - .field("lat", 59.328355000000002) - .field("lon", 18.036842) - .endObject() + .field("location", "POINT(18.036842 59.328355000000002)") .endObject() ) .setRefreshPolicy(IMMEDIATE) @@ -301,10 +237,7 @@ public class GeoBoundingBoxQueryIT extends OpenSearchIntegTestCase { jsonBuilder().startObject() .field("userid", 534) .field("title", "Place in Montreal") - .startObject("location") - .field("lat", 45.509526999999999) - .field("lon", -73.570986000000005) - .endObject() + .field("location", "POINT(-73.570986000000005 45.509526999999999)") .endObject() ) .setRefreshPolicy(IMMEDIATE) diff --git a/server/src/internalClusterTest/java/org/opensearch/search/geo/GeoBoundingBoxQueryGeoPointsIT.java b/server/src/internalClusterTest/java/org/opensearch/search/geo/GeoBoundingBoxQueryGeoPointsIT.java new file mode 100644 index 00000000000..d039e1200a8 --- /dev/null +++ b/server/src/internalClusterTest/java/org/opensearch/search/geo/GeoBoundingBoxQueryGeoPointsIT.java @@ -0,0 +1,21 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.search.geo; + +import org.opensearch.common.xcontent.XContentBuilder; + +import java.io.IOException; + +public class GeoBoundingBoxQueryGeoPointsIT extends AbstractGeoBoundingBoxQueryIT { + + @Override + public XContentBuilder addGeoMapping(XContentBuilder parentMapping) throws IOException { + return parentMapping.startObject("location").field("type", "geo_point").endObject(); + } +} diff --git a/server/src/internalClusterTest/java/org/opensearch/search/geo/GeoBoundingBoxQueryGeoShapesIT.java b/server/src/internalClusterTest/java/org/opensearch/search/geo/GeoBoundingBoxQueryGeoShapesIT.java new file mode 100644 index 00000000000..44f9dd6d011 --- /dev/null +++ b/server/src/internalClusterTest/java/org/opensearch/search/geo/GeoBoundingBoxQueryGeoShapesIT.java @@ -0,0 +1,25 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.search.geo; + +import org.opensearch.common.xcontent.XContentBuilder; + +import java.io.IOException; + +public class GeoBoundingBoxQueryGeoShapesIT extends AbstractGeoBoundingBoxQueryIT { + + @Override + public XContentBuilder addGeoMapping(XContentBuilder parentMapping) throws IOException { + parentMapping = parentMapping.startObject("location").field("type", "geo_shape"); + if (randomBoolean()) { + parentMapping.field("strategy", "recursive"); + } + return parentMapping.endObject(); + } +} diff --git a/server/src/main/java/org/opensearch/index/query/GeoBoundingBoxQueryBuilder.java b/server/src/main/java/org/opensearch/index/query/GeoBoundingBoxQueryBuilder.java index 82e95c74507..879ad07ca81 100644 --- a/server/src/main/java/org/opensearch/index/query/GeoBoundingBoxQueryBuilder.java +++ b/server/src/main/java/org/opensearch/index/query/GeoBoundingBoxQueryBuilder.java @@ -32,9 +32,6 @@ package org.opensearch.index.query; -import org.apache.lucene.document.LatLonDocValuesField; -import org.apache.lucene.document.LatLonPoint; -import org.apache.lucene.search.IndexOrDocValuesQuery; import org.apache.lucene.search.MatchNoDocsQuery; import org.apache.lucene.search.Query; import org.opensearch.OpenSearchParseException; @@ -44,13 +41,17 @@ import org.opensearch.common.ParsingException; import org.opensearch.common.geo.GeoBoundingBox; import org.opensearch.common.geo.GeoPoint; import org.opensearch.common.geo.GeoUtils; +import org.opensearch.common.geo.ShapeRelation; +import org.opensearch.common.geo.SpatialStrategy; import org.opensearch.common.io.stream.StreamInput; import org.opensearch.common.io.stream.StreamOutput; import org.opensearch.common.xcontent.XContentBuilder; import org.opensearch.common.xcontent.XContentParser; import org.opensearch.geometry.Rectangle; import org.opensearch.geometry.utils.Geohash; -import org.opensearch.index.mapper.GeoPointFieldMapper.GeoPointFieldType; +import org.opensearch.index.mapper.GeoPointFieldMapper; +import org.opensearch.index.mapper.GeoShapeFieldMapper; +import org.opensearch.index.mapper.GeoShapeQueryable; import org.opensearch.index.mapper.MappedFieldType; import java.io.IOException; @@ -315,11 +316,24 @@ public class GeoBoundingBoxQueryBuilder extends AbstractQueryBuilder qb.toQuery(context)); - assertEquals("failed to find geo_point field [" + qb.fieldName() + "]", e.getMessage()); + assertEquals("failed to find geo field [" + qb.fieldName() + "]", e.getMessage()); } public void testBrokenCoordinateCannotBeSet() { @@ -223,10 +225,11 @@ public class GeoBoundingBoxQueryBuilderTests extends AbstractQueryTestCase failingQueryBuilder.toQuery(shardContext)); - assertThat(e.getMessage(), containsString("failed to find geo_point field [unmapped]")); + assertThat(e.getMessage(), containsString("failed to find geo field [unmapped]")); } }