Copilot commented on code in PR #3040:
URL: https://github.com/apache/sedona/pull/3040#discussion_r3391733370
##########
flink/src/main/java/org/apache/sedona/flink/expressions/Aggregators.java:
##########
@@ -168,6 +170,69 @@ public void resetAccumulator(Accumulators.Envelope acc) {
}
}
+ // Aggregate the 3D bounding box of all input geometries as a Box3D. Mirrors
PostGIS
+ // ST_3DExtent. Geometries without a Z dimension fold into z = 0 per
coordinate (via
+ // Functions.box3D). Returns null when there are no rows or all inputs are
null/empty.
+ @DataTypeHint(value = "RAW", rawSerializer = Box3DTypeSerializer.class,
bridgedTo = Box3D.class)
+ public static class ST_3DExtent extends AggregateFunction<Box3D,
Accumulators.Envelope3D> {
+
+ @Override
+ public Accumulators.Envelope3D createAccumulator() {
+ return new Accumulators.Envelope3D();
+ }
+
+ @Override
+ @DataTypeHint(value = "RAW", rawSerializer = Box3DTypeSerializer.class,
bridgedTo = Box3D.class)
+ public Box3D getValue(Accumulators.Envelope3D acc) {
+ if (acc.minX > acc.maxX) return null;
+ return new Box3D(acc.minX, acc.minY, acc.minZ, acc.maxX, acc.maxY,
acc.maxZ);
+ }
+
+ public void accumulate(
+ Accumulators.Envelope3D acc,
+ @DataTypeHint(
+ value = "RAW",
+ rawSerializer = GeometryTypeSerializer.class,
+ bridgedTo = Geometry.class)
+ Object o) {
+ if (o == null) return;
+ // Functions.box3D folds missing Z to 0 and returns null for empty
geometries.
+ Box3D box = org.apache.sedona.common.Functions.box3D((Geometry) o);
+ if (box == null) return;
+ acc.minX = Math.min(acc.minX, box.getXMin());
+ acc.minY = Math.min(acc.minY, box.getYMin());
+ acc.minZ = Math.min(acc.minZ, box.getZMin());
+ acc.maxX = Math.max(acc.maxX, box.getXMax());
+ acc.maxY = Math.max(acc.maxY, box.getYMax());
+ acc.maxZ = Math.max(acc.maxZ, box.getZMax());
+ }
+
+ public void retract(
+ Accumulators.Envelope3D acc,
+ @DataTypeHint(
+ value = "RAW",
+ rawSerializer = GeometryTypeSerializer.class,
+ bridgedTo = Geometry.class)
+ Object o) {
+ assert (false);
+ }
Review Comment:
Using `assert(false)` in `retract` is unsafe because Java assertions are
often disabled in production, making `retract` a silent no-op and potentially
producing incorrect results if Flink invokes retraction (e.g., in
streaming/upsert scenarios). Throw an exception unconditionally to fail fast
when retraction is requested.
##########
flink/src/test/java/org/apache/sedona/flink/AggregatorTest.java:
##########
@@ -64,6 +65,37 @@ public void testExtent() {
assertEquals(5.0, bbox.getYMax(), 0.0);
}
+ @Test
+ public void test3DExtent() {
+ tableEnv.executeSql(
+ "CREATE OR REPLACE TEMPORARY VIEW extent3d_view AS "
+ + "SELECT ST_GeomFromWKT(wkt) as geom FROM ("
+ + "VALUES ('POINT Z (1 2 3)'), ('POINT Z (4 5 -1)'), ('LINESTRING
(-3 0, 0 0)')"
+ + ") AS t(wkt)");
+ Table result = tableEnv.sqlQuery("SELECT ST_3DExtent(geom) FROM
extent3d_view");
+ Row last = last(result);
+ Box3D bbox = (Box3D) last.getField(0);
+ assertEquals(-3.0, bbox.getXMin(), 0.0);
+ assertEquals(0.0, bbox.getYMin(), 0.0);
+ // The XY-only linestring folds to z = 0, which is the minimum across the
three rows.
Review Comment:
The inline comment is incorrect: the XY-only LINESTRING folds to z=0, but
the overall z-min across the three rows is still -1.0 (from the second point).
This can mislead future readers debugging the test expectation.
--
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]
