Skip to content

Commit b16d7f6

Browse files
committed
[GH-2857] Fix ST_S2CellIDs under-covering planar polygons along non-meridional edges
Sedona geometry-type objects are planar (edges are straight lines in (lng, lat) space) but S2 connects the same vertices with great-circle arcs. Along non-meridional edges the great-circle arc bulges poleward of the planar chord, so handing JTS vertices to S2 directly produces a spherical polygon whose interior is smaller than the planar polygon's. The S2 covering of that smaller polygon misses thin slivers of the original planar input — about 0.92% of the reporter's polygon at level 12. Compensate by JTS-buffering the input by an upper bound on the great-circle/chord deviation before converting to S2: - Polygon: per-edge midpoint deviation across exterior + interior rings (buffer offsets edges in place, so per-edge is tight). - LineString: bbox-corner pairs (SW-NE diagonal, NW-SE diagonal, worst-case east-west). The buffer turns the line into a polygon corridor whose long edges span the line's full envelope, which per-segment analysis under-bounds. Adds tests covering the reporter's polygon, lines, multi-geometries, geometry collections, polygons with holes, and high-latitude polygons. Updates SQL/Flink/Snowflake ST_S2CellIDs docs with a note explaining the planar-vs-spherical mismatch and the LineString corridor side effect. Closes #2857
1 parent ca1be45 commit b16d7f6

6 files changed

Lines changed: 376 additions & 4 deletions

File tree

common/src/main/java/org/apache/sedona/common/utils/S2Utils.java

Lines changed: 180 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -107,13 +107,189 @@ public static Polygon toJTSPolygon(S2CellId cellId) {
107107
return new GeometryFactory().createPolygon(coords);
108108
}
109109

110+
/**
111+
* Convert a JTS planar geometry into an S2Region whose lat/lng projection is guaranteed to
112+
* contain the input geometry.
113+
*
114+
* <p>Why a buffer is needed: Sedona geometries are planar — an edge between two vertices is a
115+
* straight line in (lng, lat) space — but S2 connects the same vertices with a great-circle arc
116+
* on the sphere. The two interpretations agree at the vertices but diverge along the edges (e.g.
117+
* the great-circle arc between two points at the same northern latitude bulges northward, leaving
118+
* the parallel that would form the planar chord). If we hand the JTS vertices to S2 directly, the
119+
* spherical polygon's interior is *smaller* than the planar polygon's interior along
120+
* non-meridional edges, so the S2 covering misses thin slivers of the original planar polygon
121+
* (see GH-2857).
122+
*
123+
* <p>To compensate, we JTS-buffer the input by an upper bound on the worst-case great-circle
124+
* deviation before converting to S2. A side effect for {@link LineString} inputs is that the
125+
* buffer turns the line into a polygon corridor; downstream callers therefore see cells in a thin
126+
* strip around the line rather than only cells the line geometrically traverses.
127+
*/
110128
public static S2Region toS2Region(Geometry geom) throws IllegalArgumentException {
111-
if (geom instanceof Polygon) {
112-
return S2Utils.toS2Polygon((Polygon) geom);
113-
} else if (geom instanceof LineString) {
114-
return S2Utils.toS2PolyLine((LineString) geom);
129+
if (!(geom instanceof Polygon) && !(geom instanceof LineString)) {
130+
throw new IllegalArgumentException(
131+
"only object of Polygon, LinearRing, LineString type can be converted to S2Region");
132+
}
133+
double eps = arcChordBufferDegrees(geom);
134+
Geometry buffered = (eps > 0) ? geom.buffer(eps) : geom;
135+
if (buffered instanceof Polygon) {
136+
return S2Utils.toS2Polygon((Polygon) buffered);
137+
} else if (buffered instanceof LineString) {
138+
// Only reachable when eps == 0 (e.g. a single-point degenerate line). Normal lines
139+
// become Polygon corridors after buffer and are handled above.
140+
return S2Utils.toS2PolyLine((LineString) buffered);
141+
} else if (buffered instanceof MultiPolygon && buffered.getNumGeometries() > 0) {
142+
// JTS buffer of self-touching geometries can collapse to MultiPolygon. We can only
143+
// hand a single S2Region back to callers, so cover the largest piece — the smaller
144+
// pieces are typically tiny artifacts of the buffer operation rather than real input.
145+
Polygon largest = (Polygon) buffered.getGeometryN(0);
146+
for (int i = 1; i < buffered.getNumGeometries(); i++) {
147+
Polygon p = (Polygon) buffered.getGeometryN(i);
148+
if (p.getArea() > largest.getArea()) {
149+
largest = p;
150+
}
151+
}
152+
return S2Utils.toS2Polygon(largest);
115153
}
116154
throw new IllegalArgumentException(
117155
"only object of Polygon, LinearRing, LineString type can be converted to S2Region");
118156
}
157+
158+
/**
159+
* Compute the JTS buffer amount (in degrees) needed so that the spherical interpretation of the
160+
* buffered geometry fully contains the original planar geometry.
161+
*
162+
* <p>The buffer must be at least as large as the largest great-circle/chord deviation among the
163+
* edges that S2 will eventually see. Polygons and lines need different bounds because JTS buffer
164+
* affects their edges differently:
165+
*
166+
* <ul>
167+
* <li><b>Polygon</b>: each existing edge is offset perpendicularly in place; corners get
168+
* rounded into many short arcs, but no edge is dramatically lengthened. The buffered
169+
* polygon's edges therefore have ~the same length as the originals, so the original
170+
* polygon's per-edge deviation is a tight upper bound on what the buffered polygon's edges
171+
* will exhibit. We use {@link #ringMaxDeviationDegrees}.
172+
* <li><b>LineString</b>: buffering produces a corridor whose long top/bottom edges span the
173+
* line's full envelope — far longer than any individual segment when consecutive segments
174+
* are collinear (JTS often simplifies them away). Per-segment deviation severely
175+
* under-bounds the corridor's actual edge deviation. We bound by virtual edges across the
176+
* envelope via {@link #envelopeDeviationDegrees}.
177+
* </ul>
178+
*
179+
* <p>The 1.5× safety multiplier absorbs numerical error and the small additional deviation the
180+
* buffered polygon's own (slightly different) edges introduce on top of the bound.
181+
*/
182+
private static double arcChordBufferDegrees(Geometry geom) {
183+
double maxDev = 0.0;
184+
if (geom instanceof Polygon) {
185+
Polygon poly = (Polygon) geom;
186+
maxDev = Math.max(maxDev, ringMaxDeviationDegrees(poly.getExteriorRing().getCoordinates()));
187+
for (int i = 0; i < poly.getNumInteriorRing(); i++) {
188+
maxDev =
189+
Math.max(maxDev, ringMaxDeviationDegrees(poly.getInteriorRingN(i).getCoordinates()));
190+
}
191+
} else if (geom instanceof LineString) {
192+
maxDev = envelopeDeviationDegrees(geom);
193+
}
194+
return maxDev * 1.5;
195+
}
196+
197+
/**
198+
* Conservative deviation upper bound for a geometry, derived from its bounding envelope rather
199+
* than its actual edges.
200+
*
201+
* <p>Used for {@link LineString} inputs because, after JTS buffer, the corridor's long edges are
202+
* not the line's segments — they connect the line's extreme endpoints (or close to it). To bound
203+
* them we probe three virtual edges across the envelope:
204+
*
205+
* <ul>
206+
* <li>The two diagonals (SW–NE and NW–SE) — diagonal great-circle arcs deviate more than
207+
* east-west arcs of the same Δλ at high latitudes, and a buffered corridor's long edges can
208+
* run in either direction depending on the line's orientation.
209+
* <li>The worst-case east-west edge at whichever latitude (top or bottom of the envelope) has
210+
* the larger absolute value — east-west arcs bulge poleward, so the deviation grows with
211+
* |latitude|, and an envelope-spanning east-west arc is what a horizontal collinear line
212+
* would buffer into.
213+
* </ul>
214+
*
215+
* <p>The max across these three bounds the deviation any corridor edge could plausibly exhibit.
216+
* This deliberately over-bounds zigzag lines whose actual corridor edges are short; the
217+
* alternative (per-segment analysis) silently fails on collinear inputs.
218+
*/
219+
private static double envelopeDeviationDegrees(Geometry geom) {
220+
Envelope env = geom.getEnvelopeInternal();
221+
if (env.isNull()) {
222+
return 0.0;
223+
}
224+
Coordinate sw = new Coordinate(env.getMinX(), env.getMinY());
225+
Coordinate se = new Coordinate(env.getMaxX(), env.getMinY());
226+
Coordinate ne = new Coordinate(env.getMaxX(), env.getMaxY());
227+
Coordinate nw = new Coordinate(env.getMinX(), env.getMaxY());
228+
// For the east-west probe, pick whichever latitude band of the envelope is further from
229+
// the equator — that's where same-Δλ great-circle arcs deviate most from the chord.
230+
double signedLat =
231+
Math.abs(env.getMinY()) > Math.abs(env.getMaxY()) ? env.getMinY() : env.getMaxY();
232+
Coordinate ewWest = new Coordinate(env.getMinX(), signedLat);
233+
Coordinate ewEast = new Coordinate(env.getMaxX(), signedLat);
234+
double max = edgeDeviationDegrees(sw, ne);
235+
max = Math.max(max, edgeDeviationDegrees(nw, se));
236+
max = Math.max(max, edgeDeviationDegrees(ewWest, ewEast));
237+
return max;
238+
}
239+
240+
/**
241+
* Per-edge deviation bound for a ring/path: walk consecutive vertex pairs and return the largest
242+
* single-edge great-circle/chord deviation. Used for polygon rings (exterior and interior), where
243+
* buffering preserves edge lengths and per-edge analysis is tight.
244+
*/
245+
private static double ringMaxDeviationDegrees(Coordinate[] coords) {
246+
double maxDev = 0.0;
247+
for (int i = 0; i < coords.length - 1; i++) {
248+
double dev = edgeDeviationDegrees(coords[i], coords[i + 1]);
249+
if (dev > maxDev) {
250+
maxDev = dev;
251+
}
252+
}
253+
return maxDev;
254+
}
255+
256+
/**
257+
* Primitive deviation for a single edge: the (lng, lat) distance between the planar chord
258+
* midpoint and the great-circle arc midpoint.
259+
*
260+
* <p>Why the midpoint: a great-circle arc between two points is symmetric about its midpoint, and
261+
* the (lng, lat) deviation from the chord is maximized there. So the midpoint deviation equals
262+
* the maximum deviation along the edge — there's no need to sample multiple points.
263+
*
264+
* <p>The great-circle midpoint is computed by averaging the two endpoint S2Points (unit vectors
265+
* on the sphere) and renormalizing — a standard spherical-midpoint trick. The chord midpoint is
266+
* the plain Euclidean mean of the (lng, lat) coordinates.
267+
*/
268+
private static double edgeDeviationDegrees(Coordinate a, Coordinate b) {
269+
S2Point aPt = toS2Point(a);
270+
S2Point bPt = toS2Point(b);
271+
double mx = aPt.getX() + bPt.getX();
272+
double my = aPt.getY() + bPt.getY();
273+
double mz = aPt.getZ() + bPt.getZ();
274+
double norm = Math.sqrt(mx * mx + my * my + mz * mz);
275+
if (norm < 1e-15) {
276+
// Antipodal endpoints — the great circle through them is not unique, so there is no
277+
// well-defined midpoint to compare against. Returning 0 effectively skips this edge;
278+
// antipodal inputs aren't realistic for S2 covering anyway.
279+
return 0.0;
280+
}
281+
S2LatLng midSpherical = new S2LatLng(new S2Point(mx / norm, my / norm, mz / norm));
282+
double midSphericalLat = midSpherical.latDegrees();
283+
double midSphericalLng = midSpherical.lngDegrees();
284+
double midChordLat = (a.y + b.y) / 2.0;
285+
double midChordLng = (a.x + b.x) / 2.0;
286+
double dLat = midSphericalLat - midChordLat;
287+
double dLng = midSphericalLng - midChordLng;
288+
// Wrap longitude difference into [-180, 180]. Without this, an edge straddling the
289+
// antimeridian (e.g. -179° to +179°) would compute dLng ≈ 358° and produce a bogus
290+
// ~360° deviation rather than the small actual deviation.
291+
if (dLng > 180.0) dLng -= 360.0;
292+
else if (dLng < -180.0) dLng += 360.0;
293+
return Math.sqrt(dLat * dLat + dLng * dLng);
294+
}
119295
}

