You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@sedona.apache.org by "jiayuasu (via GitHub)" <gi...@apache.org> on 2026/05/15 15:54:29 UTC

[PR] [GH-2939] Box2D spatial join: ST_BoxIntersects / ST_BoxContains [sedona]

jiayuasu opened a new pull request, #2953:
URL: https://github.com/apache/sedona/pull/2953

   ## Did you read the Contributor Guide?
   
   - Yes, I have read the [Contributor Rules](https://sedona.apache.org/latest/community/rule/) and [Contributor Development Guide](https://sedona.apache.org/latest/community/develop/)
   
   ## Is this PR related to a ticket?
   
   - Yes, and the PR name follows the format `[GH-XXX] my subject`. Closes #2939.
   
   ## What changes were proposed in this PR?
   
   Teaches Sedona's spatial join planner about the Box2D predicates from #2926. After this PR, both broadcast index joins and distributed range joins handle:
   
   - `ST_BoxIntersects(box_a, box_b)`
   - `ST_BoxContains(box_a, box_b)` (both argument orders)
   
   …using the same machinery (partitioner, R-tree, refine evaluator) that already powers `ST_Intersects` / `ST_Contains` joins. No new physical operator, no new index implementation, no new partitioning code.
   
   ### How it works
   
   At every executor boundary where a shape column is materialised — `TraitJoinQueryBase.toSpatialRDD`, `TraitJoinQueryBase.toExpandedEnvelopeRDD`, and `BroadcastIndexJoinExec.createStreamShapes` — dispatch on the shape expression's `dataType`. If it is `Box2DUDT`, read the four doubles out of the serialized `InternalRow` and materialise the implied closed rectangular Polygon via `Constructors.polygonFromEnvelope`. Geometry columns continue through `GeometrySerializer.deserialize` as before.
   
   The materialised Polygon flows through `SpatialRDD<T extends Geometry>`, the spatial partitioner's sample step, `IndexBuilder`'s R-tree, and `SpatialPredicateEvaluators` unchanged. JTS already short-circuits axis-aligned rectangle predicates via `RectangleIntersects` / `RectangleContains` (gated on `Polygon.isRectangle()`), which `polygonFromEnvelope` produces exactly. The refine step therefore pays only a four-double envelope comparison per pair — the savings the user expects from "we know the data is a box".
   
   ### Predicate-kind mapping
   
   | Source predicate | `SpatialPredicate` |
   | ---------------- | ------------------ |
   | `ST_BoxIntersects(a, b)` | `INTERSECTS` |
   | `ST_BoxContains(a, b)`   | `COVERS` |
   
   `ST_BoxContains` deliberately maps to `COVERS`, not `CONTAINS`. PostGIS-style closed-interval containment counts edge-touching pairs as contained; JTS `Geometry.contains` excludes shared-edge pairs (strict interior), whereas `Geometry.covers` accepts them — which is what we want.
   
   ### What's left untouched
   
   - The `Box2D` class hierarchy (still a value class).
   - Storage / on-wire layout (still a struct of four doubles via `Box2DUDT`).
   - `SpatialRDD`, the partitioners, the R-tree, the refine evaluator.
   - The raster join path and all Geography join paths.
   
   ### Scope notes
   
   `ST_DWithin` (distance join) only has `(Geometry, Geometry, distance)` and `(Geography, Geography, distance)` overloads today, so Box2D × Box2D distance joins are out of scope for this PR. Adding a `(Box2D, Box2D, distance)` overload is a follow-up.
   
   ## How was this patch tested?
   
   New `Box2DJoinSuite` under `spark/common/src/test/scala/org/apache/sedona/sql/`:
   
   - `ST_BoxIntersects` broadcast index join produces the expected 4 pairs from a 3×3 fixture, with `BroadcastIndexJoinExec` in the plan.
   - `ST_BoxIntersects(R, L)` produces the same 4 pairs (argument order symmetric).
   - `ST_BoxContains` broadcast index join produces the expected 2 closed-interval containment pairs.
   - Edge-touching containment (closed-interval) is counted — locks in the `COVERS`-not-`CONTAINS` mapping.
   - Non-broadcast range join produces the same 4 pairs with `RangeJoinExec` in the plan.
   - Result is equivalent (same ordered row pairs) to `ST_Intersects` on the same data materialised as polygons via `ST_GeomFromBox2D`.
   
   Run locally against Spark 3.5 / Scala 2.12. Regression runs: `BroadcastIndexJoinSuite` 65/65, `SpatialJoinSuite` 160/160, `Box2DUDTSuite` 5/5, `Box2DCastResolutionRuleSuite` 3/3, `GeoParquetSpatialFilterPushDownSuite` 25/25 — all still pass.
   
   ## Did this PR include necessary documentation updates?
   
   - No, this PR does not affect any public SQL API documentation surface in isolation. The new spatial-join behavior for `ST_BoxIntersects` / `ST_BoxContains` is covered by the consolidated Phase 1+2+3 Box2D docs update.
   


-- 
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: issues-unsubscribe@sedona.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [GH-2939] Box2D spatial join: ST_BoxIntersects / ST_BoxContains [sedona]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #2953:
URL: https://github.com/apache/sedona/pull/2953#discussion_r3249892784


##########
spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/strategy/join/TraitJoinQueryBase.scala:
##########
@@ -49,8 +51,7 @@ trait TraitJoinQueryBase {
     spatialRdd.setRawSpatialRDD(
       rdd
         .map { x =>
-          val shape =
-            GeometrySerializer.deserialize(shapeExpression.eval(x).asInstanceOf[Array[Byte]])
+          val shape = TraitJoinQueryBase.shapeToGeometry(shapeExpression, x)
           shape.setUserData(x.copy)
           shape

Review Comment:
   `shapeToGeometry` can return null (e.g., when the shape column is NULL), but `toSpatialRDD` immediately calls `shape.setUserData(...)`, which will throw NPE. Previously this path used `GeometrySerializer.deserialize(...)`, which converts null bytes to an empty geometry collection, avoiding a crash. Please either filter out null shapes here (flatMap) or map null to a non-null empty geometry consistently.



##########
spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/strategy/join/TraitJoinQueryBase.scala:
##########
@@ -178,3 +178,36 @@ trait TraitJoinQueryBase {
     }
   }
 }
+
+object TraitJoinQueryBase {
+
+  /**
+   * Materialise a shape column value as a JTS [[Geometry]]. Box2D-typed columns are turned into
+   * the closed rectangular polygon implied by their `(xmin, ymin, xmax, ymax)` bounds; all other
+   * shape columns are deserialised from the Sedona geometry binary form.
+   *
+   * Producing a JTS rectangle here lets the rest of the join machinery — partitioner, R-tree
+   * `IndexBuilder`, refine evaluator — stay shape-agnostic. JTS already short-circuits
+   * rectangle-rectangle predicates (`Polygon.isRectangle` triggers `RectangleIntersects` /
+   * `RectangleContains`), so a `ST_BoxIntersects` join naturally pays only the four-double
+   * envelope comparison at refine time.
+   */
+  def shapeToGeometry(shapeExpression: Expression, row: InternalRow): Geometry = {
+    val evaluated = shapeExpression.eval(row)
+    if (evaluated == null) {
+      null
+    } else
+      shapeExpression.dataType match {
+        case _: Box2DUDT =>
+          val box = evaluated.asInstanceOf[InternalRow]
+          Constructors
+            .polygonFromEnvelope(
+              box.getDouble(0),
+              box.getDouble(1),
+              box.getDouble(2),
+              box.getDouble(3))

Review Comment:
   In the Box2D branch, this builds a polygon directly from the four doubles without enforcing the ordered-bound contract (`xmin <= xmax` and `ymin <= ymax`). The scalar `ST_BoxIntersects/ST_BoxContains` path explicitly throws on inverted bounds (see `org.apache.sedona.common.Predicates.boxIntersects/boxContains`), so join planning should reject the same inputs to avoid silently returning misleading results. Also consider using `Constructors.geomFromBox2D` (point/linestring for degenerate boxes) to avoid creating invalid zero-area polygons.
   



##########
spark/common/src/test/scala/org/apache/sedona/sql/Box2DJoinSuite.scala:
##########
@@ -0,0 +1,147 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.sedona.sql
+
+import org.apache.spark.sql.DataFrame
+import org.apache.spark.sql.functions.{broadcast, expr}
+import org.apache.spark.sql.sedona_sql.strategy.join.{BroadcastIndexJoinExec, RangeJoinExec}
+
+class Box2DJoinSuite extends TestBaseScala {
+
+  import Box2DJoinSuite.TestBox
+
+  /**
+   * Three left-side boxes and three right-side boxes wired so we can predict exact result sizes:
+   *
+   *   - L1=(0,0,10,10) R1=(5,5,15,15) — overlapping
+   *   - L1=(0,0,10,10) R2=(2,2,8,8) — R2 fully inside L1
+   *   - L2=(0,0,10,10) R1=(5,5,15,15) — overlapping
+   *   - L2=(0,0,10,10) R2=(2,2,8,8) — R2 fully inside L2
+   *   - L3 and R3 are disjoint from everything else; (L3,R3) is itself disjoint.
+   *
+   * Intersection-pair count: 4. Containment-pair count: 2 (L1⊇R2, L2⊇R2).
+   */
+  private def leftBoxes: DataFrame = {
+    import sparkSession.implicits._
+    Seq(TestBox(1, 0, 0, 10, 10), TestBox(2, 0, 0, 10, 10), TestBox(3, 20, 20, 30, 30))
+      .toDF("id", "xmin", "ymin", "xmax", "ymax")
+      .selectExpr("id", "ST_MakeBox2D(ST_Point(xmin, ymin), ST_Point(xmax, ymax)) AS box")
+  }
+
+  private def rightBoxes: DataFrame = {
+    import sparkSession.implicits._
+    Seq(TestBox(11, 5, 5, 15, 15), TestBox(12, 2, 2, 8, 8), TestBox(13, 40, 40, 50, 50))
+      .toDF("id", "xmin", "ymin", "xmax", "ymax")
+      .selectExpr("id", "ST_MakeBox2D(ST_Point(xmin, ymin), ST_Point(xmax, ymax)) AS box")
+  }
+
+  describe("Box2D spatial join") {
+
+    it("ST_BoxIntersects: broadcast index join produces correct pairs") {
+      val df = leftBoxes
+        .alias("L")
+        .join(broadcast(rightBoxes.alias("R")), expr("ST_BoxIntersects(L.box, R.box)"))
+      val plan = df.queryExecution.sparkPlan

Review Comment:
   This suite covers broadcast/range planning and basic result counts, but it doesn’t exercise Box2D error/NULL edge cases that are part of the Box2D predicate contract (e.g., inverted bounds should throw; NULL boxes should not crash and should produce no matches). Adding targeted tests for those cases would help lock in the intended join semantics, especially given the new Box2D materialization path in the join executors.



-- 
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: issues-unsubscribe@sedona.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [GH-2939] Box2D spatial join: ST_BoxIntersects / ST_BoxContains [sedona]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #2953:
URL: https://github.com/apache/sedona/pull/2953#discussion_r3251175101


##########
spark/common/src/test/scala/org/apache/sedona/sql/Box2DJoinSuite.scala:
##########
@@ -0,0 +1,187 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.sedona.sql
+
+import org.apache.spark.sql.DataFrame
+import org.apache.spark.sql.functions.{broadcast, expr}
+import org.apache.spark.sql.sedona_sql.strategy.join.{BroadcastIndexJoinExec, RangeJoinExec}
+
+class Box2DJoinSuite extends TestBaseScala {
+
+  import Box2DJoinSuite.TestBox
+
+  /**
+   * Three left-side boxes and three right-side boxes wired so we can predict exact result sizes:
+   *
+   *   - L1=(0,0,10,10) R1=(5,5,15,15) — overlapping
+   *   - L1=(0,0,10,10) R2=(2,2,8,8) — R2 fully inside L1
+   *   - L2=(0,0,10,10) R1=(5,5,15,15) — overlapping
+   *   - L2=(0,0,10,10) R2=(2,2,8,8) — R2 fully inside L2
+   *   - L3 and R3 are disjoint from everything else; (L3,R3) is itself disjoint.
+   *
+   * Intersection-pair count: 4. Containment-pair count: 2 (L1⊇R2, L2⊇R2).
+   */
+  private def leftBoxes: DataFrame = {
+    import sparkSession.implicits._
+    Seq(TestBox(1, 0, 0, 10, 10), TestBox(2, 0, 0, 10, 10), TestBox(3, 20, 20, 30, 30))
+      .toDF("id", "xmin", "ymin", "xmax", "ymax")
+      .selectExpr("id", "ST_MakeBox2D(ST_Point(xmin, ymin), ST_Point(xmax, ymax)) AS box")
+  }
+
+  private def rightBoxes: DataFrame = {
+    import sparkSession.implicits._
+    Seq(TestBox(11, 5, 5, 15, 15), TestBox(12, 2, 2, 8, 8), TestBox(13, 40, 40, 50, 50))
+      .toDF("id", "xmin", "ymin", "xmax", "ymax")
+      .selectExpr("id", "ST_MakeBox2D(ST_Point(xmin, ymin), ST_Point(xmax, ymax)) AS box")
+  }
+
+  describe("Box2D spatial join") {
+
+    it("ST_BoxIntersects: broadcast index join produces correct pairs") {
+      val df = leftBoxes
+        .alias("L")
+        .join(broadcast(rightBoxes.alias("R")), expr("ST_BoxIntersects(L.box, R.box)"))
+      val plan = df.queryExecution.sparkPlan
+      assert(
+        plan.collect { case b: BroadcastIndexJoinExec => b }.size == 1,
+        "Expected BroadcastIndexJoinExec in the plan")
+      assert(df.count() == 4)
+    }
+
+    it("ST_BoxIntersects: argument order is symmetric") {
+      val swapped = leftBoxes
+        .alias("L")
+        .join(broadcast(rightBoxes.alias("R")), expr("ST_BoxIntersects(R.box, L.box)"))
+      assert(swapped.count() == 4)
+      assert(swapped.queryExecution.sparkPlan.collect { case b: BroadcastIndexJoinExec =>
+        b
+      }.size == 1)
+    }
+
+    it("ST_BoxContains: broadcast index join uses COVERS semantics") {
+      val df = leftBoxes
+        .alias("L")
+        .join(broadcast(rightBoxes.alias("R")), expr("ST_BoxContains(L.box, R.box)"))
+      assert(df.queryExecution.sparkPlan.collect { case b: BroadcastIndexJoinExec =>
+        b
+      }.size == 1)
+      assert(df.count() == 2)
+    }
+
+    it("ST_BoxContains: edge-touching boxes count (closed-interval semantics)") {
+      // R contained in L sharing an edge: ST_BoxContains is closed-interval, so this matches.
+      // JTS Polygon.contains would reject (strict-interior), JTS Polygon.covers accepts; the
+      // detector maps ST_BoxContains → SpatialPredicate.COVERS specifically for this case.
+      import sparkSession.implicits._
+      val outer = Seq(TestBox(1, 0, 0, 10, 10))
+        .toDF("id", "xmin", "ymin", "xmax", "ymax")
+        .selectExpr("id", "ST_MakeBox2D(ST_Point(xmin, ymin), ST_Point(xmax, ymax)) AS box")
+      // edge-sharing box: same xmax, shares the right edge with outer.
+      val inner = Seq(TestBox(11, 5, 5, 10, 10))
+        .toDF("id", "xmin", "ymin", "xmax", "ymax")
+        .selectExpr("id", "ST_MakeBox2D(ST_Point(xmin, ymin), ST_Point(xmax, ymax)) AS box")
+      val df = outer
+        .alias("O")
+        .join(broadcast(inner.alias("I")), expr("ST_BoxContains(O.box, I.box)"))
+      assert(df.count() == 1, "Closed-interval containment must include edge-touching boxes")
+    }
+
+    it("ST_BoxIntersects: non-broadcast range join produces the same count") {
+      val df = leftBoxes
+        .alias("L")
+        .join(rightBoxes.alias("R"), expr("ST_BoxIntersects(L.box, R.box)"))
+      assert(
+        df.queryExecution.sparkPlan.collect { case r: RangeJoinExec => r }.size == 1,
+        "Expected RangeJoinExec in the plan")
+      assert(df.count() == 4)
+    }
+
+    it("Null Box2D rows are safe and produce no matches") {
+      // A null shape on either side must not crash the executor and must not contribute matches
+      // (mirrors the existing GeometrySerializer.deserialize(null) → empty-collection fallback).
+      import sparkSession.implicits._
+      val withNullLeft = leftBoxes
+        .selectExpr("id", "box AS box")
+        .union(Seq((99, null.asInstanceOf[org.apache.sedona.common.geometryObjects.Box2D]))
+          .toDF("id", "box"))
+      val df = withNullLeft
+        .alias("L")
+        .join(broadcast(rightBoxes.alias("R")), expr("ST_BoxIntersects(L.box, R.box)"))
+      assert(df.count() == 4) // unchanged from the non-null fixture
+      // Range join path (no broadcast) also tolerates nulls.
+      val rangeDf = withNullLeft
+        .alias("L")
+        .join(rightBoxes.alias("R"), expr("ST_BoxIntersects(L.box, R.box)"))
+      assert(rangeDf.count() == 4)
+    }
+
+    it("Inverted Box2D bounds in a join throw IllegalArgumentException") {
+      import sparkSession.implicits._
+      // Construct an inverted Box2D directly via the Java constructor (the SQL ST_MakeBox2D
+      // doesn't validate, so this is how a stored column with inverted bounds would look).
+      val invertedLeft =
+        Seq((1, new org.apache.sedona.common.geometryObjects.Box2D(10.0, 0.0, 0.0, 10.0)))
+          .toDF("id", "box")
+      val ex = intercept[org.apache.spark.SparkException] {
+        invertedLeft
+          .alias("L")
+          .join(broadcast(rightBoxes.alias("R")), expr("ST_BoxIntersects(L.box, R.box)"))
+          .collect()

Review Comment:
   The inverted-bounds test only asserts that an IllegalArgumentException is thrown, but it does not assert that the join is actually planned as a BroadcastIndexJoinExec. Without checking the plan, this test could still pass if join optimization regressed and Spark fell back to a non-spatial join that evaluates ST_BoxIntersects row-by-row (which also throws on inverted bounds). Consider asserting the physical plan contains BroadcastIndexJoinExec (or RangeJoinExec for the non-broadcast path) to ensure this case truly exercises the new join-side Box2D materialization/validation path.
   



-- 
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: issues-unsubscribe@sedona.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [GH-2939] Box2D spatial join: ST_BoxIntersects / ST_BoxContains [sedona]

Posted by "jiayuasu (via GitHub)" <gi...@apache.org>.
jiayuasu commented on code in PR #2953:
URL: https://github.com/apache/sedona/pull/2953#discussion_r3250494928


##########
spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/strategy/join/TraitJoinQueryBase.scala:
##########
@@ -49,8 +51,7 @@ trait TraitJoinQueryBase {
     spatialRdd.setRawSpatialRDD(
       rdd
         .map { x =>
-          val shape =
-            GeometrySerializer.deserialize(shapeExpression.eval(x).asInstanceOf[Array[Byte]])
+          val shape = TraitJoinQueryBase.shapeToGeometry(shapeExpression, x)
           shape.setUserData(x.copy)
           shape

Review Comment:
   Fixed in 9144fb82 — the helper now keeps returning null on null inputs, and toSpatialRDD/toExpandedEnvelopeRDD wrap it in a shapeToGeometryOrEmpty that substitutes an empty GeometryCollection. This matches GeometrySerializer.deserialize(null)'s legacy behaviour. The KnnJoinSuite null-geom regression is gone in local runs.



##########
spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/strategy/join/TraitJoinQueryBase.scala:
##########
@@ -178,3 +178,36 @@ trait TraitJoinQueryBase {
     }
   }
 }
+
+object TraitJoinQueryBase {
+
+  /**
+   * Materialise a shape column value as a JTS [[Geometry]]. Box2D-typed columns are turned into
+   * the closed rectangular polygon implied by their `(xmin, ymin, xmax, ymax)` bounds; all other
+   * shape columns are deserialised from the Sedona geometry binary form.
+   *
+   * Producing a JTS rectangle here lets the rest of the join machinery — partitioner, R-tree
+   * `IndexBuilder`, refine evaluator — stay shape-agnostic. JTS already short-circuits
+   * rectangle-rectangle predicates (`Polygon.isRectangle` triggers `RectangleIntersects` /
+   * `RectangleContains`), so a `ST_BoxIntersects` join naturally pays only the four-double
+   * envelope comparison at refine time.
+   */
+  def shapeToGeometry(shapeExpression: Expression, row: InternalRow): Geometry = {
+    val evaluated = shapeExpression.eval(row)
+    if (evaluated == null) {
+      null
+    } else
+      shapeExpression.dataType match {
+        case _: Box2DUDT =>
+          val box = evaluated.asInstanceOf[InternalRow]
+          Constructors
+            .polygonFromEnvelope(
+              box.getDouble(0),
+              box.getDouble(1),
+              box.getDouble(2),
+              box.getDouble(3))

Review Comment:
   Fixed in 9144fb82 — the Box2D branch now validates xmin <= xmax and ymin <= ymax and throws the same IllegalArgumentException raised by Predicates.boxIntersects/boxContains. Kept polygonFromEnvelope as the materialiser; JTS handles degenerate boxes (zero-area polygon) correctly in both index and refine paths, and switching to geomFromBox2D would introduce shape polymorphism (Point/Line/Polygon) at the join boundary for what's nominally a single rectangle type.



##########
spark/common/src/test/scala/org/apache/sedona/sql/Box2DJoinSuite.scala:
##########
@@ -0,0 +1,147 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.sedona.sql
+
+import org.apache.spark.sql.DataFrame
+import org.apache.spark.sql.functions.{broadcast, expr}
+import org.apache.spark.sql.sedona_sql.strategy.join.{BroadcastIndexJoinExec, RangeJoinExec}
+
+class Box2DJoinSuite extends TestBaseScala {
+
+  import Box2DJoinSuite.TestBox
+
+  /**
+   * Three left-side boxes and three right-side boxes wired so we can predict exact result sizes:
+   *
+   *   - L1=(0,0,10,10) R1=(5,5,15,15) — overlapping
+   *   - L1=(0,0,10,10) R2=(2,2,8,8) — R2 fully inside L1
+   *   - L2=(0,0,10,10) R1=(5,5,15,15) — overlapping
+   *   - L2=(0,0,10,10) R2=(2,2,8,8) — R2 fully inside L2
+   *   - L3 and R3 are disjoint from everything else; (L3,R3) is itself disjoint.
+   *
+   * Intersection-pair count: 4. Containment-pair count: 2 (L1⊇R2, L2⊇R2).
+   */
+  private def leftBoxes: DataFrame = {
+    import sparkSession.implicits._
+    Seq(TestBox(1, 0, 0, 10, 10), TestBox(2, 0, 0, 10, 10), TestBox(3, 20, 20, 30, 30))
+      .toDF("id", "xmin", "ymin", "xmax", "ymax")
+      .selectExpr("id", "ST_MakeBox2D(ST_Point(xmin, ymin), ST_Point(xmax, ymax)) AS box")
+  }
+
+  private def rightBoxes: DataFrame = {
+    import sparkSession.implicits._
+    Seq(TestBox(11, 5, 5, 15, 15), TestBox(12, 2, 2, 8, 8), TestBox(13, 40, 40, 50, 50))
+      .toDF("id", "xmin", "ymin", "xmax", "ymax")
+      .selectExpr("id", "ST_MakeBox2D(ST_Point(xmin, ymin), ST_Point(xmax, ymax)) AS box")
+  }
+
+  describe("Box2D spatial join") {
+
+    it("ST_BoxIntersects: broadcast index join produces correct pairs") {
+      val df = leftBoxes
+        .alias("L")
+        .join(broadcast(rightBoxes.alias("R")), expr("ST_BoxIntersects(L.box, R.box)"))
+      val plan = df.queryExecution.sparkPlan

Review Comment:
   Added in 9144fb82: 'Null Box2D rows are safe and produce no matches' (covers both broadcast and range paths) and 'Inverted Box2D bounds in a join throw IllegalArgumentException' (verifies the exception type and message).



-- 
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: issues-unsubscribe@sedona.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [GH-2939] Box2D spatial join: ST_BoxIntersects / ST_BoxContains [sedona]

Posted by "jiayuasu (via GitHub)" <gi...@apache.org>.
jiayuasu merged PR #2953:
URL: https://github.com/apache/sedona/pull/2953


-- 
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: issues-unsubscribe@sedona.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [GH-2939] Box2D spatial join: ST_BoxIntersects / ST_BoxContains [sedona]

Posted by "jiayuasu (via GitHub)" <gi...@apache.org>.
jiayuasu commented on code in PR #2953:
URL: https://github.com/apache/sedona/pull/2953#discussion_r3254149449


##########
spark/common/src/test/scala/org/apache/sedona/sql/Box2DJoinSuite.scala:
##########
@@ -0,0 +1,187 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.sedona.sql
+
+import org.apache.spark.sql.DataFrame
+import org.apache.spark.sql.functions.{broadcast, expr}
+import org.apache.spark.sql.sedona_sql.strategy.join.{BroadcastIndexJoinExec, RangeJoinExec}
+
+class Box2DJoinSuite extends TestBaseScala {
+
+  import Box2DJoinSuite.TestBox
+
+  /**
+   * Three left-side boxes and three right-side boxes wired so we can predict exact result sizes:
+   *
+   *   - L1=(0,0,10,10) R1=(5,5,15,15) — overlapping
+   *   - L1=(0,0,10,10) R2=(2,2,8,8) — R2 fully inside L1
+   *   - L2=(0,0,10,10) R1=(5,5,15,15) — overlapping
+   *   - L2=(0,0,10,10) R2=(2,2,8,8) — R2 fully inside L2
+   *   - L3 and R3 are disjoint from everything else; (L3,R3) is itself disjoint.
+   *
+   * Intersection-pair count: 4. Containment-pair count: 2 (L1⊇R2, L2⊇R2).
+   */
+  private def leftBoxes: DataFrame = {
+    import sparkSession.implicits._
+    Seq(TestBox(1, 0, 0, 10, 10), TestBox(2, 0, 0, 10, 10), TestBox(3, 20, 20, 30, 30))
+      .toDF("id", "xmin", "ymin", "xmax", "ymax")
+      .selectExpr("id", "ST_MakeBox2D(ST_Point(xmin, ymin), ST_Point(xmax, ymax)) AS box")
+  }
+
+  private def rightBoxes: DataFrame = {
+    import sparkSession.implicits._
+    Seq(TestBox(11, 5, 5, 15, 15), TestBox(12, 2, 2, 8, 8), TestBox(13, 40, 40, 50, 50))
+      .toDF("id", "xmin", "ymin", "xmax", "ymax")
+      .selectExpr("id", "ST_MakeBox2D(ST_Point(xmin, ymin), ST_Point(xmax, ymax)) AS box")
+  }
+
+  describe("Box2D spatial join") {
+
+    it("ST_BoxIntersects: broadcast index join produces correct pairs") {
+      val df = leftBoxes
+        .alias("L")
+        .join(broadcast(rightBoxes.alias("R")), expr("ST_BoxIntersects(L.box, R.box)"))
+      val plan = df.queryExecution.sparkPlan
+      assert(
+        plan.collect { case b: BroadcastIndexJoinExec => b }.size == 1,
+        "Expected BroadcastIndexJoinExec in the plan")
+      assert(df.count() == 4)
+    }
+
+    it("ST_BoxIntersects: argument order is symmetric") {
+      val swapped = leftBoxes
+        .alias("L")
+        .join(broadcast(rightBoxes.alias("R")), expr("ST_BoxIntersects(R.box, L.box)"))
+      assert(swapped.count() == 4)
+      assert(swapped.queryExecution.sparkPlan.collect { case b: BroadcastIndexJoinExec =>
+        b
+      }.size == 1)
+    }
+
+    it("ST_BoxContains: broadcast index join uses COVERS semantics") {
+      val df = leftBoxes
+        .alias("L")
+        .join(broadcast(rightBoxes.alias("R")), expr("ST_BoxContains(L.box, R.box)"))
+      assert(df.queryExecution.sparkPlan.collect { case b: BroadcastIndexJoinExec =>
+        b
+      }.size == 1)
+      assert(df.count() == 2)
+    }
+
+    it("ST_BoxContains: edge-touching boxes count (closed-interval semantics)") {
+      // R contained in L sharing an edge: ST_BoxContains is closed-interval, so this matches.
+      // JTS Polygon.contains would reject (strict-interior), JTS Polygon.covers accepts; the
+      // detector maps ST_BoxContains → SpatialPredicate.COVERS specifically for this case.
+      import sparkSession.implicits._
+      val outer = Seq(TestBox(1, 0, 0, 10, 10))
+        .toDF("id", "xmin", "ymin", "xmax", "ymax")
+        .selectExpr("id", "ST_MakeBox2D(ST_Point(xmin, ymin), ST_Point(xmax, ymax)) AS box")
+      // edge-sharing box: same xmax, shares the right edge with outer.
+      val inner = Seq(TestBox(11, 5, 5, 10, 10))
+        .toDF("id", "xmin", "ymin", "xmax", "ymax")
+        .selectExpr("id", "ST_MakeBox2D(ST_Point(xmin, ymin), ST_Point(xmax, ymax)) AS box")
+      val df = outer
+        .alias("O")
+        .join(broadcast(inner.alias("I")), expr("ST_BoxContains(O.box, I.box)"))
+      assert(df.count() == 1, "Closed-interval containment must include edge-touching boxes")
+    }
+
+    it("ST_BoxIntersects: non-broadcast range join produces the same count") {
+      val df = leftBoxes
+        .alias("L")
+        .join(rightBoxes.alias("R"), expr("ST_BoxIntersects(L.box, R.box)"))
+      assert(
+        df.queryExecution.sparkPlan.collect { case r: RangeJoinExec => r }.size == 1,
+        "Expected RangeJoinExec in the plan")
+      assert(df.count() == 4)
+    }
+
+    it("Null Box2D rows are safe and produce no matches") {
+      // A null shape on either side must not crash the executor and must not contribute matches
+      // (mirrors the existing GeometrySerializer.deserialize(null) → empty-collection fallback).
+      import sparkSession.implicits._
+      val withNullLeft = leftBoxes
+        .selectExpr("id", "box AS box")
+        .union(Seq((99, null.asInstanceOf[org.apache.sedona.common.geometryObjects.Box2D]))
+          .toDF("id", "box"))
+      val df = withNullLeft
+        .alias("L")
+        .join(broadcast(rightBoxes.alias("R")), expr("ST_BoxIntersects(L.box, R.box)"))
+      assert(df.count() == 4) // unchanged from the non-null fixture
+      // Range join path (no broadcast) also tolerates nulls.
+      val rangeDf = withNullLeft
+        .alias("L")
+        .join(rightBoxes.alias("R"), expr("ST_BoxIntersects(L.box, R.box)"))
+      assert(rangeDf.count() == 4)
+    }
+
+    it("Inverted Box2D bounds in a join throw IllegalArgumentException") {
+      import sparkSession.implicits._
+      // Construct an inverted Box2D directly via the Java constructor (the SQL ST_MakeBox2D
+      // doesn't validate, so this is how a stored column with inverted bounds would look).
+      val invertedLeft =
+        Seq((1, new org.apache.sedona.common.geometryObjects.Box2D(10.0, 0.0, 0.0, 10.0)))
+          .toDF("id", "box")
+      val ex = intercept[org.apache.spark.SparkException] {
+        invertedLeft
+          .alias("L")
+          .join(broadcast(rightBoxes.alias("R")), expr("ST_BoxIntersects(L.box, R.box)"))
+          .collect()

Review Comment:
   Fixed in 5faf0cd8 — added a BroadcastIndexJoinExec plan assertion before the intercept, so the test will surface a regression that falls back to row-by-row predicate evaluation rather than silently passing on the predicate's own throw.



-- 
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: issues-unsubscribe@sedona.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org