Skip to content

feat: add ST_Relate(geometry, geometry, text) boolean variant#741

Merged
petern48 merged 11 commits intoapache:mainfrom
Mehak3010:feat/st-relate-boolean
Apr 9, 2026
Merged

feat: add ST_Relate(geometry, geometry, text) boolean variant#741
petern48 merged 11 commits intoapache:mainfrom
Mehak3010:feat/st-relate-boolean

Conversation

@Mehak3010
Copy link
Copy Markdown
Contributor

@Mehak3010 Mehak3010 commented Mar 28, 2026

Adds the boolean overload of ST_Relate:

boolean ST_Relate(geometry geomA, geometry geomB, text intersectionMatrixPattern)

This allows checking if two geometries satisfy a given DE-9IM pattern, complementing the existing text-returning variant.

@Mehak3010 Mehak3010 marked this pull request as draft March 28, 2026 16:32
@Mehak3010 Mehak3010 marked this pull request as ready for review March 30, 2026 11:32
@Mehak3010
Copy link
Copy Markdown
Contributor Author

@petern48 CI checks are all passing now. The PR #174 is ready for review — let me know if any changes are needed.

Copy link
Copy Markdown
Member

@petern48 petern48 left a comment

Choose a reason for hiding this comment

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

Looks like a good start. There are still a few things missing:

  • Add Python integration tests. You can start by using the same test cases as the Python tests added by your previous st_relate PR (except inverted) + including some cases that evaluate to False.

  • Update the docs (docs/reference/sql/st_relate.qmd) with this new syntax. You can take a look at st_buffer.qmd for an example of how to write docs for a function with multiple kernels.

  • Minor nit. Could you link the issue in your PR description? You can just put something like "part of #ISSUE."

At a quick skim, things look good so far. I'll take a more thorough look once these are in place.

@Mehak3010
Copy link
Copy Markdown
Contributor Author

@petern48 Thanks for the feedback! I’ve added the Python integration tests (including false cases), updated the docs with the new syntax, and linked the issue in the PR description.
All CI checks are passing now — ready for further review.

Copy link
Copy Markdown
Member

@petern48 petern48 left a comment

Choose a reason for hiding this comment

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

Looking great. I have a bit more feedback about testing, but I think this should be ready afterwards.

@Mehak3010
Copy link
Copy Markdown
Contributor Author

@petern48 Added the suggested cases — included point-in-polygon-hole, linestring, and geometry collection cases, along with additional False cases for better coverage.
Let me know if anything else should be added!

Copy link
Copy Markdown
Member

@petern48 petern48 left a comment

Choose a reason for hiding this comment

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

Looks like I made a small mistake in my previous suggestion which led to those test failures. Let's bring those test cases back, but I explained how to get them to pass.

@Mehak3010
Copy link
Copy Markdown
Contributor Author

@petern48 Thanks for the clarification! I’ve reverted the NULL handling change (now passing actual NULL), re-added the removed test case, and kept all suggested edge cases. Everything should now align with the expected behavior — ready for review.

Copy link
Copy Markdown
Member

@petern48 petern48 left a comment

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor Author

@Mehak3010 Mehak3010 left a comment

Choose a reason for hiding this comment

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

Thanks for the detailed review and helpful suggestions! I’ve addressed all the changes across tests, docs, and implementation.

@Mehak3010 Mehak3010 requested a review from petern48 April 9, 2026 09:06
Co-authored-by: Peter Nguyen <petern0408@gmail.com>
Copy link
Copy Markdown
Member

@petern48 petern48 left a comment

Choose a reason for hiding this comment

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

Thanks!

@petern48 petern48 merged commit 8b8b75b into apache:main Apr 9, 2026
17 checks passed
@paleolimbot
Copy link
Copy Markdown
Member

Thanks @petern48 and @Mehak3010!

@Mehak3010
Copy link
Copy Markdown
Contributor Author

Thanks @petern48, really appreciate the support and guidance throughout!

Also, building on this work, I’m exploring a proposal around extending boolean spatial predicates and improving API consistency in Sedona. Would love to hear any suggestions or directions if you have in mind. @paleolimbot, @petern48

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.

4 participants