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/22 07:13:01 UTC

[PR] [GH-2973] Box3D constructors: ST_Box3D and ST_3DMakeBox [sedona]

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

   ## 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`. Second of 5 slices for #2973. Builds on the foundation that landed in #2978.
   
   ## What changes were proposed in this PR?
   
   Adds the two scalar constructors that produce a `Box3D` from a Geometry or two corner points. Mirrors `ST_Box2D` / `ST_MakeBox2D` and PostGIS's `Box3D(geometry)` / `ST_3DMakeBox`.
   
   - `Functions.box3D(geom)`: returns the 3D bbox of a Geometry. Geometries whose coordinates have NaN Z fold into the `z = 0` plane on a per-coordinate basis, matching PostGIS's flat-XY-treated-as-XY[Z=0] convention.
   - `Constructors.make3DBox(p1, p2)`: builds a Box3D from two corner POINTZ geometries. Missing-Z point inputs contribute `z = 0`. Non-POINT input raises `IllegalArgumentException` (same as `ST_MakeBox2D`).
   - `ST_Box3D` and `ST_3DMakeBox` Catalyst case classes wired through `InferredExpression` and registered in `Catalog.boundingBoxExprs` / `Catalog.geometryConstructorExprs`.
   
   ## How was this patch tested?
   
   `Box3DConstructorSuite` (5 tests, all passing locally on Spark 3.5 / Scala 2.12):
   
   - `ST_Box3D` on XY input (zmin/zmax = 0) and XYZ input.
   - `ST_Box3D` returns NULL for null and empty geometry input.
   - `ST_3DMakeBox` from two POINTZ corners.
   - `ST_3DMakeBox` from XY-only points (Z folds to zero).
   - `ST_3DMakeBox` returns NULL for null point input.
   
   ## Did this PR include necessary documentation updates?
   
   - No, the consolidated Box3D documentation lands as a follow-up PR analogous to the Box2D docs PR #2966.
   


-- 
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-2973] Box3D constructors: ST_Box3D and ST_3DMakeBox [sedona]

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


##########
spark/common/src/test/scala/org/apache/sedona/sql/Box3DConstructorSuite.scala:
##########
@@ -0,0 +1,66 @@
+/*
+ * 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.sedona.common.geometryObjects.Box3D
+
+class Box3DConstructorSuite extends TestBaseScala {
+
+  describe("Box3D constructors") {
+
+    it("ST_Box3D returns the 3D bbox, defaulting Z=0 for XY input") {
+      val row = sparkSession
+        .sql("SELECT ST_Box3D(ST_GeomFromText('LINESTRING(0 0, 10 20)')) AS box2d, " +
+          "ST_Box3D(ST_GeomFromWKT('LINESTRING Z(0 0 -3, 5 10 7)')) AS box3d")
+        .collect()(0)
+      assert(row.getAs[Box3D]("box2d") == new Box3D(0, 0, 0, 10, 20, 0))

Review Comment:
   In this test, the SQL alias/name `box2d` is used for the result of `ST_Box3D(...)`, and the assertion reads the column back as `Box3D`. Renaming the alias (and corresponding `getAs` key) to something like `box3d_xy` / `box3d_from_xy` would avoid confusion when reading or extending the suite.
   



##########
spark/common/src/test/scala/org/apache/sedona/sql/Box3DConstructorSuite.scala:
##########
@@ -0,0 +1,66 @@
+/*
+ * 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.sedona.common.geometryObjects.Box3D
+
+class Box3DConstructorSuite extends TestBaseScala {
+
+  describe("Box3D constructors") {
+
+    it("ST_Box3D returns the 3D bbox, defaulting Z=0 for XY input") {
+      val row = sparkSession
+        .sql("SELECT ST_Box3D(ST_GeomFromText('LINESTRING(0 0, 10 20)')) AS box2d, " +
+          "ST_Box3D(ST_GeomFromWKT('LINESTRING Z(0 0 -3, 5 10 7)')) AS box3d")
+        .collect()(0)
+      assert(row.getAs[Box3D]("box2d") == new Box3D(0, 0, 0, 10, 20, 0))
+      assert(row.getAs[Box3D]("box3d") == new Box3D(0, 0, -3, 5, 10, 7))
+    }
+
+    it("ST_Box3D returns NULL for null and empty input") {
+      val row = sparkSession
+        .sql("SELECT ST_Box3D(ST_GeomFromText(NULL)) AS b_null, " +
+          "ST_Box3D(ST_GeomFromText('LINESTRING EMPTY')) AS b_empty")
+        .collect()(0)
+      assert(row.isNullAt(0))
+      assert(row.isNullAt(1))
+    }
+
+    it("ST_3DMakeBox builds a Box3D from two POINTZ corners") {
+      val row = sparkSession
+        .sql("SELECT ST_3DMakeBox(ST_PointZ(0, 0, 0), ST_PointZ(2, 4, 6)) AS b")
+        .collect()(0)
+      assert(row.getAs[Box3D]("b") == new Box3D(0, 0, 0, 2, 4, 6))
+    }
+
+    it("ST_3DMakeBox treats missing Z as 0") {
+      val row = sparkSession
+        .sql("SELECT ST_3DMakeBox(ST_Point(0, 0), ST_Point(2, 4)) AS b")
+        .collect()(0)
+      assert(row.getAs[Box3D]("b") == new Box3D(0, 0, 0, 2, 4, 0))
+    }
+
+    it("ST_3DMakeBox returns NULL for null point input") {
+      val row = sparkSession
+        .sql("SELECT ST_3DMakeBox(ST_GeomFromText(NULL), ST_PointZ(2, 4, 6)) AS b")
+        .collect()(0)
+      assert(row.isNullAt(0))
+    }

Review Comment:
   `ST_3DMakeBox` is specified to throw `IllegalArgumentException` for non-POINT inputs and to return NULL for empty POINT inputs (mirroring the existing `ST_MakeBox2D` behavior), but this suite currently only covers the happy path and null handling. Adding tests for a non-POINT argument (e.g., LINESTRING) and for `POINT EMPTY` would lock in the intended contract and error 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-2973] Box3D constructors: ST_Box3D and ST_3DMakeBox [sedona]

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


-- 
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