Skip to content

Commit

Permalink
Geo: Don't flip longitude of envelopes crossing dateline (#34535)
Browse files Browse the repository at this point in the history
When a envelope that crosses the dateline is specified as a part of
geo_shape query is parsed it shouldn't have its left and right points
flipped.

Fixes #34418
  • Loading branch information
imotov committed Oct 19, 2018
1 parent fba5d39 commit 94bde37
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 8 deletions.
2 changes: 1 addition & 1 deletion docs/reference/mapping/types/geo-shape.asciidoc
Expand Up @@ -567,7 +567,7 @@ POST /example/doc

Elasticsearch supports an `envelope` type, which consists of coordinates
for upper left and lower right points of the shape to represent a
bounding rectangle:
bounding rectangle in the format [[minLon, maxLat],[maxLon, minLat]]:

[source,js]
--------------------------------------------------
Expand Down
4 changes: 4 additions & 0 deletions docs/reference/migration/migrate_7_0/search.asciidoc
Expand Up @@ -20,6 +20,10 @@
* Attempts to generate multi-term phrase queries against non-text fields
with a custom analyzer will now throw an exception

* An `envelope` crossing the dateline in a `geo_shape `query is now processed
correctly when specified using REST API instead of having its left and
right corners flipped.

[float]
==== Adaptive replica selection enabled by default

Expand Down
Expand Up @@ -223,11 +223,6 @@ public EnvelopeBuilder getBuilder(CoordinateNode coordinates, DistanceUnit.Dista
// verify coordinate bounds, correct if necessary
Coordinate uL = coordinates.children.get(0).coordinate;
Coordinate lR = coordinates.children.get(1).coordinate;
if (((lR.x < uL.x) || (uL.y < lR.y))) {
Coordinate uLtmp = uL;
uL = new Coordinate(Math.min(uL.x, lR.x), Math.max(uL.y, lR.y));
lR = new Coordinate(Math.max(uLtmp.x, lR.x), Math.min(uLtmp.y, lR.y));
}
return new EnvelopeBuilder(uL, lR);
}

Expand Down
Expand Up @@ -175,15 +175,15 @@ public void testParseEnvelope() throws IOException {
Rectangle expected = SPATIAL_CONTEXT.makeRectangle(-50, 50, -30, 30);
assertGeometryEquals(expected, multilinesGeoJson);

// test #2: envelope with agnostic coordinate order (TopRight, BottomLeft)
// test #2: envelope that spans dateline
multilinesGeoJson = XContentFactory.jsonBuilder().startObject().field("type", "envelope")
.startArray("coordinates")
.startArray().value(50).value(30).endArray()
.startArray().value(-50).value(-30).endArray()
.endArray()
.endObject();

expected = SPATIAL_CONTEXT.makeRectangle(-50, 50, -30, 30);
expected = SPATIAL_CONTEXT.makeRectangle(50, -50, -30, 30);
assertGeometryEquals(expected, multilinesGeoJson);

// test #3: "envelope" (actually a triangle) with invalid number of coordinates (TopRight, BottomLeft, BottomRight)
Expand Down
Expand Up @@ -20,7 +20,9 @@
package org.elasticsearch.search.geo;

import org.elasticsearch.action.get.GetResponse;
import org.elasticsearch.action.index.IndexRequest;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.common.CheckedSupplier;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.geo.ShapeRelation;
import org.elasticsearch.common.geo.builders.CoordinatesBuilder;
Expand All @@ -32,6 +34,7 @@
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.index.mapper.MapperParsingException;
import org.elasticsearch.index.query.GeoShapeQueryBuilder;
Expand Down Expand Up @@ -531,4 +534,73 @@ public void testFieldAlias() throws IOException {
.execute().actionGet();
assertEquals(1, response.getHits().getTotalHits());
}

// Test for issue #34418
public void testEnvelopeSpanningDateline() throws IOException {
XContentBuilder mapping = XContentFactory.jsonBuilder().startObject()
.startObject("doc")
.startObject("properties")
.startObject("geo").field("type", "geo_shape").endObject()
.endObject()
.endObject()
.endObject();

createIndex("test", Settings.builder().put("index.number_of_shards", 1).build(), "doc", mapping);

String doc1 = "{\"geo\": {\r\n" + "\"coordinates\": [\r\n" + "-33.918711,\r\n" + "18.847685\r\n" + "],\r\n" +
"\"type\": \"Point\"\r\n" + "}}";
client().index(new IndexRequest("test", "doc", "1").source(doc1, XContentType.JSON).setRefreshPolicy(IMMEDIATE)).actionGet();

String doc2 = "{\"geo\": {\r\n" + "\"coordinates\": [\r\n" + "-49.0,\r\n" + "18.847685\r\n" + "],\r\n" +
"\"type\": \"Point\"\r\n" + "}}";
client().index(new IndexRequest("test", "doc", "2").source(doc2, XContentType.JSON).setRefreshPolicy(IMMEDIATE)).actionGet();

String doc3 = "{\"geo\": {\r\n" + "\"coordinates\": [\r\n" + "49.0,\r\n" + "18.847685\r\n" + "],\r\n" +
"\"type\": \"Point\"\r\n" + "}}";
client().index(new IndexRequest("test", "doc", "3").source(doc3, XContentType.JSON).setRefreshPolicy(IMMEDIATE)).actionGet();

@SuppressWarnings("unchecked") CheckedSupplier<GeoShapeQueryBuilder, IOException> querySupplier = randomFrom(
() -> QueryBuilders.geoShapeQuery(
"geo",
new EnvelopeBuilder(new Coordinate(-21, 44), new Coordinate(-39, 9))
).relation(ShapeRelation.WITHIN),
() -> {
XContentBuilder builder = XContentFactory.jsonBuilder().startObject()
.startObject("geo")
.startObject("shape")
.field("type", "envelope")
.startArray("coordinates")
.startArray().value(-21).value(44).endArray()
.startArray().value(-39).value(9).endArray()
.endArray()
.endObject()
.field("relation", "within")
.endObject()
.endObject();
try (XContentParser parser = createParser(builder)){
parser.nextToken();
return GeoShapeQueryBuilder.fromXContent(parser);
}
},
() -> {
XContentBuilder builder = XContentFactory.jsonBuilder().startObject()
.startObject("geo")
.field("shape", "BBOX (-21, -39, 44, 9)")
.field("relation", "within")
.endObject()
.endObject();
try (XContentParser parser = createParser(builder)){
parser.nextToken();
return GeoShapeQueryBuilder.fromXContent(parser);
}
}
);

SearchResponse response = client().prepareSearch("test")
.setQuery(querySupplier.get())
.execute().actionGet();
assertEquals(2, response.getHits().getTotalHits());
assertNotEquals("1", response.getHits().getAt(0).getId());
assertNotEquals("1", response.getHits().getAt(1).getId());
}
}

0 comments on commit 94bde37

Please sign in to comment.