You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@sedona.apache.org by "CaptainAni187 (via GitHub)" <gi...@apache.org> on 2026/10/06 00:11:59 UTC
[PR] fix(rust/sedona-geo-generic-alg): skip empty members of a collection in distance [sedona-db]
CaptainAni187 opened a new pull request, #1408:
URL: https://github.com/apache/sedona-db/pull/1408
Closes #1374
The Euclidean distance kernels in `sedona-geo-generic-alg` take the minimum distance over the members of a geometry collection. An empty member such as `POINT EMPTY` gives distance zero, so it always won: `ST_Distance(POINT (0 0), GEOMETRYCOLLECTION (POINT EMPTY, POINT (10 0)))` returned `0`, and `ST_DWithin(..., 1)` returned `true`.
Empty members are now skipped when taking the minimum. This applies in the four places that loop over collection members: collection to single geometry, collection to collection, and the two `Geometry` paths that handle a collection. If every member is empty, the result stays zero, as for other empty operands. Wholly empty operands are still turned into NULL by the callers, as after #1369.
Tests in `rust/sedona-geo/src/st_distance.rs`:
- `nonempty_collection_with_empty_point` is the test from the issue: distance `10` and `ST_DWithin(..., 1) == false`.
- `collection_with_empty_members` covers the same case with the operands reversed, collection to collection, empty `LINESTRING` / `POLYGON` members, and a nested collection that only holds an empty point.
All six fail on main. `cargo test` for `sedona-geo-generic-alg`, `sedona-geo` and `sedona-spatial-join`, `cargo clippy -- -D warnings` and `cargo fmt --check` pass locally.
--
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: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(rust/sedona-geo-generic-alg): skip empty members of a collection in distance [sedona-db]
Posted by "jiayuasu (via GitHub)" <gi...@apache.org>.
jiayuasu commented on PR #1408:
URL: https://github.com/apache/sedona-db/pull/1408#issuecomment-6044726125
lgtm. please fix the failed test
--
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: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(rust/sedona-geo-generic-alg): skip empty members of a collection in distance [sedona-db]
Posted by "CaptainAni187 (via GitHub)" <gi...@apache.org>.
CaptainAni187 commented on code in PR #1408:
URL: https://github.com/apache/sedona-db/pull/1408#discussion_r4194432262
##########
rust/sedona-geo-generic-alg/src/algorithm/line_measures/metric_spaces/euclidean/distance.rs:
##########
@@ -778,6 +778,18 @@ impl_cross_type_array!(symmetric_single_to_multi: TriangleTraitExt, TriangleTag
(MultiPolygonTraitExt, MultiPolygonTag)
]);
+/// The minimum distance over the non-empty members of a collection. Empty members, such
+/// as the `POINT EMPTY` in `GEOMETRYCOLLECTION (POINT EMPTY, POINT (10 0))`, are skipped
+/// because they have no distance to anything. If every member is empty, the distance is
+/// zero, as for other empty operands.
+fn non_empty_min_distance<F: GeoFloat>(min_distance: F) -> F {
+ if min_distance == <F as Bounded>::max_value() {
Review Comment:
Good catch, thanks. Fixed in this PR rather than a follow-up: the minimum is now tracked as an `Option`, so `f64::MAX` stays a real distance. `collection_member_at_the_largest_distance` covers your example (distance `f64::MAX`, `ST_DWithin(..., 1)` false) and fails on the previous commit.
--
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: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(rust/sedona-geo-generic-alg): skip empty members of a collection in distance [sedona-db]
Posted by "jiayuasu (via GitHub)" <gi...@apache.org>.
jiayuasu commented on code in PR #1408:
URL: https://github.com/apache/sedona-db/pull/1408#discussion_r4203467855
##########
rust/sedona-geo-generic-alg/src/algorithm/line_measures/metric_spaces/euclidean/distance.rs:
##########
@@ -918,18 +928,21 @@ macro_rules! impl_distance_geometry_to_type {
{
fn generic_distance_trait(&self, rhs: &RHS) -> F {
if self.is_collection() {
- let mut min_distance = <F as Bounded>::max_value();
+ let mut min_distance = None;
for lhs_geom in self.geometries_ext() {
let lhs_geom = lhs_geom.borrow();
+ if lhs_geom.is_empty() {
Review Comment:
Nonblocking perf note: this `is_empty()` walks the nested collection, and `generic_distance_trait()` immediately walks it again. To reproduce, wrap `Point(10, 0)` in 100 one-member `GeometryCollection`s and measure `distance_ext` to `Point(0, 0)` in a release build. My local microbenchmark went from 0.72 µs on the base to 33 µs here (both return 10); the WKB path went from 1.6 to 35 µs. Could the internal recursive distance path return `Option<F>` (`None` for an all-empty subtree), so the min and emptiness are handled in one walk? I'm fine with doing this here or in a follow-up PR.
--
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: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(rust/sedona-geo-generic-alg): skip empty members of a collection in distance [sedona-db]
Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot merged PR #1408:
URL: https://github.com/apache/sedona-db/pull/1408
--
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: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(rust/sedona-geo-generic-alg): skip empty members of a collection in distance [sedona-db]
Posted by "jiayuasu (via GitHub)" <gi...@apache.org>.
jiayuasu commented on code in PR #1408:
URL: https://github.com/apache/sedona-db/pull/1408#discussion_r4191731052
##########
rust/sedona-geo-generic-alg/src/algorithm/line_measures/metric_spaces/euclidean/distance.rs:
##########
@@ -778,6 +778,18 @@ impl_cross_type_array!(symmetric_single_to_multi: TriangleTraitExt, TriangleTag
(MultiPolygonTraitExt, MultiPolygonTag)
]);
+/// The minimum distance over the non-empty members of a collection. Empty members, such
+/// as the `POINT EMPTY` in `GEOMETRYCOLLECTION (POINT EMPTY, POINT (10 0))`, are skipped
+/// because they have no distance to anything. If every member is empty, the distance is
+/// zero, as for other empty operands.
+fn non_empty_min_distance<F: GeoFloat>(min_distance: F) -> F {
+ if min_distance == <F as Bounded>::max_value() {
Review Comment:
Nonblocking nit for an extreme finite coordinate: `f64::MAX` is also a real distance, so this check turns it into zero. In a local diagnostic, `POINT (0 0)` against `GEOMETRYCOLLECTION (POINT (1.7976931348623157e308 0))` returned distance 0, and `ST_DWithin` with bound 1 returned true; the direct point-to-point distance is `f64::MAX`. Could we track whether the collection had a non-empty member separately (for example with `Option<F>`) in a follow-up?
--
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: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(rust/sedona-geo-generic-alg): skip empty members of a collection in distance [sedona-db]
Posted by "CaptainAni187 (via GitHub)" <gi...@apache.org>.
CaptainAni187 commented on code in PR #1408:
URL: https://github.com/apache/sedona-db/pull/1408#discussion_r4204809849
##########
rust/sedona-geo-generic-alg/src/algorithm/line_measures/metric_spaces/euclidean/distance.rs:
##########
@@ -918,18 +928,21 @@ macro_rules! impl_distance_geometry_to_type {
{
fn generic_distance_trait(&self, rhs: &RHS) -> F {
if self.is_collection() {
- let mut min_distance = <F as Bounded>::max_value();
+ let mut min_distance = None;
for lhs_geom in self.geometries_ext() {
let lhs_geom = lhs_geom.borrow();
+ if lhs_geom.is_empty() {
Review Comment:
Thanks, done here in 5c5f7adfd. The collection paths now call `non_empty_geometry_distance()`, which returns `None` for an empty subtree. So emptiness and the minimum come from one walk, and members are no longer checked with `is_empty()`. With your case (`Point(10, 0)` in 100 one-member `GeometryCollection`s, `distance_ext` to `Point(0, 0)`, release build, geo-types path), I measured 34.7 µs before this commit and 1.03 µs after, against 1.00 µs on main. All three return 10.
--
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: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(rust/sedona-geo-generic-alg): skip empty members of a collection in distance [sedona-db]
Posted by "CaptainAni187 (via GitHub)" <gi...@apache.org>.
CaptainAni187 commented on PR #1408:
URL: https://github.com/apache/sedona-db/pull/1408#issuecomment-6031693523
@paleolimbot Added in 224b8748f: `test_st_distance` in `python/sedonadb/tests/functions/test_distance.py` now has `POINT (0 0)` against `GEOMETRYCOLLECTION (POINT EMPTY, POINT (10 0))` in both argument orders, expecting `10`. Locally the SedonaDB cases fail without the kernel change and pass with it. The PostGIS cases are skipped here because I have no PostGIS runtime.
--
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: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(rust/sedona-geo-generic-alg): skip empty members of a collection in distance [sedona-db]
Posted by "CaptainAni187 (via GitHub)" <gi...@apache.org>.
CaptainAni187 commented on PR #1408:
URL: https://github.com/apache/sedona-db/pull/1408#issuecomment-6058603205
@jiayuasu Thanks. The failing test is `test_sjoin_dwithin_distances_match_geopandas[-1.0]` in `python/sedonadb-geopandas`. It isn't from this PR: it compares two plain LINESTRINGs, and it fails the same way on other PRs since yesterday (for example the runs on `jw/gdal-loader-concurrency` and `feat/native-flatgeobuf-reader`). The passing run on main installed Shapely 2.1.2 and the failing runs install Shapely 2.2.0. With 2.2.0, geopandas' dwithin join at a negative distance matches the intersecting `LINESTRING (0 0, 2 2)` / `LINESTRING (0 2, 2 0)` pair, while SedonaDB returns no pairs. The distance-kernel tests for this PR pass. Should I open a separate issue for the Shapely 2.2 difference, or is someone already on it?
--
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: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org