Skip to content

Commit d07f1ed

Browse files
Fixed flaky #98063 with MultiPoint identical to Point (#98872) (#98967)
We had previously fixed a similar case where all points in a line were the same and identical to the test point, so WITHIN and CONTAINS would both be true. This fix generalizes this to all geometries. The particular failing test involved a MultiPoint with two points at (0 0) and one close enough to that to quantize to it. Rather than just add a fix for MultiPoint, we added a GeometryVisitor supporting all possible geometry types so we cover future potential failures of the same type.
1 parent 37d3b05 commit d07f1ed

File tree

1 file changed

+128
-12
lines changed

1 file changed

+128
-12
lines changed

x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/fielddata/LatLonGeometryRelationVisitorTests.java

Lines changed: 128 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,15 @@
1717
import org.elasticsearch.common.geo.GeometryNormalizer;
1818
import org.elasticsearch.common.geo.Orientation;
1919
import org.elasticsearch.geo.GeometryTestUtils;
20+
import org.elasticsearch.geometry.Circle;
2021
import org.elasticsearch.geometry.Geometry;
22+
import org.elasticsearch.geometry.GeometryCollection;
23+
import org.elasticsearch.geometry.GeometryVisitor;
2124
import org.elasticsearch.geometry.LinearRing;
25+
import org.elasticsearch.geometry.MultiLine;
2226
import org.elasticsearch.geometry.MultiPoint;
27+
import org.elasticsearch.geometry.MultiPolygon;
28+
import org.elasticsearch.geometry.Rectangle;
2329
import org.elasticsearch.test.ESTestCase;
2430
import org.elasticsearch.xpack.spatial.util.GeoTestUtils;
2531

@@ -135,6 +141,18 @@ public void testContainedPolygons() throws Exception {
135141
}
136142
}
137143

