This is an automated email from the ASF dual-hosted git repository.
jiayu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sedona.git
The following commit(s) were added to refs/heads/master by this push:
new 55bff92904 [GH-2148] fix(ST_MakePolygon) warn when holes lie outside
shell (#2613)
55bff92904 is described below
commit 55bff9290440c878150e0f0d718a91a3a93382ce
Author: Jia Yu <[email protected]>
AuthorDate: Sat Feb 7 12:12:30 2026 -0700
[GH-2148] fix(ST_MakePolygon) warn when holes lie outside shell (#2613)
---
.../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))",