common/src/test/java/org/apache/sedona/common/FunctionsTest.java

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1081,6 +1081,139 @@ public void testS2ToGeom() {
10811081
assertTrue(polygons[100].intersects(target));
10821082
}
10831083

1084+
@Test
1085+
public void testS2CoverageContainsInput() throws ParseException {
1086+
String wkt =
1087+
"POLYGON ((-102.060778 39.9999603, -102.0535384 40.0119065, -101.98532 40.0122906, "
1088+
+ "-95.30829 40.009008, -95.2456364 39.9564784, -95.1982467 39.9455019, "
1089+
+ "-95.1964657 39.9113444, -95.1460439 39.9142017, -95.1316877 39.8855881, "
1090+
+ "-95.087643 39.8717975, -95.0389987 39.8749063, -95.0146232 39.9088422, "
1091+
+ "-94.9403146 39.906409, -94.9183761 39.8846514, -94.9329504 39.8578468, "
1092+
+ "-94.8824331 39.8409102, -94.8675709 39.8227528, -94.878404 39.787242, "
1093+
+ "-94.9266292 39.7786779, -94.9070076 39.7679231, -94.8631596 39.779774, "
1094+
+ "-94.8497103 39.7604914, -94.8545514 39.7397163, -94.8985678 39.7150641, "
1095+
+ "-94.9584897 39.7331377, -94.9637588 39.6814526, -95.0187723 39.6615712, "
1096+
+ "-95.0456083 39.6252182, -95.0430365 39.5826542, -95.0650104 39.5677387, "
1097+
+ "-95.0985514 39.570063, -95.1012286 39.5462821, -95.0459187 39.5064755, "
1098+
+ "-95.0320969 39.4709074, -94.976457 39.4475392, -94.9362514 39.3964717, "
1099+
+ "-94.8781205 39.3949417, -94.8733156 39.3663291, -94.9014695 39.3495654, "
1100+
+ "-94.8963639 39.3135051, -94.8237994 39.2609956, -94.8194328 39.2178517, "
1101+
+ "-94.7789019 39.214907, -94.7398645 39.1789812, -94.6785238 39.1931279, "
1102+
+ "-94.6533801 39.1816662, -94.6517728 39.1640754, -94.605515 39.1696807, "
1103+
+ "-94.5791896 39.1504025, -94.5983384 39.1134256, -94.6095667 36.9948123, "
1104+
+ "-102.045765 36.9847897, -102.060778 39.9999603))";
1105+
Geometry input = geomFromWKT(wkt, 0);
1106+
Long[] cellIds = Functions.s2CellIDs(input, 12);
1107+
Geometry[] polygons =
1108+
Functions.s2ToGeom(Arrays.stream(cellIds).mapToLong(Long::longValue).toArray());
1109+
Geometry coverage = Functions.union(polygons);
1110+
Geometry uncovered = input.difference(coverage);
1111+
double uncoveredArea = uncovered.getArea();
1112+
log.info(
1113+
"S2 cells: {}, input area: {}, uncovered area: {} ({} %)",
1114+
cellIds.length, input.getArea(), uncoveredArea, (uncoveredArea / input.getArea()) * 100.0);
1115+
assertTrue(
1116+
String.format(
1117+
"Coverage does not contain input. Missing %.8f deg^2 (%.6f%%)",
1118+
uncoveredArea, (uncoveredArea / input.getArea()) * 100.0),
1119+
coverage.covers(input));
1120+
}
1121+
1122+
@Test
1123+
public void testS2CoverageContainsLineString() throws ParseException {
1124+
// Long east-west line at mid-latitude. The great-circle arc bulges poleward of the JTS
1125+
// chord, so before the buffer fix the cells along the parallel were missed in the middle.
1126+
Geometry line = geomFromWKT("LINESTRING (-102 37, -94 37)", 0);
1127+
Long[] cellIds = Functions.s2CellIDs(line, 12);
1128+
Geometry[] cells =
1129+
Functions.s2ToGeom(Arrays.stream(cellIds).mapToLong(Long::longValue).toArray());
1130+
Geometry coverage = Functions.union(cells);
1131+
assertTrue(
1132+
"S2 cell coverage of LineString does not contain the line itself.", coverage.covers(line));
1133+
}
1134+
1135+
@Test
1136+
public void testS2CoverageContainsMultiPolygon() throws ParseException {
1137+
// Two disjoint polygons, both at mid-northern latitude with long east-west edges.
1138+
String wkt =
1139+
"MULTIPOLYGON ("
1140+
+ "((-102 37, -94 37, -94 40, -102 40, -102 37)),"
1141+
+ "((-90 50, -80 50, -80 53, -90 53, -90 50)))";
1142+
Geometry input = geomFromWKT(wkt, 0);
1143+
Long[] cellIds = Functions.s2CellIDs(input, 10);
1144+
Geometry[] cells =
1145+
Functions.s2ToGeom(Arrays.stream(cellIds).mapToLong(Long::longValue).toArray());
1146+
Geometry coverage = Functions.union(cells);
1147+
assertTrue(
1148+
"S2 cell coverage does not contain every member of the MultiPolygon.",
1149+
coverage.covers(input));
1150+
}
1151+
1152+
@Test
1153+
public void testS2CoverageContainsMultiLineString() throws ParseException {
1154+
// Three disjoint multi-segment lines: northern hemisphere, southern hemisphere, and a
1155+
// diagonal climb. Each is decomposed and buffered independently before S2 covering.
1156+
String wkt =
1157+
"MULTILINESTRING ("
1158+
+ "(-102 37, -98 37, -94 37),"
1159+
+ "(-90 -42, -85 -42, -80 -42),"
1160+
+ "(-100 50, -95 55, -90 60))";
1161+
Geometry input = geomFromWKT(wkt, 0);
1162+
Long[] cellIds = Functions.s2CellIDs(input, 10);
1163+
Geometry[] cells =
1164+
Functions.s2ToGeom(Arrays.stream(cellIds).mapToLong(Long::longValue).toArray());
1165+
Geometry coverage = Functions.union(cells);
1166+
assertTrue(
1167+
"S2 cell coverage does not contain every member of the MultiLineString.",
1168+
coverage.covers(input));
1169+
}
1170+
1171+
@Test
1172+
public void testS2CoverageContainsGeometryCollection() throws ParseException {
1173+
String wkt =
1174+
"GEOMETRYCOLLECTION ("
1175+
+ "POLYGON ((-102 37, -94 37, -94 40, -102 40, -102 37)),"
1176+
+ "LINESTRING (10 60, 20 60, 30 60))";
1177+
Geometry input = geomFromWKT(wkt, 0);
1178+
Long[] cellIds = Functions.s2CellIDs(input, 10);
1179+
Geometry[] cells =
1180+
Functions.s2ToGeom(Arrays.stream(cellIds).mapToLong(Long::longValue).toArray());
1181+
Geometry coverage = Functions.union(cells);
1182+
assertTrue(
1183+
"S2 cell coverage does not contain every member of the GeometryCollection.",
1184+
coverage.covers(input));
1185+
}
1186+
1187+
@Test
1188+
public void testS2CoverageContainsPolygonWithHole() throws ParseException {
1189+
// Outer ring at mid-latitude with long east-west edges; an inner hole. Both rings need
1190+
// their great-circle bulge accounted for so the buffer applies to interior rings too.
1191+
String wkt =
1192+
"POLYGON ("
1193+
+ "(-102 37, -94 37, -94 40, -102 40, -102 37),"
1194+
+ "(-100 38, -96 38, -96 39, -100 39, -100 38))";
1195+
Geometry input = geomFromWKT(wkt, 0);
1196+
Long[] cellIds = Functions.s2CellIDs(input, 12);
1197+
Geometry[] cells =
1198+
Functions.s2ToGeom(Arrays.stream(cellIds).mapToLong(Long::longValue).toArray());
1199+
Geometry coverage = Functions.union(cells);
1200+
assertTrue("S2 cell coverage does not contain a polygon with a hole.", coverage.covers(input));
1201+
}
1202+
1203+
@Test
1204+
public void testS2CoverageContainsHighLatitudePolygon() throws ParseException {
1205+
// Polygon near 80°N: tan(φ) is large, so the buffer cap (1°) and 89° latitude clamp need
1206+
// to keep the arc-chord bound from blowing up.
1207+
String wkt = "POLYGON ((-30 80, 30 80, 30 82, -30 82, -30 80))";
1208+
Geometry input = geomFromWKT(wkt, 0);
1209+
Long[] cellIds = Functions.s2CellIDs(input, 8);
1210+
Geometry[] cells =
1211+
Functions.s2ToGeom(Arrays.stream(cellIds).mapToLong(Long::longValue).toArray());
1212+
Geometry coverage = Functions.union(cells);
1213+
assertTrue(
1214+
"S2 cell coverage does not contain a high-latitude polygon.", coverage.covers(input));
1215+
}
1216+
10841217
@Test
10851218
public void testUnion() throws ParseException {
10861219
long[] cellIds =

docs/api/flink/Spatial-Indexing/ST_S2CellIDs.md

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,27 @@ Return type: `Array<Long>`
2929

3030
Since: `v1.4.0`
3131

32+
!!! note "Planar input, spherical cells"
33+
Sedona geometry type objects are planar: an edge between two vertices is a straight line in
34+
`(longitude, latitude)` space. S2 cells are spherical: an edge between two vertices is a
35+
great-circle arc on the unit sphere. The two interpretations agree at the vertices but not
36+
along the edges — for example, a great-circle arc connecting two points at the same
37+
non-equatorial latitude bulges *toward the nearer pole* rather than following the parallel.
38+
39+
Without compensation this would let the returned cells under-cover the original planar
40+
geometry along long, non-meridional edges (the bug reported in
41+
[GH-2857](https://github.com/apache/sedona/issues/2857)). To prevent that, `ST_S2CellIDs`
42+
JTS-buffers the input by an upper bound on the great-circle/chord deviation before
43+
converting it to an S2 region. The result is a covering that always *contains* the
44+
original planar geometry, at the cost of a small number of extra boundary cells. Inputs
45+
that are themselves spherical (e.g. polygons whose edges are explicitly meridians or the
46+
equator) see no additional cells.
47+
48+
For `LineString` and `MultiLineString` inputs the buffer turns the line into a polygon
49+
corridor, so the returned cells cover a thin strip *around* the line rather than only
50+
cells the line geometrically passes through. Use a sufficiently fine `level` to keep the
51+
corridor narrow.
52+
3253
Example:
3354

3455
```sql

0 commit comments

Comments
 (0)