petern48 commented on code in PR #2447:
URL: https://github.com/apache/sedona/pull/2447#discussion_r2462357514


##########
spark/common/src/test/scala/org/apache/sedona/sql/SpatialJoinSuite.scala:
##########
@@ -268,6 +268,15 @@ class SpatialJoinSuite extends TestBaseScala with 
TableDrivenPropertyChecks {
         assert(resultRows.isEmpty)
       }
     }
+
+    it("ST_Distance join involving empty geometries should evaluate to false") 
{
+      val result1 = sparkSession.sql(

Review Comment:
   I know, this *technically* isn't a spatial join (so yeah, maybe I can change 
the test name), but isn't this sufficient for what we want to test? If I were 
to write a test for this behavior that **is technically** a spatial join. I 
would join an existing df with a table with all empty geometries. That would be 
equivalent to just testing against a constant empty geometry, wouldn't it? 
Doing this was just more convenient since an empty geom df didn't already exist.
   
   Personally, I didn't find a need to test distance joins. We're just 
modifying the behavior of a predicate. This shouldn't change anything in the 
spatial join outside of whether the predicate evaluates to true or not.
   
   `null < 1` -> `null`. And a result of `null` is treated as `False` for 
joining purposes. It evaluates to something successfully and doesn't error. 
Should be simple as that, right? I don't see any other way for things to fail.
   
   If you disagree, is there a certain case you'd be concerned about? Or do you 
have a suggestion for how to test instead?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to