144+
/** Explicitly test failure found in <a href="https://github.com/elastic/elasticsearch/issues/98063">#98063</a> */
145+
public void testOriginPointInMultipoint() throws Exception {
146+
ArrayList<org.elasticsearch.geometry.Point> points = new ArrayList<>();
147+
points.add(new org.elasticsearch.geometry.Point(0.0, 0.0));
148+
points.add(new org.elasticsearch.geometry.Point(0.0, 0.0));
149+
points.add(new org.elasticsearch.geometry.Point(0.0, 1.401298464324817E-45));
150+
Geometry geometry = new MultiPoint(points);
151+
GeoShapeValues.GeoShapeValue geoShapeValue = GeoTestUtils.geoShapeValue(geometry);
152+
GeometryDocValueReader reader = GeoTestUtils.geometryDocValueReader(geometry, CoordinateEncoder.GEO);
153+
doTestShape(geometry, geoShapeValue, reader, new Point(0, 0));
154+
}
155+
138156
private <T extends LatLonGeometry> void doTestShapes(Supplier<T> supplier) throws Exception {
139157
Geometry geometry = GeometryNormalizer.apply(Orientation.CCW, GeometryTestUtils.randomGeometryWithoutCircle(0, false));
140158
GeoShapeValues.GeoShapeValue geoShapeValue = GeoTestUtils.geoShapeValue(geometry);
@@ -161,18 +179,7 @@ private void doTestShape(Geometry geometry, GeometryDocValueReader reader, LatLo
161179

162180
private boolean isIdenticalPoint(Geometry geometry, LatLonGeometry latLonGeometry) {
163181
if (latLonGeometry instanceof Point latLonPoint) {
164-
if (geometry instanceof org.elasticsearch.geometry.Point point) {
165-
return encodeLatitude(point.getLat()) == encodeLatitude(latLonPoint.getLat())
166-
&& encodeLongitude(point.getLon()) == encodeLongitude(latLonPoint.getLon());
167-
} else if (geometry instanceof org.elasticsearch.geometry.Line line) {
168-
for (int i = 0; i < line.length(); i++) {
169-
if (encodeLatitude(line.getLat(i)) != encodeLatitude(latLonPoint.getLat())
170-
|| encodeLongitude(line.getLon(i)) != encodeLongitude(latLonPoint.getLon())) {
171-
return false;
172-
}
173-
}
174-
return true;
175-
}
182+
return geometry.visit(new TestIdenticalPointVisitor(latLonPoint));
176183
}
177184
return false;
178185
}
@@ -280,4 +287,113 @@ private double quantizeLat(double lat) {
280287
private double quantizeLon(double lon) {
281288
return decodeLongitude(encodeLongitude(lon));
282289
}
290+
291+
/**
292+
* This visitor returns false if any point in the geometry is not identical to the provided point.
293+
* Identical means that the encoded lat and lon values are the same.
294+
*/
295+
private static class TestIdenticalPointVisitor implements GeometryVisitor<Boolean, RuntimeException> {
296+
private final int encodedLat;
297+
private final int encodedLon;
298+
299+
private TestIdenticalPointVisitor(Point latLonPoint) {
300+
encodedLat = encodeLatitude(latLonPoint.getLat());
301+
encodedLon = encodeLongitude(latLonPoint.getLon());
302+
}
303+
304+
private boolean isIdenticalPoint(double lat, double lon) {
305+
return encodeLatitude(lat) == encodedLat && encodeLongitude(lon) == encodedLon;
306+
}
307+
308+
@Override
309+
public Boolean visit(Circle circle) {
310+
if (circle.getRadiusMeters() == 0) {
311+
return isIdenticalPoint(circle.getLat(), circle.getLon());
312+
}
313+
return false;
314+
}
315+
316+
@Override
317+
public Boolean visit(GeometryCollection<?> collection) {
318+
for (Geometry shape : collection) {
319+
if (shape.visit(this) == false) {
320+
return false;
321+
}
322+
}
323+
return collection.size() > 0;
324+
}
325+
326+
@Override
327+
public Boolean visit(org.elasticsearch.geometry.Line line) {
328+
for (int i = 0; i < line.length(); i++) {
329+
if (isIdenticalPoint(line.getLat(i), line.getLon(i)) == false) {
330+
return false;
331+
}
332+
}
333+
return line.length() > 0;
334+
}
335+
336+
@Override
337+
public Boolean visit(LinearRing ring) {
338+
return visit((org.elasticsearch.geometry.Line) ring);
339+
}
340+
341+
@Override
342+
public Boolean visit(MultiLine multiLine) {
343+
for (org.elasticsearch.geometry.Line line : multiLine) {
344+
if (visit(line) == false) {
345+
return false;
346+
}
347+
}
348+
return multiLine.size() > 0;
349+
}
350+
351+
@Override
352+
public Boolean visit(MultiPoint multiPoint) {
353+
for (org.elasticsearch.geometry.Point point : multiPoint) {
354+
if (visit(point) == false) {
355+
return false;
356+
}
357+
}
358+
return multiPoint.size() > 0;
359+
}
360+
361+
@Override
362+
public Boolean visit(MultiPolygon multiPolygon) {
363+
for (org.elasticsearch.geometry.Polygon polygon : multiPolygon) {
364+
if (visit(polygon) == false) {
365+
return false;
366+
}
367+
}
368+
return multiPolygon.size() > 0;
369+
}
370+
371+
@Override
372+
public Boolean visit(org.elasticsearch.geometry.Point point) {
373+
return isIdenticalPoint(point.getLat(), point.getLon());
374+
}
375+
376+
@Override
377+
public Boolean visit(org.elasticsearch.geometry.Polygon polygon) {
378+
if (visit(polygon.getPolygon()) == false) {
379+
return false;
380+
}
381+
for (int i = 0; i < polygon.getNumberOfHoles(); i++) {
382+
LinearRing hole = polygon.getHole(i);
383+
if (visit(hole) == false) {
384+
return false;
385+
}
386+
}
387+
return polygon.getPolygon().length() > 0;
388+
}
389+
390+
@Override
391+
public Boolean visit(Rectangle rectangle) {
392+
int eMinX = encodeLongitude(rectangle.getMinX());
393+
int eMaxX = encodeLongitude(rectangle.getMaxX());
394+
int eMinY = encodeLatitude(rectangle.getMinY());
395+
int eMaxY = encodeLatitude(rectangle.getMaxY());
396+
return eMinX == eMaxX && eMinY == eMaxY && isIdenticalPoint(rectangle.getMinLat(), rectangle.getMinLon());
397+
}
398+
}
283399
}

0 commit comments

Comments
 (0)