From a11456fca0cf02c8dcdfada74deba96b6d5cc7e5 Mon Sep 17 00:00:00 2001 From: iverase Date: Thu, 10 Jan 2019 13:39:26 +0100 Subject: [PATCH 1/3] Do not normalize the longitude with value -180 and a test showing the issue --- .../common/geo/builders/PolygonBuilder.java | 2 +- .../search/geo/GeoShapeIntegrationIT.java | 83 +++++++++++++++++++ 2 files changed, 84 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/common/geo/builders/PolygonBuilder.java b/server/src/main/java/org/elasticsearch/common/geo/builders/PolygonBuilder.java index 8f7876d2ba9f2..d336f9e6f5396 100644 --- a/server/src/main/java/org/elasticsearch/common/geo/builders/PolygonBuilder.java +++ b/server/src/main/java/org/elasticsearch/common/geo/builders/PolygonBuilder.java @@ -357,7 +357,7 @@ protected static org.apache.lucene.geo.Polygon polygonLucene(Coordinate[][] poly double[] x = new double[shell.length]; double[] y = new double[shell.length]; for (int i = 0; i < shell.length; ++i) { - x[i] = normalizeLon(shell[i].x); + x[i] = Math.abs(shell[i].x) > 180 ? normalizeLon(shell[i].x) : shell[i].x; y[i] = normalizeLat(shell[i].y); } diff --git a/server/src/test/java/org/elasticsearch/search/geo/GeoShapeIntegrationIT.java b/server/src/test/java/org/elasticsearch/search/geo/GeoShapeIntegrationIT.java index b120b54687607..a7faa04017258 100644 --- a/server/src/test/java/org/elasticsearch/search/geo/GeoShapeIntegrationIT.java +++ b/server/src/test/java/org/elasticsearch/search/geo/GeoShapeIntegrationIT.java @@ -22,6 +22,7 @@ import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.routing.IndexShardRoutingTable; import org.elasticsearch.common.Strings; +import org.elasticsearch.common.geo.builders.PointBuilder; import org.elasticsearch.common.geo.builders.ShapeBuilder; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentType; @@ -158,6 +159,88 @@ public void testIndexShapeRouting() throws Exception { assertThat(searchResponse.getHits().getTotalHits().value, equalTo(1L)); } + public void testIndexPolygonDateLine() throws Exception { + String mappingVector = "{\n" + + " \"properties\": {\n" + + " \"shape\": {\n" + + " \"type\": \"geo_shape\"\n" + + " }\n" + + " }\n" + + " }"; + + String mappingQuad = "{\n" + + " \"properties\": {\n" + + " \"shape\": {\n" + + " \"type\": \"geo_shape\",\n" + + " \"tree\": \"quadtree\"\n" + + " }\n" + + " }\n" + + " }"; + + + // create index + assertAcked(client().admin().indices().prepareCreate("vector").addMapping("doc", mappingVector, XContentType.JSON).get()); + ensureGreen(); + + assertAcked(client().admin().indices().prepareCreate("quad").addMapping("doc", mappingQuad, XContentType.JSON).get()); + ensureGreen(); + + String source = "{\n" + + " \"shape\" : \"POLYGON((179 0, -179 0, -179 2, 179 2, 179 0))\""+ + "}"; + + indexRandom(true, client().prepareIndex("quad", "doc", "0").setSource(source, XContentType.JSON)); + indexRandom(true, client().prepareIndex("vector", "doc", "0").setSource(source, XContentType.JSON)); + + SearchResponse searchResponse = client().prepareSearch("quad").setQuery( + geoShapeQuery("shape", new PointBuilder(-179.75, 1)) + ).get(); + + + assertThat(searchResponse.getHits().getTotalHits().value, equalTo(1L)); + + searchResponse = client().prepareSearch("quad").setQuery( + geoShapeQuery("shape", new PointBuilder(90, 1)) + ).get(); + + assertThat(searchResponse.getHits().getTotalHits().value, equalTo(0L)); + + searchResponse = client().prepareSearch("quad").setQuery( + geoShapeQuery("shape", new PointBuilder(-180, 1)) + ).get(); + + assertThat(searchResponse.getHits().getTotalHits().value, equalTo(1L)); + searchResponse = client().prepareSearch("quad").setQuery( + geoShapeQuery("shape", new PointBuilder(180, 1)) + ).get(); + + assertThat(searchResponse.getHits().getTotalHits().value, equalTo(1L)); + + searchResponse = client().prepareSearch("vector").setQuery( + geoShapeQuery("shape", new PointBuilder(90, 1)) + ).get(); + + assertThat(searchResponse.getHits().getTotalHits().value, equalTo(0L)); + + searchResponse = client().prepareSearch("vector").setQuery( + geoShapeQuery("shape", new PointBuilder(-179.75, 1)) + ).get(); + + assertThat(searchResponse.getHits().getTotalHits().value, equalTo(1L)); + + searchResponse = client().prepareSearch("vector").setQuery( + geoShapeQuery("shape", new PointBuilder(-180, 1)) + ).get(); + + assertThat(searchResponse.getHits().getTotalHits().value, equalTo(1L)); + + searchResponse = client().prepareSearch("vector").setQuery( + geoShapeQuery("shape", new PointBuilder(180, 1)) + ).get(); + + assertThat(searchResponse.getHits().getTotalHits().value, equalTo(1L)); + } + private String findNodeName(String index) { ClusterState state = client().admin().cluster().prepareState().get().getState(); IndexShardRoutingTable shard = state.getRoutingTable().index(index).shard(0); From 7318d8c5c6bc959e92673e3ce0ab3367a2bf7cd1 Mon Sep 17 00:00:00 2001 From: iverase Date: Thu, 10 Jan 2019 14:20:07 +0100 Subject: [PATCH 2/3] Update failing test to take into account this change --- .../org/elasticsearch/common/geo/GeoJsonShapeParserTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/common/geo/GeoJsonShapeParserTests.java b/server/src/test/java/org/elasticsearch/common/geo/GeoJsonShapeParserTests.java index 2acabee8797f4..7cfa4e7d9c5dc 100644 --- a/server/src/test/java/org/elasticsearch/common/geo/GeoJsonShapeParserTests.java +++ b/server/src/test/java/org/elasticsearch/common/geo/GeoJsonShapeParserTests.java @@ -1126,7 +1126,7 @@ public void testParseGeometryCollection() throws IOException { ), new org.apache.lucene.geo.Polygon( new double[] {12.142857142857142d, -12.142857142857142d, -10d, 10d, 12.142857142857142d}, - new double[] {180d, 180d, -177d, -177d, 180d} + new double[] {-180d, -180d, -177d, -177d, -180d} ) }; assertGeometryEquals(luceneExpected, geometryCollectionGeoJson, false); From acbd4870f1651a060fc44134e6b261601f966c4a Mon Sep 17 00:00:00 2001 From: iverase Date: Thu, 10 Jan 2019 16:50:04 +0100 Subject: [PATCH 3/3] add explanatory comments --- .../org/elasticsearch/common/geo/builders/PolygonBuilder.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/server/src/main/java/org/elasticsearch/common/geo/builders/PolygonBuilder.java b/server/src/main/java/org/elasticsearch/common/geo/builders/PolygonBuilder.java index d336f9e6f5396..ac19642949c86 100644 --- a/server/src/main/java/org/elasticsearch/common/geo/builders/PolygonBuilder.java +++ b/server/src/main/java/org/elasticsearch/common/geo/builders/PolygonBuilder.java @@ -342,6 +342,8 @@ protected static org.apache.lucene.geo.Polygon polygonLucene(Coordinate[][] poly holes = new org.apache.lucene.geo.Polygon[polygon.length - 1]; for (int i = 0; i < holes.length; ++i) { Coordinate[] coords = polygon[i+1]; + //We do not have holes on the dateline as they get eliminated + //when breaking the polygon around it. double[] x = new double[coords.length]; double[] y = new double[coords.length]; for (int c = 0; c < coords.length; ++c) { @@ -357,6 +359,8 @@ protected static org.apache.lucene.geo.Polygon polygonLucene(Coordinate[][] poly double[] x = new double[shell.length]; double[] y = new double[shell.length]; for (int i = 0; i < shell.length; ++i) { + //Lucene Tessellator treats different +180 and -180 and we should keep the sign. + //normalizeLon method excludes -180. x[i] = Math.abs(shell[i].x) > 180 ? normalizeLon(shell[i].x) : shell[i].x; y[i] = normalizeLat(shell[i].y); }