Skip to content

Redesign GeometryTraitExt to eliminate GAT recursion to work around a Rust compiler bug in 1.90.0#11

Merged
Kontinuation merged 6 commits intogeneric-algfrom
geo-trait-ext-no-recursion
Sep 25, 2025
Merged

Redesign GeometryTraitExt to eliminate GAT recursion to work around a Rust compiler bug in 1.90.0#11
Kontinuation merged 6 commits intogeneric-algfrom
geo-trait-ext-no-recursion

Conversation

@Kontinuation
Copy link
Copy Markdown

This patch removes the GAT GeometryCollectionTypeExt from GeometryTraitExt because it would introduce recursive GATs such as G::GeometryCollectionTypeExt::GeometryTypeExt::GeometryCollectionTypeExt::... and easily
trigger a Rust compiler bug: rust-lang/rust#128887 and rust-lang/rust#131960. See also geoarrow/geoarrow-rs#1339.

Although this could be worked around by not implementing generic functions using trait-based approach and use
function-based approach instead, see geoarrow/geoarrow-rs#956 and georust/wkb#77, we are not certain if there will be other issues caused by recursive GATs in the future. So we decided to completely get rid of recursive GATs.

@Kontinuation
Copy link
Copy Markdown
Author

This is a breaking change, wherobots/wkb needs to be updated as well. We need to merge PRs for both repos at once.

@jiayuasu
Copy link
Copy Markdown
Member

@zhangfengcdt can you review?

Copy link
Copy Markdown
Member

@zhangfengcdt zhangfengcdt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@zhangfengcdt
Copy link
Copy Markdown
Member

I think we might consider to add some new macros to simplify some of the repeated patterns with macro, such as

if self.is_collection() { ... } else { match self.as_type_ext() { ... } }

But we can do that later in one PR so we don't mix it with the fix here.

@Kontinuation Kontinuation merged commit 66ff859 into generic-alg Sep 25, 2025
44 checks passed
Kontinuation added a commit to wherobots/wkb that referenced this pull request Sep 25, 2025
The change made to GeometryTraitExt is introduced by wherobots/geo#11. This PR should be merged after the wherobots/geo PR is merged.
Kontinuation added a commit to apache/sedona-db that referenced this pull request Sep 25, 2025
We've worked around the Rust compiler 1.90.0 regression in its forked dependency wherobots/geo: wherobots/geo#11. Please refer to the description of that PR for details.

Once we resolve the compilation issue, we'll start working on moving geo-traits-ext and geo-generic-alg modules into the sedona-db repository, and get rid of our own geo, wkb, and wkt fork. This will unblock us from publishing our sedona-db crate to crates.io, and make the project grow in a healthier state.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants