This is an automated email from the ASF dual-hosted git repository. jiayu pushed a commit to branch fix/geometry-bugs-2720-2723 in repository https://gitbox.apache.org/repos/asf/sedona.git
commit 80d6c00029c60ca65084d550190485c580505294 Author: Jia Yu <[email protected]> AuthorDate: Wed Mar 11 19:34:54 2026 -0700 [GH-2720][GH-2721][GH-2722][GH-2723] Fix geometry function bugs - ST_FrechetDistance: return Double.NaN instead of 0.0 for empty geometries - ST_HausdorffDistance: return Double.NaN instead of 0.0 for empty geometries - ST_Equals: use equalsTopo instead of symDifference to support GeometryCollection - ST_VoronoiPolygons: move from FunctionsGeoTools to Functions (no GeoTools dependency) --- .../java/org/apache/sedona/common/Functions.java | 20 +++++++++++++++++++- .../org/apache/sedona/common/FunctionsGeoTools.java | 19 ------------------- .../java/org/apache/sedona/common/Predicates.java | 2 +- .../org/apache/sedona/common/utils/GeomUtils.java | 6 +++--- .../org/apache/sedona/common/FunctionsTest.java | 21 +++++++++------------ .../org/apache/sedona/common/PredicatesTest.java | 10 ++++++++++ .../apache/sedona/flink/expressions/Functions.java | 7 +++---- .../org/apache/sedona/snowflake/snowsql/UDFs.java | 9 ++++----- .../org/apache/sedona/snowflake/snowsql/UDFsV2.java | 9 ++++----- .../sql/sedona_sql/expressions/Functions.scala | 2 +- 10 files changed, 54 insertions(+), 51 deletions(-) diff --git a/common/src/main/java/org/apache/sedona/common/Functions.java b/common/src/main/java/org/apache/sedona/common/Functions.java index 62232d2e20..3760c33115 100644 --- a/common/src/main/java/org/apache/sedona/common/Functions.java +++ b/common/src/main/java/org/apache/sedona/common/Functions.java @@ -70,6 +70,7 @@ import org.locationtech.jts.simplify.PolygonHullSimplifier; import org.locationtech.jts.simplify.TopologyPreservingSimplifier; import org.locationtech.jts.simplify.VWSimplifier; import org.locationtech.jts.triangulate.DelaunayTriangulationBuilder; +import org.locationtech.jts.triangulate.VoronoiDiagramBuilder; import org.locationtech.jts.triangulate.polygon.ConstrainedDelaunayTriangulator; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -1153,6 +1154,23 @@ public class Functions { return 0; } + public static Geometry voronoiPolygons(Geometry geom, double tolerance, Geometry extendTo) { + if (geom == null) { + return null; + } + VoronoiDiagramBuilder builder = new VoronoiDiagramBuilder(); + builder.setSites(geom); + builder.setTolerance(tolerance); + if (extendTo != null) { + builder.setClipEnvelope(extendTo.getEnvelopeInternal()); + } else { + Envelope e = geom.getEnvelopeInternal(); + e.expandBy(Math.max(e.getWidth(), e.getHeight())); + builder.setClipEnvelope(e); + } + return builder.getDiagram(geom.getFactory()); + } + public static Geometry concaveHull(Geometry geometry, double pctConvex, boolean allowHoles) { ConcaveHull concave_hull = new ConcaveHull(geometry); concave_hull.setMaximumEdgeLengthRatio(pctConvex); @@ -2478,7 +2496,7 @@ public class Functions { return geometricMedian(geometry, DEFAULT_TOLERANCE, DEFAULT_MAX_ITER, false); } - public static double frechetDistance(Geometry g1, Geometry g2) { + public static Double frechetDistance(Geometry g1, Geometry g2) { return GeomUtils.getFrechetDistance(g1, g2); } diff --git a/common/src/main/java/org/apache/sedona/common/FunctionsGeoTools.java b/common/src/main/java/org/apache/sedona/common/FunctionsGeoTools.java index 3ae6f5b105..741cdb724c 100644 --- a/common/src/main/java/org/apache/sedona/common/FunctionsGeoTools.java +++ b/common/src/main/java/org/apache/sedona/common/FunctionsGeoTools.java @@ -29,11 +29,9 @@ import org.geotools.geometry.jts.JTS; import org.geotools.referencing.CRS; import org.geotools.referencing.ReferencingFactoryFinder; import org.geotools.util.factory.Hints; -import org.locationtech.jts.geom.Envelope; import org.locationtech.jts.geom.Geometry; import org.locationtech.jts.operation.buffer.BufferOp; import org.locationtech.jts.operation.buffer.BufferParameters; -import org.locationtech.jts.triangulate.VoronoiDiagramBuilder; public class FunctionsGeoTools { @@ -174,23 +172,6 @@ public class FunctionsGeoTools { } } - public static Geometry voronoiPolygons(Geometry geom, double tolerance, Geometry extendTo) { - if (geom == null) { - return null; - } - VoronoiDiagramBuilder builder = new VoronoiDiagramBuilder(); - builder.setSites(geom); - builder.setTolerance(tolerance); - if (extendTo != null) { - builder.setClipEnvelope(extendTo.getEnvelopeInternal()); - } else { - Envelope e = geom.getEnvelopeInternal(); - e.expandBy(Math.max(e.getWidth(), e.getHeight())); - builder.setClipEnvelope(e); - } - return builder.getDiagram(geom.getFactory()); - } - public static Geometry bufferSpheroid(Geometry geometry, double radius, BufferParameters params) throws IllegalArgumentException { // Determine the best SRID for spheroidal calculations diff --git a/common/src/main/java/org/apache/sedona/common/Predicates.java b/common/src/main/java/org/apache/sedona/common/Predicates.java index 1db1f92828..21fd11cff9 100644 --- a/common/src/main/java/org/apache/sedona/common/Predicates.java +++ b/common/src/main/java/org/apache/sedona/common/Predicates.java @@ -56,7 +56,7 @@ public class Predicates { } public static boolean equals(Geometry leftGeometry, Geometry rightGeometry) { - return leftGeometry.symDifference(rightGeometry).isEmpty(); + return leftGeometry.equalsTopo(rightGeometry); } public static boolean disjoint(Geometry leftGeometry, Geometry rightGeometry) { diff --git a/common/src/main/java/org/apache/sedona/common/utils/GeomUtils.java b/common/src/main/java/org/apache/sedona/common/utils/GeomUtils.java index e5611682cd..c9d31228cf 100644 --- a/common/src/main/java/org/apache/sedona/common/utils/GeomUtils.java +++ b/common/src/main/java/org/apache/sedona/common/utils/GeomUtils.java @@ -542,13 +542,13 @@ public class GeomUtils { geometry.geometryChanged(); } - public static double getFrechetDistance(Geometry g1, Geometry g2) { - if (g1.isEmpty() || g2.isEmpty()) return 0.0; + public static Double getFrechetDistance(Geometry g1, Geometry g2) { + if (g1.isEmpty() || g2.isEmpty()) return Double.NaN; return DiscreteFrechetDistance.distance(g1, g2); } public static Double getHausdorffDistance(Geometry g1, Geometry g2, double densityFrac) { - if (g1.isEmpty() || g2.isEmpty()) return 0.0; + if (g1.isEmpty() || g2.isEmpty()) return Double.NaN; DiscreteHausdorffDistance hausdorffDistanceObj = new DiscreteHausdorffDistance(g1, g2); if (densityFrac != -1) { hausdorffDistanceObj.setDensifyFraction(densityFrac); diff --git a/common/src/test/java/org/apache/sedona/common/FunctionsTest.java b/common/src/test/java/org/apache/sedona/common/FunctionsTest.java index 6d9e04b65f..32bb24bb3d 100644 --- a/common/src/test/java/org/apache/sedona/common/FunctionsTest.java +++ b/common/src/test/java/org/apache/sedona/common/FunctionsTest.java @@ -3564,9 +3564,8 @@ public class FunctionsTest extends TestBase { public void testFrechetGeomEmpty() { Polygon p1 = GEOMETRY_FACTORY.createPolygon(coordArray(1, 0, 1, 1, 2, 1, 2, 0, 1, 0)); LineString emptyPoint = GEOMETRY_FACTORY.createLineString(); - double expected = 0.0; - double actual = Functions.frechetDistance(p1, emptyPoint); - assertEquals(expected, actual, 1e-9); + Double actual = Functions.frechetDistance(p1, emptyPoint); + assertTrue(Double.isNaN(actual)); } @Test @@ -4278,18 +4277,16 @@ public class FunctionsTest extends TestBase { public void hausdorffDistanceEmptyGeom() throws Exception { Polygon polygon = GEOMETRY_FACTORY.createPolygon(coordArray(1, 2, 2, 1, 2, 0, 4, 1, 1, 2)); LineString emptyLineString = GEOMETRY_FACTORY.createLineString(); - Double expected = 0.0; Double actual = Functions.hausdorffDistance(polygon, emptyLineString, 0.00001); - assertEquals(expected, actual); + assertTrue(Double.isNaN(actual)); } @Test public void hausdorffDistanceDefaultEmptyGeom() throws Exception { Polygon polygon = GEOMETRY_FACTORY.createPolygon(coordArray(1, 2, 2, 1, 2, 0, 4, 1, 1, 2)); LineString emptyLineString = GEOMETRY_FACTORY.createLineString(); - Double expected = 0.0; Double actual = Functions.hausdorffDistance(polygon, emptyLineString); - assertEquals(expected, actual); + assertTrue(Double.isNaN(actual)); } @Test @@ -4367,26 +4364,26 @@ public class FunctionsTest extends TestBase { @Test public void voronoiPolygons() { MultiPoint multiPoint = GEOMETRY_FACTORY.createMultiPointFromCoords(coordArray(0, 0, 2, 2)); - Geometry actual1 = FunctionsGeoTools.voronoiPolygons(multiPoint, 0, null); + Geometry actual1 = Functions.voronoiPolygons(multiPoint, 0, null); assertGeometryEquals( "GEOMETRYCOLLECTION (POLYGON ((-2 -2, -2 4, 4 -2, -2 -2)), POLYGON ((-2 4, 4 4, 4 -2, -2 4)))", actual1.toText()); - Geometry actual2 = FunctionsGeoTools.voronoiPolygons(multiPoint, 30, null); + Geometry actual2 = Functions.voronoiPolygons(multiPoint, 30, null); assertGeometryEquals( "GEOMETRYCOLLECTION (POLYGON ((-2 -2, -2 4, 4 4, 4 -2, -2 -2)))", actual2.toText()); Geometry buf = Functions.buffer(GEOMETRY_FACTORY.createPoint(new Coordinate(1, 1)), 10); - Geometry actual3 = FunctionsGeoTools.voronoiPolygons(multiPoint, 0, buf); + Geometry actual3 = Functions.voronoiPolygons(multiPoint, 0, buf); assertGeometryEquals( "GEOMETRYCOLLECTION (POLYGON ((-9 -9, -9 11, 11 -9, -9 -9)), POLYGON ((-9 11, 11 11, 11 -9, -9 11)))", actual3.toText()); - Geometry actual4 = FunctionsGeoTools.voronoiPolygons(multiPoint, 30, buf); + Geometry actual4 = Functions.voronoiPolygons(multiPoint, 30, buf); assertGeometryEquals( "GEOMETRYCOLLECTION (POLYGON ((-9 -9, -9 11, 11 11, 11 -9, -9 -9)))", actual4.toText()); - Geometry actual5 = FunctionsGeoTools.voronoiPolygons(null, 0, null); + Geometry actual5 = Functions.voronoiPolygons(null, 0, null); assertEquals(null, actual5); } diff --git a/common/src/test/java/org/apache/sedona/common/PredicatesTest.java b/common/src/test/java/org/apache/sedona/common/PredicatesTest.java index 0b4c0f0b4a..7e8616c976 100644 --- a/common/src/test/java/org/apache/sedona/common/PredicatesTest.java +++ b/common/src/test/java/org/apache/sedona/common/PredicatesTest.java @@ -92,6 +92,16 @@ public class PredicatesTest extends TestBase { assertEquals("1010F0212", actual); } + @Test + public void testEqualsGeometryCollection() throws ParseException { + Geometry gc1 = geomFromEWKT("GEOMETRYCOLLECTION(POINT(0 0), LINESTRING(0 0, 1 1))"); + Geometry gc2 = geomFromEWKT("GEOMETRYCOLLECTION(POINT(0 0), LINESTRING(0 0, 1 1))"); + assertTrue(Predicates.equals(gc1, gc2)); + + Geometry gc3 = geomFromEWKT("GEOMETRYCOLLECTION(POINT(0 0), LINESTRING(0 0, 2 2))"); + assertFalse(Predicates.equals(gc1, gc3)); + } + @Test public void testRelateBoolean() throws ParseException { Geometry geom1 = geomFromEWKT("POINT(1 2)"); diff --git a/flink/src/main/java/org/apache/sedona/flink/expressions/Functions.java b/flink/src/main/java/org/apache/sedona/flink/expressions/Functions.java index c5610fe1d6..635cf86a2d 100644 --- a/flink/src/main/java/org/apache/sedona/flink/expressions/Functions.java +++ b/flink/src/main/java/org/apache/sedona/flink/expressions/Functions.java @@ -23,7 +23,6 @@ import org.apache.commons.lang3.tuple.Pair; import org.apache.flink.table.annotation.DataTypeHint; import org.apache.flink.table.annotation.InputGroup; import org.apache.flink.table.functions.ScalarFunction; -import org.apache.sedona.common.FunctionsGeoTools; import org.apache.sedona.flink.GeometryArrayTypeSerializer; import org.apache.sedona.flink.GeometryTypeSerializer; import org.geotools.api.referencing.FactoryException; @@ -3039,7 +3038,7 @@ public class Functions { Object extend) { Geometry geom = (Geometry) o; Geometry extendTo = (Geometry) extend; - return FunctionsGeoTools.voronoiPolygons(geom, tolerance, extendTo); + return org.apache.sedona.common.Functions.voronoiPolygons(geom, tolerance, extendTo); } @DataTypeHint( @@ -3054,7 +3053,7 @@ public class Functions { Object o, @DataTypeHint("Double") Double tolerance) { Geometry geom = (Geometry) o; - return FunctionsGeoTools.voronoiPolygons(geom, tolerance, null); + return org.apache.sedona.common.Functions.voronoiPolygons(geom, tolerance, null); } @DataTypeHint( @@ -3068,7 +3067,7 @@ public class Functions { bridgedTo = Geometry.class) Object o) { Geometry geom = (Geometry) o; - return FunctionsGeoTools.voronoiPolygons(geom, 0, null); + return org.apache.sedona.common.Functions.voronoiPolygons(geom, 0, null); } } diff --git a/snowflake/src/main/java/org/apache/sedona/snowflake/snowsql/UDFs.java b/snowflake/src/main/java/org/apache/sedona/snowflake/snowsql/UDFs.java index a7d98082ea..f9f56101a9 100644 --- a/snowflake/src/main/java/org/apache/sedona/snowflake/snowsql/UDFs.java +++ b/snowflake/src/main/java/org/apache/sedona/snowflake/snowsql/UDFs.java @@ -22,7 +22,6 @@ import java.io.IOException; import javax.xml.parsers.ParserConfigurationException; import org.apache.sedona.common.Constructors; import org.apache.sedona.common.Functions; -import org.apache.sedona.common.FunctionsGeoTools; import org.apache.sedona.common.FunctionsProj4; import org.apache.sedona.common.Predicates; import org.apache.sedona.common.enums.FileDataSplitter; @@ -1171,19 +1170,19 @@ public class UDFs { @UDFAnnotations.ParamMeta(argNames = {"geometry"}) public static byte[] ST_VoronoiPolygons(byte[] geometry) { return GeometrySerde.serialize( - FunctionsGeoTools.voronoiPolygons(GeometrySerde.deserialize(geometry), 0.0, null)); + Functions.voronoiPolygons(GeometrySerde.deserialize(geometry), 0.0, null)); } @UDFAnnotations.ParamMeta(argNames = {"geometry", "tolerance"}) public static byte[] ST_VoronoiPolygons(byte[] geometry, double tolerance) { return GeometrySerde.serialize( - FunctionsGeoTools.voronoiPolygons(GeometrySerde.deserialize(geometry), tolerance, null)); + Functions.voronoiPolygons(GeometrySerde.deserialize(geometry), tolerance, null)); } @UDFAnnotations.ParamMeta(argNames = {"geometry", "tolerance", "extent"}) public static byte[] ST_VoronoiPolygons(byte[] geometry, double tolerance, byte[] extent) { return GeometrySerde.serialize( - FunctionsGeoTools.voronoiPolygons( + Functions.voronoiPolygons( GeometrySerde.deserialize(geometry), tolerance, GeometrySerde.deserialize(extent))); } @@ -1266,7 +1265,7 @@ public class UDFs { } @UDFAnnotations.ParamMeta(argNames = {"geomA", "geomB"}) - public static double ST_FrechetDistance(byte[] geomA, byte[] geomB) { + public static Double ST_FrechetDistance(byte[] geomA, byte[] geomB) { return Functions.frechetDistance( GeometrySerde.deserialize(geomA), GeometrySerde.deserialize(geomB)); } diff --git a/snowflake/src/main/java/org/apache/sedona/snowflake/snowsql/UDFsV2.java b/snowflake/src/main/java/org/apache/sedona/snowflake/snowsql/UDFsV2.java index 8fcc980d8a..4861fce4ce 100644 --- a/snowflake/src/main/java/org/apache/sedona/snowflake/snowsql/UDFsV2.java +++ b/snowflake/src/main/java/org/apache/sedona/snowflake/snowsql/UDFsV2.java @@ -20,7 +20,6 @@ package org.apache.sedona.snowflake.snowsql; import java.io.IOException; import org.apache.sedona.common.Functions; -import org.apache.sedona.common.FunctionsGeoTools; import org.apache.sedona.common.FunctionsProj4; import org.apache.sedona.common.Predicates; import org.apache.sedona.common.sphere.Haversine; @@ -1402,7 +1401,7 @@ public class UDFsV2 { returnTypes = "Geometry") public static String ST_VoronoiPolygons(String geometry) { return GeometrySerde.serGeoJson( - FunctionsGeoTools.voronoiPolygons(GeometrySerde.deserGeoJson(geometry), 0.0, null)); + Functions.voronoiPolygons(GeometrySerde.deserGeoJson(geometry), 0.0, null)); } @UDFAnnotations.ParamMeta( @@ -1411,7 +1410,7 @@ public class UDFsV2 { returnTypes = "Geometry") public static String ST_VoronoiPolygons(String geometry, double tolerance) { return GeometrySerde.serGeoJson( - FunctionsGeoTools.voronoiPolygons(GeometrySerde.deserGeoJson(geometry), tolerance, null)); + Functions.voronoiPolygons(GeometrySerde.deserGeoJson(geometry), tolerance, null)); } @UDFAnnotations.ParamMeta( @@ -1420,7 +1419,7 @@ public class UDFsV2 { returnTypes = "Geometry") public static String ST_VoronoiPolygons(String geometry, double tolerance, String extent) { return GeometrySerde.serGeoJson( - FunctionsGeoTools.voronoiPolygons( + Functions.voronoiPolygons( GeometrySerde.deserGeoJson(geometry), tolerance, GeometrySerde.deserGeoJson(extent))); } @@ -1535,7 +1534,7 @@ public class UDFsV2 { @UDFAnnotations.ParamMeta( argNames = {"geomA", "geomB"}, argTypes = {"Geometry", "Geometry"}) - public static double ST_FrechetDistance(String geomA, String geomB) { + public static Double ST_FrechetDistance(String geomA, String geomB) { return Functions.frechetDistance( GeometrySerde.deserGeoJson(geomA), GeometrySerde.deserGeoJson(geomB)); } diff --git a/spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/Functions.scala b/spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/Functions.scala index 31beddc720..0fab6060cc 100644 --- a/spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/Functions.scala +++ b/spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/Functions.scala @@ -1854,7 +1854,7 @@ private[apache] case class ST_TriangulatePolygon(inputExpressions: Seq[Expressio } private[apache] case class ST_VoronoiPolygons(inputExpressions: Seq[Expression]) - extends InferredExpression(nullTolerantInferrableFunction3(FunctionsGeoTools.voronoiPolygons)) + extends InferredExpression(nullTolerantInferrableFunction3(Functions.voronoiPolygons)) with FoldableExpression { protected def withNewChildrenInternal(newChildren: IndexedSeq[Expression]) = { copy(inputExpressions = newChildren)
