jiayuasu commented on code in PR #2932:
URL: https://github.com/apache/sedona/pull/2932#discussion_r3206902469
##########
common/src/main/java/org/apache/sedona/common/Predicates.java:
##########
@@ -27,6 +28,30 @@ public static boolean contains(Geometry leftGeometry,
Geometry rightGeometry) {
return leftGeometry.contains(rightGeometry);
}
+ /**
+ * Closed-interval bbox intersection: true if {@code a} and {@code b} share
any point on either
+ * axis (matches PostGIS {@code &&} on box2d). Either argument being null
returns null at the SQL
+ * layer; this Java entry point throws {@link NullPointerException} on null
input.
+ */
+ public static boolean boxIntersects(Box2D a, Box2D b) {
+ return !(a.getXMax() < b.getXMin()
+ || a.getXMin() > b.getXMax()
+ || a.getYMax() < b.getYMin()
+ || a.getYMin() > b.getYMax());
Review Comment:
Done in 6726b1b6 — went with option (a). `boxIntersects`/`boxContains` now
call `requireOrderedPlanarBox` and throw `IllegalArgumentException` with a
clear message on inverted bounds. Defining wraparound semantics on a planar
predicate without the geography work in #2929 would be premature; failing fast
is the honest contract for now.
##########
common/src/main/java/org/apache/sedona/common/Predicates.java:
##########
@@ -27,6 +28,30 @@ public static boolean contains(Geometry leftGeometry,
Geometry rightGeometry) {
return leftGeometry.contains(rightGeometry);
}
+ /**
+ * Closed-interval bbox intersection: true if {@code a} and {@code b} share
any point on either
+ * axis (matches PostGIS {@code &&} on box2d). Either argument being null
returns null at the SQL
+ * layer; this Java entry point throws {@link NullPointerException} on null
input.
+ */
+ public static boolean boxIntersects(Box2D a, Box2D b) {
+ return !(a.getXMax() < b.getXMin()
+ || a.getXMin() > b.getXMax()
+ || a.getYMax() < b.getYMin()
+ || a.getYMin() > b.getYMax());
+ }
+
+ /**
+ * True if {@code a} fully contains {@code b} (closed intervals; matches
PostGIS {@code ~} on
+ * box2d). Either argument being null returns null at the SQL layer; this
Java entry point throws
+ * {@link NullPointerException} on null input.
+ */
+ public static boolean boxContains(Box2D a, Box2D b) {
+ return a.getXMin() <= b.getXMin()
+ && a.getYMin() <= b.getYMin()
+ && a.getXMax() >= b.getXMax()
+ && a.getYMax() >= b.getYMax();
Review Comment:
Same fix in 6726b1b6 — `boxContains` now validates ordered bounds before
evaluating containment.
##########
common/src/main/java/org/apache/sedona/common/Predicates.java:
##########
@@ -27,6 +28,30 @@ public static boolean contains(Geometry leftGeometry,
Geometry rightGeometry) {
return leftGeometry.contains(rightGeometry);
}
+ /**
+ * Closed-interval bbox intersection: true if {@code a} and {@code b} share
any point on either
+ * axis (matches PostGIS {@code &&} on box2d). Either argument being null
returns null at the SQL
+ * layer; this Java entry point throws {@link NullPointerException} on null
input.
Review Comment:
Reworded in 6726b1b6: now says overlap on **both** the X and Y axes.
##########
common/src/test/java/org/apache/sedona/common/PredicatesTest.java:
##########
@@ -32,6 +33,38 @@ public class PredicatesTest extends TestBase {
private static final GeometryFactory GEOMETRY_FACTORY = new
GeometryFactory();
+ @Test
+ public void testBoxIntersects() {
+ Box2D a = new Box2D(0.0, 0.0, 5.0, 5.0);
+
+ // Full overlap
+ assertTrue(Predicates.boxIntersects(a, new Box2D(1.0, 1.0, 2.0, 2.0)));
+ // Partial overlap
+ assertTrue(Predicates.boxIntersects(a, new Box2D(3.0, 3.0, 7.0, 7.0)));
+ // Edge-touching (closed intervals)
+ assertTrue(Predicates.boxIntersects(a, new Box2D(5.0, 0.0, 10.0, 5.0)));
+ // Corner-touching (closed intervals)
+ assertTrue(Predicates.boxIntersects(a, new Box2D(5.0, 5.0, 10.0, 10.0)));
+ // Disjoint on X
+ assertFalse(Predicates.boxIntersects(a, new Box2D(6.0, 0.0, 10.0, 5.0)));
+ // Disjoint on Y
+ assertFalse(Predicates.boxIntersects(a, new Box2D(0.0, 6.0, 5.0, 10.0)));
+ }
Review Comment:
Added `testBoxPredicatesRejectInvertedBounds` in 6726b1b6 covering both
inverted-X (the antimeridian-style case) and inverted-Y, asserting the
documented `IllegalArgumentException` is thrown.
##########
spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/Predicates.scala:
##########
@@ -95,6 +96,22 @@ private[apache] case class ST_Contains(inputExpressions:
Seq[Expression])
*
* @param inputExpressions
*/
+private[apache] case class ST_BoxIntersects(inputExpressions: Seq[Expression])
+ extends InferredExpression(Predicates.boxIntersects _) {
+
Review Comment:
Fixed in 6726b1b6 — replaced the bleed-through Scaladoc above
`ST_BoxIntersects` with proper Box2D semantics, added a parallel doc on
`ST_BoxContains`, and restored the original `ST_Intersects` Scaladoc below.
##########
spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/Predicates.scala:
##########
@@ -19,6 +19,7 @@
package org.apache.spark.sql.sedona_sql.expressions
import org.apache.sedona.common.Predicates
+import org.apache.sedona.common.geometryObjects.Box2D
Review Comment:
Removed in 6726b1b6 — `Box2D` was used implicitly via
`Predicates.boxIntersects _` eta-expansion, no explicit reference needed in
this file.
--
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]
