feat: add ST_Relate(geometry, geometry, text) boolean variant#741
feat: add ST_Relate(geometry, geometry, text) boolean variant#741petern48 merged 11 commits intoapache:mainfrom
Conversation
petern48
left a comment
There was a problem hiding this comment.
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.
|
@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. |
petern48
left a comment
There was a problem hiding this comment.
Looking great. I have a bit more feedback about testing, but I think this should be ready afterwards.
Co-authored-by: Peter Nguyen <petern0408@gmail.com>
Co-authored-by: Peter Nguyen <petern0408@gmail.com>
|
@petern48 Added the suggested cases — included point-in-polygon-hole, linestring, and geometry collection cases, along with additional False cases for better coverage. |
petern48
left a comment
There was a problem hiding this comment.
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.
|
@petern48 Thanks for the clarification! I’ve reverted the NULL handling change (now passing actual |
Mehak3010
left a comment
There was a problem hiding this comment.
Thanks for the detailed review and helpful suggestions! I’ve addressed all the changes across tests, docs, and implementation.
Co-authored-by: Peter Nguyen <petern0408@gmail.com>
|
Thanks @petern48 and @Mehak3010! |
|
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 |
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.
This work addresses issue feat: Implement ST_Relate using
geos#539 and is part of the broader implementation tracked in ossue epic: st function coverage #174 .