feat: Implement ST_Crosses and ST_Overlaps predicates #204

Abeeujah · 2025-10-10T14:32:57Z

closes #199

paleolimbot

Thank you!

A few optional things to consider, but in general this looks good to me!

We also include integration tests for all our functions in Python (to check against PostGIS). It would be helpful if you did that here, but it is also no problem if you prefer not to (you or one of us can create a follow-on issue to ensure we do it at some point). These integration tests will be very similar to the other predicate integration tests:

sedona-db/python/sedonadb/tests/functions/test_predicates.py

Lines 53 to 82 in 95c156d

    
           @pytest.mark.parametrize("eng", [SedonaDB, PostGIS]) 
        
           @pytest.mark.parametrize( 
        
               ("geom1", "geom2", "expected"), 
        
               [ 
        
                   (None, None, None), 
        
                   ("POINT (0 0)", None, None), 
        
                   (None, "POINT (0 0)", None), 
        
                   ("POINT (0 0)", "POLYGON ((0 0, 1 0, 1 1, 0 1, 0 0))", True), 
        
                   ("POINT (0.5 0.5)", "POLYGON ((0 0, 1 0, 1 1, 0 1, 0 0))", True), 
        
                   ("POINT (0 0)", "POINT EMPTY", False), 
        
                   ("POINT (0 0)", "LINESTRING (0 0, 1 1)", True), 
        
                   ("LINESTRING (0 0, 1 1)", "POLYGON ((0 0, 1 0, 1 1, 0 1, 0 0))", True), 
        
                   ( 
        
                       "POLYGON ((0 0, 1 0, 1 1, 0 1, 0 0))", 
        
                       "POLYGON ((5 5, 6 5, 6 6, 5 6, 5 5))", 
        
                       False, 
        
                   ), 
        
                   ( 
        
                       "POINT (1 1)", 
        
                       "GEOMETRYCOLLECTION (POINT (0 0), POLYGON ((0 0, 1 0, 1 1, 0 1, 0 0)), LINESTRING (0 0, 1 1))", 
        
                       True, 
        
                   ), 
        
               ], 
        
           ) 
        
           def test_st_covered_by(eng, geom1, geom2, expected): 
        
               eng = eng.create_or_skip() 
        
               eng.assert_query_result( 
        
                   f"SELECT ST_CoveredBy({geom_or_null(geom1)}, {geom_or_null(geom2)})", 
        
                   expected, 
        
               )

...and some instructions on how to get started with the Python package are here: https://github.com/apache/sedona-db/blob/main/docs/contributors-guide.md#python

paleolimbot · 2025-10-10T15:32:03Z

c/sedona-geos/src/binary_predicates.rs

+        let arg2 = create_array(
+            &[
+                Some("POLYGON ((1 1, 1 3, 3 3, 3 1, 1 1))"),
+                Some("LINESTRING (1 0, 3 0)"),
+                Some("POLYGON ((0 0, 0 1, 1 1, 1 0, 0 0))"),
+            ],
+            &WKB_GEOMETRY,
+        );
+        let expected: ArrayRef = arrow_array!(Boolean, [Some(true), Some(true), None]);


I think this test would be more useful if one of the results were false

paleolimbot · 2025-10-10T15:32:34Z

c/sedona-geos/src/geos.rs

    }
 }
+
+/// Check if the geometries crosses


nit:

Suggested change

/// Check if the geometries crosses

/// Check if the geometries cross

paleolimbot · 2025-10-10T15:32:44Z

c/sedona-geos/src/geos.rs

+    }
+}
+
+/// Check if the geometries overlaps


nit:

Suggested change

/// Check if the geometries overlaps

/// Check if the geometries overlap

jiayuasu · 2025-10-10T15:42:58Z

Great work!

@paleolimbot
@Kontinuation

how do these functions work with our current spatial join implementation?

Kontinuation · 2025-10-10T15:58:14Z

how do these functions work with our current spatial join implementation?

Spatial join has its own spatial predicate evaluators for statistics awareness and better performance; UDF implementations of these predicates have no impact on spatial join.

Abeeujah · 2025-10-12T00:16:14Z

Hi @paleolimbot My bad, I was following the reference PR mentioned in the issue description, it didn't seem to include integrated tests on the python side, but I've currently implemented that, I'd love you to give it a look again,

For the nitpick suggestions/comments in the rust side, I've resolved them again as well, looking forward to getting a review at your earliest.

@jiayuasu and @Kontinuation Thank y'all so much for the kind words!

petern48 · 2025-10-12T00:45:39Z

My bad, I was following the reference PR mentioned in the issue description, it didn't seem to include integrated tests on the Python side, but I've currently implemented that, I'd love you to give it a look again

Yeah, this is my fault for not mentioning it in the issue, not yours. We added the integration test suite completely after all of the other predicates. Great job on adding them.

petern48 · 2025-10-12T00:48:55Z

python/sedonadb/tests/functions/test_predicates.py

