This is an automated email from the ASF dual-hosted git repository. jiayu pushed a commit to branch fix/2148-make-polygon-hole-validation in repository https://gitbox.apache.org/repos/asf/sedona.git
commit 07b303dd6a9d6093c1dc6d0b41b6841584f5bfb6 Author: Jia Yu <[email protected]> AuthorDate: Sat Feb 7 00:26:28 2026 -0800 fix(ST_MakePolygon): warn when holes lie outside shell (PostGIS-compatible) Match PostGIS behavior: create the polygon even when holes are not contained within the shell, but log a warning via slf4j. The resulting polygon will be invalid (ST_IsValid returns false), matching PostGIS output byte-for-byte. Also fix duplicate stream processing bug where the holes array was streamed twice (once to filter, once to map), which would silently produce incorrect results with non-replayable streams. Fixes #2148 --- .../java/org/apache/sedona/common/Functions.java | 25 +++++++------- .../org/apache/sedona/common/FunctionsTest.java | 40 ++++++++++++++++++++++ docs/api/flink/Function.md | 8 ++--- docs/api/snowflake/vector-data/Function.md | 16 ++++----- docs/api/sql/Function.md | 8 ++--- .../sedona/sql/functions/StMakePolygonSpec.scala | 2 +- 6 files changed, 70 insertions(+), 29 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 0be88a9b1b..b593a82065 100644 --- a/common/src/main/java/org/apache/sedona/common/Functions.java +++ b/common/src/main/java/org/apache/sedona/common/Functions.java @@ -71,10 +71,13 @@ import org.locationtech.jts.simplify.TopologyPreservingSimplifier; import org.locationtech.jts.simplify.VWSimplifier; import org.locationtech.jts.triangulate.DelaunayTriangulationBuilder; import org.locationtech.jts.triangulate.polygon.ConstrainedDelaunayTriangulator; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.wololo.geojson.Feature; import org.wololo.geojson.FeatureCollection; public class Functions { + private static final Logger log = LoggerFactory.getLogger(Functions.class); private static final double DEFAULT_TOLERANCE = 1e-6; private static final int DEFAULT_MAX_ITER = 1000; private static final int OGC_SFS_VALIDITY = 0; // Use usual OGC SFS validity semantics @@ -1935,6 +1938,7 @@ public class Functions { public static Geometry makePolygon(Geometry shell, Geometry[] holes, GeometryFactory factory) { try { + LinearRing shellRing = factory.createLinearRing(shell.getCoordinates()); if (holes != null) { LinearRing[] interiorRings = Arrays.stream(holes) @@ -1947,20 +1951,17 @@ public class Functions { .map(h -> factory.createLinearRing(h.getCoordinates())) .toArray(LinearRing[]::new); if (interiorRings.length != 0) { - return factory.createPolygon( - factory.createLinearRing(shell.getCoordinates()), - Arrays.stream(holes) - .filter( - h -> - h != null - && !h.isEmpty() - && h instanceof LineString - && ((LineString) h).isClosed()) - .map(h -> factory.createLinearRing(h.getCoordinates())) - .toArray(LinearRing[]::new)); + Polygon shellPolygon = factory.createPolygon(shellRing); + for (LinearRing hole : interiorRings) { + Polygon holePolygon = factory.createPolygon(hole); + if (!shellPolygon.contains(holePolygon)) { + log.warn("Hole lies outside shell at or near point {}", hole.getCoordinate()); + } + } + return factory.createPolygon(shellRing, interiorRings); } } - return factory.createPolygon(factory.createLinearRing(shell.getCoordinates())); + return factory.createPolygon(shellRing); } catch (IllegalArgumentException e) { return null; } 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 e379e67d6d..b3c162e7b3 100644 --- a/common/src/test/java/org/apache/sedona/common/FunctionsTest.java +++ b/common/src/test/java/org/apache/sedona/common/FunctionsTest.java @@ -1603,6 +1603,46 @@ public class FunctionsTest extends TestBase { assertEquals(123, actual2.getSRID()); } + @Test + public void makePolygonWithValidHoles() { + Geometry shell = + GEOMETRY_FACTORY.createLineString(coordArray(0, 0, 10, 0, 10, 10, 0, 10, 0, 0)); + Geometry hole1 = GEOMETRY_FACTORY.createLineString(coordArray(2, 2, 4, 2, 4, 4, 2, 4, 2, 2)); + Geometry hole2 = GEOMETRY_FACTORY.createLineString(coordArray(6, 6, 8, 6, 8, 8, 6, 8, 6, 6)); + Geometry result = Functions.makePolygon(shell, new Geometry[] {hole1, hole2}); + assertNotNull(result); + assertEquals( + "POLYGON ((0 0, 10 0, 10 10, 0 10, 0 0), (2 2, 4 2, 4 4, 2 4, 2 2), (6 6, 8 6, 8 8, 6 8, 6 6))", + result.toText()); + } + + @Test + public void makePolygonWithHolesOutsideShell() { + Geometry shell = + GEOMETRY_FACTORY.createLineString(coordArray(0, 0, 10, 0, 10, 10, 0, 10, 0, 0)); + Geometry hole = + GEOMETRY_FACTORY.createLineString(coordArray(20, 20, 30, 20, 30, 30, 20, 30, 20, 20)); + Geometry result = Functions.makePolygon(shell, new Geometry[] {hole}); + // Matches PostGIS behavior: polygon is created but is invalid + assertNotNull(result); + assertFalse(result.isValid()); + assertEquals( + "POLYGON ((0 0, 10 0, 10 10, 0 10, 0 0), (20 20, 30 20, 30 30, 20 30, 20 20))", + result.toText()); + } + + @Test + public void makePolygonWithHolesPartiallyOutsideShell() { + Geometry shell = + GEOMETRY_FACTORY.createLineString(coordArray(0, 0, 10, 0, 10, 10, 0, 10, 0, 0)); + Geometry hole = + GEOMETRY_FACTORY.createLineString(coordArray(-1, -1, 5, -1, 5, 5, -1, 5, -1, -1)); + Geometry result = Functions.makePolygon(shell, new Geometry[] {hole}); + // Matches PostGIS behavior: polygon is created but is invalid + assertNotNull(result); + assertFalse(result.isValid()); + } + @Test public void distance_empty_geometries() throws ParseException { Point point = GEOMETRY_FACTORY.createPoint(new Coordinate(90, 0)); diff --git a/docs/api/flink/Function.md b/docs/api/flink/Function.md index d74552617f..7701a6d93b 100644 --- a/docs/api/flink/Function.md +++ b/docs/api/flink/Function.md @@ -2873,7 +2873,7 @@ Output: ## ST_MakePolygon -Introduction: Function to convert closed linestring to polygon including holes +Introduction: Function to convert closed linestring to polygon including holes. If holes are provided, they should be fully contained within the shell. Holes outside the shell will produce an invalid polygon (matching PostGIS behavior). Use `ST_IsValid` to check the result. Format: `ST_MakePolygon(geom: Geometry, holes: ARRAY[Geometry])` @@ -2883,15 +2883,15 @@ Example: ```sql SELECT ST_MakePolygon( - ST_GeomFromText('LINESTRING(7 -1, 7 6, 9 6, 9 1, 7 -1)'), - ARRAY(ST_GeomFromText('LINESTRING(6 2, 8 2, 8 1, 6 1, 6 2)')) + ST_GeomFromText('LINESTRING(0 0, 10 0, 10 10, 0 10, 0 0)'), + ARRAY(ST_GeomFromText('LINESTRING(2 2, 4 2, 4 4, 2 4, 2 2)')) ) ``` Output: ``` -POLYGON ((7 -1, 7 6, 9 6, 9 1, 7 -1), (6 2, 8 2, 8 1, 6 1, 6 2)) +POLYGON ((0 0, 10 0, 10 10, 0 10, 0 0), (2 2, 4 2, 4 4, 2 4, 2 2)) ``` ## ST_MakeValid diff --git a/docs/api/snowflake/vector-data/Function.md b/docs/api/snowflake/vector-data/Function.md index 28eb433c3b..1e0a6ce7d8 100644 --- a/docs/api/snowflake/vector-data/Function.md +++ b/docs/api/snowflake/vector-data/Function.md @@ -2063,7 +2063,7 @@ Output: ## ST_MakePolygon -Introduction: Function to convert closed linestring to polygon including holes. The holes must be a MultiLinestring. +Introduction: Function to convert closed linestring to polygon including holes. The holes must be a MultiLinestring. If holes are provided, they should be fully contained within the shell. Holes outside the shell will produce an invalid polygon (matching PostGIS behavior). Use `ST_IsValid` to check the result. Format: `ST_MakePolygon(geom: geometry, holes: <geometry>)` @@ -2074,19 +2074,19 @@ Query: ```sql SELECT ST_MakePolygon( - ST_GeomFromText('LINESTRING(7 -1, 7 6, 9 6, 9 1, 7 -1)'), - ST_GeomFromText('MultiLINESTRING((6 2, 8 2, 8 1, 6 1, 6 2))') + ST_GeomFromText('LINESTRING(0 0, 10 0, 10 10, 0 10, 0 0)'), + ST_GeomFromText('MultiLINESTRING((2 2, 4 2, 4 4, 2 4, 2 2))') ) AS polygon ``` Result: ``` -+----------------------------------------------------------------+ -|polygon | -+----------------------------------------------------------------+ -|POLYGON ((7 -1, 7 6, 9 6, 9 1, 7 -1), (6 2, 8 2, 8 1, 6 1, 6 2))| -+----------------------------------------------------------------+ ++--------------------------------------------------------------------+ +|polygon | ++--------------------------------------------------------------------+ +|POLYGON ((0 0, 10 0, 10 10, 0 10, 0 0), (2 2, 4 2, 4 4, 2 4, 2 2))| ++--------------------------------------------------------------------+ ``` diff --git a/docs/api/sql/Function.md b/docs/api/sql/Function.md index 9c61520f6d..3972ea3ffc 100644 --- a/docs/api/sql/Function.md +++ b/docs/api/sql/Function.md @@ -3071,7 +3071,7 @@ Output: ## ST_MakePolygon -Introduction: Function to convert closed linestring to polygon including holes +Introduction: Function to convert closed linestring to polygon including holes. If holes are provided, they should be fully contained within the shell. Holes outside the shell will produce an invalid polygon (matching PostGIS behavior). Use `ST_IsValid` to check the result. Format: `ST_MakePolygon(geom: Geometry, holes: ARRAY[Geometry])` @@ -3081,15 +3081,15 @@ SQL Example ```sql SELECT ST_MakePolygon( - ST_GeomFromText('LINESTRING(7 -1, 7 6, 9 6, 9 1, 7 -1)'), - ARRAY(ST_GeomFromText('LINESTRING(6 2, 8 2, 8 1, 6 1, 6 2)')) + ST_GeomFromText('LINESTRING(0 0, 10 0, 10 10, 0 10, 0 0)'), + ARRAY(ST_GeomFromText('LINESTRING(2 2, 4 2, 4 4, 2 4, 2 2)')) ) ``` Output: ``` -POLYGON ((7 -1, 7 6, 9 6, 9 1, 7 -1), (6 2, 8 2, 8 1, 6 1, 6 2)) +POLYGON ((0 0, 10 0, 10 10, 0 10, 0 0), (2 2, 4 2, 4 4, 2 4, 2 2)) ``` ## ST_MakeValid diff --git a/spark/common/src/test/scala/org/apache/sedona/sql/functions/StMakePolygonSpec.scala b/spark/common/src/test/scala/org/apache/sedona/sql/functions/StMakePolygonSpec.scala index df2687f494..02051e3e54 100644 --- a/spark/common/src/test/scala/org/apache/sedona/sql/functions/StMakePolygonSpec.scala +++ b/spark/common/src/test/scala/org/apache/sedona/sql/functions/StMakePolygonSpec.scala @@ -101,7 +101,7 @@ class StMakePolygonSpec .collect() .toList - Then("valid polygon with holes should be created") + Then("polygon with holes should be created (matching PostGIS behavior)") transformedGeometriesWithHoles should contain theSameElementsAs Seq( "POLYGON ((0 5, 1 7, 2 9, 2 5, 5 7, 4 6, 3 2, 1 3, 0 5), (2 3, 1 4, 2 4, 2 3), (2 4, 3 5, 3 4, 2 4))", "POLYGON ((7 -1, 7 6, 9 6, 9 1, 7 -1), (6 2, 8 2, 8 1, 6 1, 6 2))",
