jiayuasu opened a new pull request, #1164:
URL: https://github.com/apache/sedona-db/pull/1164

   Fixes #1156.
   
   ## Summary
   
   - route each LineString segment pair through the existing intersection-aware 
distance helper
   - return immediately when the distance reaches zero
   - add regressions at the generic algorithm, Rust UDF, and SedonaDB/PostGIS 
conformance layers
   
   ## Root cause
   
   The native `geo` kernel routes non-Point distance calculations through 
`sedona-geo-generic-alg::DistanceExt`. Its specialized LineString-to-LineString 
branch visited every segment pair, but calculated only the four 
endpoint-to-opposite-segment distances.
   
   That formula is valid only after ruling out a segment intersection. For the 
two crossing lines from #1156, none of the endpoints lies on the other segment, 
so all four projections have a positive distance even though the segment 
interiors cross. The result was `sqrt(2)` instead of zero.
   
   The same crate already had the correct `distance_line_to_line_generic()` 
primitive: it tests segment intersection first and then, only for disjoint 
segments, takes the same four endpoint distances. The public LineString 
dispatch duplicated the endpoint portion and bypassed that helper.
   
   This affected more than the direct `ST_Distance` example:
   
   - `ST_DWithin` shares the same distance backend, so a zero bound was also 
wrong.
   - MultiLineString and GeometryCollection distance calculations delegate to 
the same LineString branch.
   - Callers of the generic distance trait, including native distance 
refinement paths, inherited the same result.
   
   The concrete upstream `geo` LineString implementation does not have this 
defect; it checks for intersection before searching for a positive minimum 
distance. The bug was specific to the generic adaptation in this repository.
   
   ## History
   
   The generic distance work originated in 
[`wherobots/geo#7`](https://github.com/wherobots/geo/pull/7) and was first 
consumed by SedonaDB in 
[`apache/sedona-db#73`](https://github.com/apache/sedona-db/pull/73). A broader 
generic rewrite in [`wherobots/geo#9`](https://github.com/wherobots/geo/pull/9) 
introduced the endpoint-only LineString specialization. That source was 
imported into this repository by 
[`apache/sedona-db#195`](https://github.com/apache/sedona-db/pull/195) in 
commit 
[`91f1b7e`](https://github.com/apache/sedona-db/commit/91f1b7e70d656ab136ea6037488f190b0c88b014),
 then integrated into the workspace by 
[`apache/sedona-db#203`](https://github.com/apache/sedona-db/pull/203).
   
   The existing tests did exercise crossing segments, but only through the 
correct helper. The randomized LineString cross-validation also compared 
upstream `geo` with `nearest_neighbour_distance()` rather than the public 
`DistanceExt` LineString dispatch. Existing benchmarks used disjoint 
LineStrings. Consequently, all of those checks bypassed the duplicated faulty 
branch.
   
   ## Fix
   
   The LineString specialization now calls `distance_line_to_line_generic()` 
inside its existing segment-pair traversal and stops as soon as any pair 
returns zero. This keeps intersection detection and positive-distance 
calculation in one traversal: disjoint inputs are not subjected to a separate 
whole-LineString intersection pass followed by a second distance pass. 
Complexity remains O(n*m) with O(1) additional memory, and the existing 
empty/no-segment behavior is preserved.
   
   This is intentionally a localized correction to `geo-generic-alg`; it does 
not switch `ST_Distance` or `ST_DWithin` to another backend.
   
   ## Validation
   
   - `cargo test --offline -p sedona-geo-generic-alg` (323 tests and 30 
doctests passed)
   - `cargo test --offline -p sedona-geo` (159 tests passed)
   - `cargo clippy --offline -p sedona-geo-generic-alg --all-targets --no-deps 
-- -D warnings`
   - `cargo clippy --offline -p sedona-geo --all-targets --no-deps -- -D 
warnings`
   - targeted Python `ST_Distance` and `ST_DWithin` conformance tests (18 
passed; 18 PostGIS parameterizations skipped because PostGIS was unavailable 
locally)
   - `ruff format --check` and `ruff check` for the modified Python tests
   
   The new coverage includes both operand orders, a crossing that occurs on a 
later segment, MultiLineString and GeometryCollection delegation, all supported 
WKB/WKB-view/item-CRS UDF type pairs, and `ST_DWithin(..., 0)`.
   


-- 
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]

Reply via email to