+        ("POINT (0 0)", "POINT (0 0)", False),
+        ("LINESTRING (0 0, 2 2)", "LINESTRING (1 1, 3 3)", True),
+        ("LINESTRING (0 0, 1 1)", "LINESTRING (0 1, 1 0)", False),
+        ("LINESTRING (0 0, 1 1)", "LINESTRING (1 1, 2 2)", False),


I like that we have some good edge cases here like identical geometries ("POINT (0 0)", "POINT (0 0)") and "touching geometries" (i.e. "LINESTRING (0 0, 1 1)", "LINESTRING (1 1, 2 2)")

petern48 · 2025-10-12T00:56:51Z

python/sedonadb/tests/functions/test_predicates.py

+    [
+        (None, None, None),
+        ("POINT (0 0)", None, None),
+        (None, "POINT (0 0)", None),


Suggested change

(None, "POINT (0 0)", None),

(None, "POINT (0 0)", None),

("POINT (0 0)", "POINT EMPTY", False),

How about we add one more case (empty geometries) while we're here. I checked out the branch and tested these locally, and saw that they already pass.

petern48 · 2025-10-12T00:56:55Z

python/sedonadb/tests/functions/test_predicates.py

+    [
+        (None, None, None),
+        ("POINT (0 0)", None, None),
+        (None, "POINT (0 0)", None),


Suggested change

(None, "POINT (0 0)", None),

(None, "POINT (0 0)", None),

("POINT (0 0)", "POINT EMPTY", False),

Abeeujah · 2025-10-12T01:34:38Z

@petern48 it's all cool, your issue description and the PR linked was super helpful, gave me enough context to raise a PR, Thank you for that.

paleolimbot

Thank you @Abeeujah (and thank you @petern48 for the review!) 🚀

feat: Implement ST_Crosses and ST_Overlaps predicates

2ccf81e

paleolimbot approved these changes Oct 10, 2025

View reviewed changes

Abeeujah added 3 commits October 12, 2025 00:14

dev: fix doc comments

632a2c0

dev: Fix nitpick suggestion for overlaps_udf test

a497152

dev: Add Python integration test for crosses and overlap

b46130f

petern48 reviewed Oct 12, 2025

View reviewed changes

dev: Add Edge case for identical and touching geometries

c926c2a

paleolimbot approved these changes Oct 12, 2025

View reviewed changes

paleolimbot merged commit 5493b8e into apache:main Oct 12, 2025
12 checks passed

	@pytest.mark.parametrize("eng", [SedonaDB, PostGIS])
	@pytest.mark.parametrize(
	("geom1", "geom2", "expected"),
	[
	(None, None, None),
	("POINT (0 0)", None, None),
	(None, "POINT (0 0)", None),
	("POINT (0 0)", "POLYGON ((0 0, 1 0, 1 1, 0 1, 0 0))", True),
	("POINT (0.5 0.5)", "POLYGON ((0 0, 1 0, 1 1, 0 1, 0 0))", True),
	("POINT (0 0)", "POINT EMPTY", False),
	("POINT (0 0)", "LINESTRING (0 0, 1 1)", True),
	("LINESTRING (0 0, 1 1)", "POLYGON ((0 0, 1 0, 1 1, 0 1, 0 0))", True),
	(
	"POLYGON ((0 0, 1 0, 1 1, 0 1, 0 0))",
	"POLYGON ((5 5, 6 5, 6 6, 5 6, 5 5))",
	False,
	),
	(
	"POINT (1 1)",
	"GEOMETRYCOLLECTION (POINT (0 0), POLYGON ((0 0, 1 0, 1 1, 0 1, 0 0)), LINESTRING (0 0, 1 1))",
	True,
	),
	],
	)
	def test_st_covered_by(eng, geom1, geom2, expected):
	eng = eng.create_or_skip()
	eng.assert_query_result(
	f"SELECT ST_CoveredBy({geom_or_null(geom1)}, {geom_or_null(geom2)})",
	expected,
	)

	/// Check if the geometries crosses
	/// Check if the geometries cross

	/// Check if the geometries overlaps
	/// Check if the geometries overlap

feat: Implement ST_Crosses and ST_Overlaps predicates #204

feat: Implement ST_Crosses and ST_Overlaps predicates #204

Uh oh!

Conversation

Abeeujah commented Oct 10, 2025

Uh oh!

paleolimbot left a comment

Choose a reason for hiding this comment

Uh oh!

paleolimbot Oct 10, 2025

Choose a reason for hiding this comment

Uh oh!

paleolimbot Oct 10, 2025

Choose a reason for hiding this comment

Uh oh!

paleolimbot Oct 10, 2025

Choose a reason for hiding this comment

Uh oh!

jiayuasu commented Oct 10, 2025

Uh oh!

Kontinuation commented Oct 10, 2025

Uh oh!

Abeeujah commented Oct 12, 2025

Uh oh!

petern48 commented Oct 12, 2025

Uh oh!

petern48 Oct 12, 2025

Choose a reason for hiding this comment

Uh oh!

petern48 Oct 12, 2025

Choose a reason for hiding this comment

Uh oh!

petern48 Oct 12, 2025

Choose a reason for hiding this comment

Uh oh!

Abeeujah commented Oct 12, 2025

Uh oh!

paleolimbot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants