Skip to content

Commit 854bfa6

Browse files
craigtavernercraigtaverner
authored
Fix 92551 with point touching edge (elastic#92769)
* Fix 92551 with point touching edge If an H3 cell touches an outside polygon edge, it should be considered intersecting. * Always include border for point crossing line When a triangle tests if a point intersects a line, the case of the point just touching the line also counts as intersecting. * Fix issue of long rectangles contained in h3 cells A long rectangle can cross the cell edges due to distortion, so we need to check for this case. Previously we only considered this for dateline crossings. * Extended fix to more cases However, it turned out that the issue was already fixed in another PR, so this commit merely improves test coverage to the known tricky cases. Also the calculation of expected results in GeoHexTilerTests was optimized to not repeatedly create the same tilers.
1 parent 98e067b commit 854bfa6

File tree

6 files changed

+239
-33
lines changed

6 files changed

+239
-33
lines changed

‎x-pack/plugin/spatial/src/internalClusterTest/java/org/elasticsearch/xpack/spatial/search/GeoGridAggAndQueryConsistencyIT.java‎

Lines changed: 93 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,11 +88,102 @@ public void testGeoShapeGeoTile() throws IOException {
8888
);
8989
}
9090

91-
@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/92551")
9291
public void testGeoShapeGeoHex() throws IOException {
9392
doTestGeohexGrid(GeoShapeWithDocValuesFieldMapper.CONTENT_TYPE, () -> GeometryTestUtils.randomGeometryWithoutCircle(0, false));
9493
}
9594

95+
public void testKnownIssueWithCellLeftOfDatelineTouchingPolygonOnRightOfDateline() throws IOException {
96+
XContentBuilder xcb = XContentFactory.jsonBuilder()
97+
.startObject()
98+
.startObject("properties")
99+
.startObject("geometry")
100+
.field("type", "geo_shape")
101+
.endObject()
102+
.endObject()
103+
.endObject();
104+
client().admin().indices().prepareCreate("test").setMapping(xcb).get();
105+
106+
BulkRequestBuilder builder = client().prepareBulk();
107+
builder.add(
108+
new IndexRequest("test").source("{\"geometry\" : \"BBOX (179.99999, 180.0, -11.29550, -11.29552)\"}", XContentType.JSON)
109+
);
110+
builder.add(
111+
new IndexRequest("test").source("{\"geometry\" : \"BBOX (-180.0, -179.99999, -11.29550, -11.29552)\"}", XContentType.JSON)
112+
);
113+
114+
assertFalse(builder.get().hasFailures());
115+
client().admin().indices().prepareRefresh("test").get();
116+
117+
GeoBoundingBox boundingBox = new GeoBoundingBox(new GeoPoint(-11.29550, 179.999992), new GeoPoint(-11.29552, -179.999992));
118+
119+
GeoGridAggregationBuilder builderPoint = new GeoHexGridAggregationBuilder("geometry").field("geometry")
120+
.precision(15)
121+
.setGeoBoundingBox(boundingBox)
122+
.size(256 * 256);
123+
SearchResponse response = client().prepareSearch("test").addAggregation(builderPoint).setSize(0).get();
124+
InternalGeoGrid<?> gridPoint = response.getAggregations().get("geometry");
125+
for (InternalGeoGridBucket bucket : gridPoint.getBuckets()) {
126+
assertThat(bucket.getDocCount(), Matchers.greaterThan(0L));
127+
QueryBuilder queryBuilder = new GeoGridQueryBuilder("geometry").setGridId(
128+
GeoGridQueryBuilder.Grid.GEOHEX,
129+
bucket.getKeyAsString()
130+
);
131+
response = client().prepareSearch("test").setTrackTotalHits(true).setQuery(queryBuilder).get();
132+
assertThat(
133+
"Bucket " + bucket.getKeyAsString(),
134+
response.getHits().getTotalHits().value,
135+
Matchers.equalTo(bucket.getDocCount())
136+
);
137+
}
138+
}
139+
140+
public void testKnownIssueWithCellIntersectingPolygonAndBoundingBox() throws IOException {
141+
XContentBuilder xcb = XContentFactory.jsonBuilder()
142+
.startObject()
143+
.startObject("properties")
144+
.startObject("geometry")
145+
.field("type", "geo_shape")
146+
.endObject()
147+
.endObject()
148+
.endObject();
149+
client().admin().indices().prepareCreate("test").setMapping(xcb).get();
150+
151+
BulkRequestBuilder builder = client().prepareBulk();
152+
builder.add(
153+
new IndexRequest("test").source("{\"geometry\" : \"POINT (169.12088680200193 86.17678739494652)\"}", XContentType.JSON)
154+
);
155+
builder.add(
156+
new IndexRequest("test").source("{\"geometry\" : \"POINT (169.12088680200193 86.17678739494652)\"}", XContentType.JSON)
157+
);
158+
String mp = "POLYGON ((150.0 70.0, 150.0 85.91811374669217, 168.77544806565834 85.91811374669217, 150.0 70.0))";
159+
builder.add(new IndexRequest("test").source("{\"geometry\" : \"" + mp + "\"}", XContentType.JSON));
160+
161+
assertFalse(builder.get().hasFailures());
162+
client().admin().indices().prepareRefresh("test").get();
163+
164+
// BBOX (172.21916569181505, -173.17785081207947, 86.17678739494652, 83.01600086049713)
165+
GeoBoundingBox boundingBox = new GeoBoundingBox(
166+
new GeoPoint(86.17678739494652, 172.21916569181505),
167+
new GeoPoint(83.01600086049713, 179)
168+
);
169+
int precision = 4;
170+
GeoGridAggregationBuilder builderPoint = new GeoHexGridAggregationBuilder("geometry").field("geometry")
171+
.precision(precision)
172+
.setGeoBoundingBox(boundingBox)
173+
.size(256 * 256);
174+
SearchResponse response = client().prepareSearch("test").addAggregation(builderPoint).setSize(0).get();
175+
InternalGeoGrid<?> gridPoint = response.getAggregations().get("geometry");
176+
for (InternalGeoGridBucket bucket : gridPoint.getBuckets()) {
177+
assertThat(bucket.getDocCount(), Matchers.greaterThan(0L));
178+
QueryBuilder queryBuilder = new GeoGridQueryBuilder("geometry").setGridId(
179+
GeoGridQueryBuilder.Grid.GEOHEX,
180+
bucket.getKeyAsString()
181+
);
182+
response = client().prepareSearch("test").setTrackTotalHits(true).setQuery(queryBuilder).get();
183+
assertThat(response.getHits().getTotalHits().value, Matchers.equalTo(bucket.getDocCount()));
184+
}
185+
}
186+
96187
private void doTestGeohashGrid(String fieldType, Supplier<Geometry> randomGeometriesSupplier) throws IOException {
97188
doTestGrid(
98189
1,
@@ -223,7 +314,7 @@ private void assertQuery(List<InternalGeoGridBucket> buckets, BiFunction<String,
223314
QueryBuilder queryBuilder = queryFunction.apply("geometry", bucket.getKeyAsString());
224315
SearchResponse response = client().prepareSearch("test").setTrackTotalHits(true).setQuery(queryBuilder).get();
225316
assertThat(
226-
"Expected hits at precision " + precision,
317+
"Expected hits at precision " + precision + " for H3 cell " + bucket.getKeyAsString(),
227318
response.getHits().getTotalHits().value,
228319
Matchers.equalTo(bucket.getDocCount())
229320
);

‎x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/common/H3CartesianGeometry.java‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -109,14 +109,13 @@ public PointValues.Relation relate(double minX, double maxX, double minY, double
109109
if (Component2D.within(getMinX(), getMaxX(), getMinY(), getMaxY(), minX, maxX, minY, maxY)) {
110110
return PointValues.Relation.CELL_CROSSES_QUERY;
111111
}
112-
int numCorners = numberOfCorners(minX, maxX, minY, maxY);
113-
if (numCorners == 4) {
114-
// need to check in case we are crossing the dateline
115-
if (crossesDateline && H3CartesianUtil.crossesBox(xs, ys, xs.length, crossesDateline, minX, maxX, minY, maxY, true)) {
112+
ContainsCorners containsCorners = containsCorners(minX, maxX, minY, maxY);
113+
if (containsCorners == ContainsCorners.ALL) {
114+
if (H3CartesianUtil.crossesBox(xs, ys, xs.length, crossesDateline, minX, maxX, minY, maxY, true)) {
116115
return PointValues.Relation.CELL_CROSSES_QUERY;
117116
}
118117
return PointValues.Relation.CELL_INSIDE_QUERY;
119-
} else if (numCorners == 0) {
118+
} else if (containsCorners == ContainsCorners.NONE) {
120119
if (Component2D.containsPoint(xs[0], ys[0], minX, maxX, minY, maxY)) {
121120
return PointValues.Relation.CELL_CROSSES_QUERY;
122121
}
@@ -325,7 +324,7 @@ private boolean crossesLine(
325324
return H3CartesianUtil.crossesLine(xs, ys, xs.length, crossesDateline, minX, maxX, minY, maxY, aX, aY, bX, bY, includeBoundary);
326325
}
327326

328-
private int numberOfCorners(double minX, double maxX, double minY, double maxY) {
327+
private ContainsCorners containsCorners(double minX, double maxX, double minY, double maxY) {
329328
int containsCount = 0;
330329
if (contains(minX, minY)) {
331330
containsCount++;
@@ -334,18 +333,24 @@ private int numberOfCorners(double minX, double maxX, double minY, double maxY)
334333
containsCount++;
335334
}
336335
if (containsCount == 1) {
337-
return containsCount;
336+
return ContainsCorners.SOME;
338337
}
339338
if (contains(maxX, maxY)) {
340339
containsCount++;
341340
}
342341
if (containsCount == 2) {
343-
return containsCount;
342+
return ContainsCorners.SOME;
344343
}
345344
if (contains(minX, maxY)) {
346345
containsCount++;
347346
}
348-
return containsCount;
347+
return containsCount == 0 ? ContainsCorners.NONE : containsCount == 4 ? ContainsCorners.ALL : ContainsCorners.SOME;
348+
}
349+
350+
private enum ContainsCorners {
351+
NONE,
352+
SOME,
353+
ALL
349354
}
350355
}
351356
}

‎x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/search/aggregations/bucket/geogrid/GeoHexVisitor.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ private GeoRelation relateLine(double aX, double aY, double bX, double bY) {
234234
return GeoRelation.QUERY_CONTAINS;
235235
}
236236
// 2. check crossings
237-
if (edgeIntersectsQuery(aX, aY, bX, bY, true)) {
237+
if (edgeIntersectsQuery(aX, aY, bX, bY)) {
238238
return GeoRelation.QUERY_CROSSES;
239239
}
240240
return GeoRelation.QUERY_DISJOINT;
@@ -297,21 +297,21 @@ private GeoRelation relateTriangle(
297297
}
298298

299299
boolean within = false;
300-
if (edgeIntersectsQuery(aX, aY, bX, bY, false)) {
300+
if (edgeIntersectsQuery(aX, aY, bX, bY)) {
301301
if (ab) {
302302
return GeoRelation.QUERY_CROSSES;
303303
}
304304
within = true;
305305
}
306306

307-
if (edgeIntersectsQuery(bX, bY, cX, cY, false)) {
307+
if (edgeIntersectsQuery(bX, bY, cX, cY)) {
308308
if (bc) {
309309
return GeoRelation.QUERY_CROSSES;
310310
}
311311
within = true;
312312
}
313313

314-
if (edgeIntersectsQuery(cX, cY, aX, aY, false)) {
314+
if (edgeIntersectsQuery(cX, cY, aX, aY)) {
315315
if (ca) {
316316
return GeoRelation.QUERY_CROSSES;
317317
}
@@ -328,13 +328,13 @@ private GeoRelation relateTriangle(
328328
/**
329329
* returns true if the edge (defined by (ax, ay) (bx, by)) intersects the query
330330
*/
331-
private boolean edgeIntersectsQuery(double ax, double ay, double bx, double by, boolean includeBoundary) {
331+
private boolean edgeIntersectsQuery(double ax, double ay, double bx, double by) {
332332
final double minX = Math.min(ax, bx);
333333
final double maxX = Math.max(ax, bx);
334334
final double minY = Math.min(ay, by);
335335
final double maxY = Math.max(ay, by);
336336
return boxesAreDisjoint(minX, maxX, minY, maxY) == false
337-
&& H3CartesianUtil.crossesLine(xs, ys, numPoints, crossesDateline, minX, maxX, minY, maxY, ax, ay, bx, by, includeBoundary);
337+
&& H3CartesianUtil.crossesLine(xs, ys, numPoints, crossesDateline, minX, maxX, minY, maxY, ax, ay, bx, by, true);
338338
}
339339

340340
/**

‎x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/common/H3CartesianGeometryTests.java‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0; you may not use this file except in compliance with the Elastic License
5+
* 2.0.
6+
*/
7+
8+
package org.elasticsearch.xpack.spatial.common;
9+
10+
import org.apache.lucene.geo.Component2D;
11+
import org.apache.lucene.geo.LatLonGeometry;
12+
import org.apache.lucene.index.PointValues;
13+
import org.elasticsearch.h3.H3;
14+
import org.elasticsearch.test.ESTestCase;
15+
16+
public class H3CartesianGeometryTests extends ESTestCase {
17+
public void testPolarH3Crossing() {
18+
long h3 = H3.geoToH3(-90, 0, 2);
19+
Component2D component2D = LatLonGeometry.create(H3CartesianUtil.getLatLonGeometry(h3));
20+
assertEquals(
21+
PointValues.Relation.CELL_CROSSES_QUERY,
22+
component2D.relate(-155.15317029319704, 163.99789148010314, -88.69673718698323, -88.26949733309448)
23+
);
24+
}
25+
}

‎x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/search/aggregations/bucket/geogrid/GeoHexTilerTests.java‎

Lines changed: 65 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,52 @@ public void testTroublesomePolarCellLevel1_UnboundedGeoShapeCellValues() throws
200200
assertThat("[" + precision + "] bucket count", numBuckets, equalTo(expected));
201201
}
202202

203+
public void testTroublesomeCellLevel2_BoundedGeoShapeCellValues() throws Exception {
204+
int precision = 2;
205+
String wkt = """
206+
GEOMETRYCOLLECTION (
207+
GEOMETRYCOLLECTION (
208+
POINT(-170 0),
209+
POINT (-178.5 0)
210+
)
211+
)
212+
""";
213+
GeoBoundingBox boundingBox = new GeoBoundingBox(new GeoPoint(4E-4, 179.999), new GeoPoint(-4E-4, -179.999));
214+
Geometry geometry = WellKnownText.fromWKT(StandardValidator.instance(true), true, wkt);
215+
GeoShapeValues.GeoShapeValue value = geoShapeValue(geometry);
216+
GeoShapeCellValues cellValues = new GeoShapeCellValues(
217+
makeGeoShapeValues(value),
218+
getBoundedGridTiler(boundingBox, precision),
219+
NOOP_BREAKER
220+
);
221+
222+
assertTrue(cellValues.advanceExact(0));
223+
int numBuckets = cellValues.docValueCount();
224+
int expected = expectedBuckets(value, precision, boundingBox);
225+
assertThat("[" + precision + "] bucket count", numBuckets, equalTo(expected));
226+
}
227+
228+
public void testTroublesomeCellLevel4_BoundedGeoShapeCellValues() throws Exception {
229+
int precision = 4;
230+
String polygon = "POLYGON ((150.0 70.0, 150.0 85.91811374669217, 168.77544806565834 85.91811374669217, 150.0 70.0))";
231+
Geometry geometry = WellKnownText.fromWKT(StandardValidator.instance(true), true, polygon);
232+
GeoBoundingBox boundingBox = new GeoBoundingBox(
233+
new GeoPoint(86.17678739494652, 172.21916569181505),
234+
new GeoPoint(83.01600086049713, 179)
235+
);
236+
GeoShapeValues.GeoShapeValue value = geoShapeValue(geometry);
237+
GeoShapeCellValues cellValues = new GeoShapeCellValues(
238+
makeGeoShapeValues(value),
239+
getBoundedGridTiler(boundingBox, precision),
240+
NOOP_BREAKER
241+
);
242+
243+
assertTrue(cellValues.advanceExact(0));
244+
int numBuckets = cellValues.docValueCount();
245+
int expected = expectedBuckets(value, precision, boundingBox);
246+
assertThat("[" + precision + "] bucket count", numBuckets, equalTo(expected));
247+
}
248+
203249
private void assertCorner(long[] h3bins, Point point, int precision, String msg) throws IOException {
204250
GeoShapeValues.GeoShapeValue cornerValue = geoShapeValue(point);
205251
GeoShapeCellValues cornerValues = new GeoShapeCellValues(
@@ -260,37 +306,40 @@ private int addBruteForce(
260306

261307
@Override
262308
protected int expectedBuckets(GeoShapeValues.GeoShapeValue geoValue, int precision, GeoBoundingBox bbox) throws Exception {
263-
return computeBuckets(H3.getLongRes0Cells(), bbox, geoValue, precision);
309+
BoundedGeoHexGridTiler bounded = bbox == null ? null : new BoundedGeoHexGridTiler(precision, bbox);
310+
UnboundedGeoHexGridTiler predicate = new UnboundedGeoHexGridTiler(precision);
311+
return computeBuckets(H3.getLongRes0Cells(), bounded, predicate, geoValue, precision);
264312
}
265313

266-
private int computeBuckets(long[] children, GeoBoundingBox bbox, GeoShapeValues.GeoShapeValue geoValue, int finalPrecision)
267-
throws IOException {
314+
private int computeBuckets(
315+
long[] children,
316+
BoundedGeoHexGridTiler bounded,
317+
UnboundedGeoHexGridTiler predicate,
318+
GeoShapeValues.GeoShapeValue geoValue,
319+
int finalPrecision
320+
) throws IOException {
268321
int count = 0;
269322
for (long child : children) {
270323
if (H3.getResolution(child) == finalPrecision) {
271-
if (intersects(child, geoValue, bbox, finalPrecision)) {
324+
if (intersects(child, geoValue, bounded, predicate)) {
272325
count++;
273326
}
274327
} else {
275-
count += computeBuckets(H3.h3ToChildren(child), bbox, geoValue, finalPrecision);
328+
count += computeBuckets(H3.h3ToChildren(child), bounded, predicate, geoValue, finalPrecision);
276329
}
277330
}
278331
return count;
279332
}
280333

281-
private boolean intersects(long h3, GeoShapeValues.GeoShapeValue geoValue, GeoBoundingBox bbox, int finalPrecision) throws IOException {
282-
if (addressIntersectsBounds(h3, bbox, finalPrecision) == false) {
334+
private boolean intersects(
335+
long h3,
336+
GeoShapeValues.GeoShapeValue geoValue,
337+
BoundedGeoHexGridTiler bounded,
338+
UnboundedGeoHexGridTiler predicate
339+
) throws IOException {
340+
if (bounded != null && bounded.h3IntersectsBounds(h3) == false) {
283341
return false;
284342
}
285-
UnboundedGeoHexGridTiler predicate = new UnboundedGeoHexGridTiler(finalPrecision);
286343
return predicate.relateTile(geoValue, h3) != GeoRelation.QUERY_DISJOINT;
287344
}
288-
289-
private boolean addressIntersectsBounds(long h3, GeoBoundingBox bbox, int finalPrecision) {
290-
if (bbox == null) {
291-
return true;
292-
}
293-
BoundedGeoHexGridTiler predicate = new BoundedGeoHexGridTiler(finalPrecision, bbox);
294-
return predicate.h3IntersectsBounds(h3);
295-
}
296345
}

0 commit comments

Comments
 (0)