Skip to content

fix(c/sedona-tg): handle collection point containment boundaries - #1042

Merged
paleolimbot merged 1 commit into
apache:mainfrom
ajaypadwal73:ap/fix-geometrycollection-contains
Jul 10, 2026
Merged

fix(c/sedona-tg): handle collection point containment boundaries#1042
paleolimbot merged 1 commit into
apache:mainfrom
ajaypadwal73:ap/fix-geometrycollection-contains

Conversation

@ajaypadwal73

@ajaypadwal73 ajaypadwal73 commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Collection containment in tg treated a target geometry as contained if any flattened child contained it. This made ST_Contains(GEOMETRYCOLLECTION (POINT (0 0), LINESTRING (0 0, 0 1)), POINT (0 0)) return true via the point child even though GEOS/PostGIS classify that point on the collection boundary.

Require collection contains to satisfy covers first, then find an interior intersection against flattened point, line, or polygon targets. This preserves covers/coveredby behavior while making contains/within respect point components shadowed by higher-dimensional boundaries and line targets covered by polygon boundaries.

Fixes #1035.

Tests: cargo test -p sedona-tg; cargo test -p sedona-tg predicates -- --nocapture; cargo fmt --check --package sedona-tg; git diff --check.

@github-actions
github-actions Bot requested a review from zhangfengcdt July 8, 2026 23:58

@paleolimbot paleolimbot left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you for this!

There are a few test cases I want to confirm this covers...I believe the weirdness here is because the interior and boundary as far as DE-9IM is concerned is the union of the boundaries (where the highest dimension wins, so for a collection that has points and lines, only the lines count as an interior).

Because of that I'm not sure the fix here handles all cases. You may have to do something like check "covers", which is hopefully already done, then if true, only iterate over the higest dimension shapes (i.e., only the lines or only the polygons) and check that there's at least one point of it on the element's "interior".

Comment on lines +48 to +52
(
"GEOMETRYCOLLECTION (POINT (0 0), LINESTRING (0 0, 0 1))",
"POINT (0 0)",
False,
),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you add some more related cases here to make sure we correctly figured this one out? I am not sure about all of these but we should probably match PostGIS here. When confirmed, these can go in Rust as well.

Suggested change
(
"GEOMETRYCOLLECTION (POINT (0 0), LINESTRING (0 0, 0 1))",
"POINT (0 0)",
False,
),
(
"GEOMETRYCOLLECTION (POINT (0 0))",
"POINT (0 0)",
True,
),
(
"GEOMETRYCOLLECTION (POINT (0 0), LINESTRING (0 0, 0 1))",
"POINT (0 0)",
False,
),
# True because the point is no longer on the boundary
(
"GEOMETRYCOLLECTION (POINT (0 0), LINESTRING (0 0, 0 1))",
"POINT (0 0.5)",
True,
),
# True because the point is no longer on the boundary
(
"GEOMETRYCOLLECTION (POINT (-1 -1), LINESTRING (0 0, 0 1))",
"POINT (-1 -1)",
True,
),
(
"GEOMETRYCOLLECTION (POINT (0 0), POLYGON ((0 0, 0 1, 1 0, 0 0)))",
"POINT (0 0)",
False,
),
(
"GEOMETRYCOLLECTION (POINT (0 0), POLYGON ((0 0, 0 1, 1 0, 0 0)))",
"POINT (0.25 0.25)",
True,
),
(
"GEOMETRYCOLLECTION (LINESTRING (0 0, 0 1), POLYGON ((0 0, 0 1, 1 0, 0 0)))",
"LINESTRING (0 0, 0 1)",
False,
),
(
"GEOMETRYCOLLECTION (LINESTRING (0 0, 0 1), POLYGON ((0 0, 0 1, 1 0, 0 0)))",
"LINESTRING (0 0, 0.25 0.25)",
True,
),

Collection containment previously delegated each target geometry to any child that contained it, so GEOMETRYCOLLECTION (POINT (0 0), LINESTRING (0 0, 0 1)) incorrectly contained POINT (0 0) via the point child even though the point lies on the collection boundary.

Require collection contains to satisfy covers and then find an interior intersection against flattened point, line, or polygon targets. This matches GEOS/PostGIS behavior for point components shadowed by higher-dimensional boundaries and for line targets covered by polygon boundaries.

Add low-level tg predicate regressions and SQL predicate expectations for the related collection cases.
@ajaypadwal73
ajaypadwal73 force-pushed the ap/fix-geometrycollection-contains branch from bbd99d4 to 2bd55ae Compare July 9, 2026 19:51
@ajaypadwal73

Copy link
Copy Markdown
Contributor Author

Thanks! I reworked this to follow the covers + interior-intersection shape you suggested instead of the earlier point-only special case.

The updated path now requires collection contains to pass covers first, then checks for an interior hit against flattened point, line, and polygon targets. I also added the related PostGIS/GEOS cases you listed to both the Python SQL predicate tests and the low-level sedona-tg Rust predicate tests.

Reran:

  • cargo test -p sedona-tg
  • cargo test -p sedona-tg predicates -- --nocapture
  • cargo fmt --check --package sedona-tg
  • git diff --check

@paleolimbot paleolimbot left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you!

@paleolimbot
paleolimbot merged commit 50e24ed into apache:main Jul 10, 2026
17 checks passed
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.

Containment for sedona-tg for geometrycollections may not be correct

2 participants