jiayuasu commented on code in PR #3040:
URL: https://github.com/apache/sedona/pull/3040#discussion_r3393608493
##########
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:
Fixed in latest push. The comment now states z-min is -1.0 from POINT Z (4 5
-1), and that the XY-only linestring folds to z=0 which sits between the -1 and
3 of the two POINT Z rows, so it moves neither Z bound.
##########
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:
Valid in principle, but `assert(false)` in retract is the established
convention across all five aggregators in this file (ST_Envelope_Aggr,
ST_Extent, ST_Intersection_Aggr, ST_Union_Aggr, and this one) — none of these
aggregates support retraction. Making ST_3DExtent the only one that throws
unconditionally would be an inconsistent one-off; ST_3DExtent deliberately
mirrors its Box2D sibling ST_Extent line-for-line. Converting retract to
fail-fast is worth doing, but uniformly across all five in a separate cleanup
rather than as a divergence introduced in this Box3D feature slice.
--
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]
