jiayuasu opened a new pull request, #1143: URL: https://github.com/apache/sedona-db/pull/1143
## Description Fixes #1085. One polygon in the Overture divisions dataset came out inverted when read as a geography: `ST_Area` returned a negative value and `ST_Within`/`ST_Intersects` matched points anywhere on Earth, silently corrupting every spatial join against the table. ### Root cause Every geography UDF materializes WKB through `GeoArrowGeography::Init`, which calls `GeoArrowLaxPolygonShape::NormalizeOrientation()` to make shells counterclockwise and holes clockwise. That function derived each ring's winding direction from the sign of its curvature (turning angle). The affected ring is digitized with an out-and-back tail whose return path is offset by ~10 m; interpreted as geodesics, the return path crosses the outgoing path once. The crossing collapses the ring's turning number to zero, making the curvature sign meaningless, so the shell was reversed in place and its interior inverted to cover nearly the entire sphere. (This also explains why `ST_Reverse`/`ST_Normalize` had no effect: orientation is re-derived, and re-broken, on every evaluation.) ### Fix paleolimbot/s2geography#125 changes `NormalizeOrientation()` to use the sign of the ring's signed area instead. The signed area agrees with the curvature sign for every valid ring (S2 computes it from the surface integral, with a fallback to the curvature sign when the magnitude is within numerical error), and additionally reflects the net winding direction for slightly invalid rings. This PR bumps the s2geography submodule to that fix and adds a Python regression test with a minimal self-crossing sliver ring. **Blocked on paleolimbot/s2geography#125** — the submodule currently points at the commit from the PR branch, which does not exist in the submodule's upstream remote yet. I will re-point the submodule to the merged commit once it lands, which should let submodule checkout and CI pass. ### Verification Against the original 258-point ring from #1085: | Expression | Before | After | |---|---|---| | `ST_Area(geog)` | −11 876 076 760.37 | +11 876 076 760.37 | | `ST_Perimeter(geog)` | 574 613.63 | 574 613.63 (unchanged) | | `ST_Within(POINT (-150 0), geog)` | true | false | | `ST_Within(POINT (33.5 16.5), geog)` | — | true | - s2geography C++ suite: 946/946 (new regression test fails without the fix, passes with it) - `cargo test -p sedona-s2geography`: 68/68 - `pytest tests/geography/`: 653 passed, 1 pre-existing local-PostGIS environment failure unrelated to this change -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
