Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix stack overflow when converting GeometryCollection scalar to geo #984

Merged
merged 3 commits into from
Jan 31, 2025

Conversation

kylebarron
Copy link
Member

@kylebarron kylebarron commented Jan 29, 2025

As described in a contained comment:

We can't implement both

impl From<GeometryCollection<'_>> for geo::GeometryCollection

and

impl From<GeometryCollection<'_>> for geo::Geometry

because of this problematic blanket impl
(https://github.com/georust/geo/blob/ef55eabe9029b27f753d4c40db9f656e3670202e/geo-types/src/geometry/geometry_collection.rs#L113-L120).

Thus we need to choose either one or the other to implement.

If we implemented only for geo::Geometry, then the default blanket impl for
geo::GeometryCollection would be wrong because it would doubly-nest the
GeometryCollection:

GeometryCollection(
    [
        GeometryCollection(
            GeometryCollection(
                [
                    Point(
                        Point(
                            Coord {
                                x: 0.0,
                                y: 0.0,
                            },
                        ),
                    ),
                ],
            ),
        ),
    ],
)

Therefore we must implement only for geo::GeometryCollection

Closes #979

@kylebarron
Copy link
Member Author

It looks like a new version of clippy was released. I'll put up a PR to fix those new lints once I'm off this plane or tomorrow morning.

Copy link

@BlakeOrth BlakeOrth left a comment

Choose a reason for hiding this comment

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

lgtm

Thanks for the quick turn around on the bug report!

@kylebarron kylebarron mentioned this pull request Jan 31, 2025
@kylebarron kylebarron merged commit 58db3cc into main Jan 31, 2025
21 checks passed
@kylebarron kylebarron deleted the kyle/fix-issue-979 branch January 31, 2025 20:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Using bounding_rect() on a GeometryCollectionArray causes a stack overflow
2 participants