[GH-3039] Flink Box3D accessors, ST_AsText, and ST_3DExtent#3040
Conversation
There was a problem hiding this comment.
Pull request overview
Adds the next slice of Flink Box3D support, aligning Flink’s Box3D SQL surface with the already-existing Box2D patterns (scalar accessors, text formatting, and an extent aggregate).
Changes:
- Added Box3D overloads for
ST_XMin/YMin/ZMin/XMax/YMax/ZMaxandST_AsText(Box3D)in Flink scalar functions. - Implemented
ST_3DExtentaggregate (Geometry → Box3D) with a new six-doubleEnvelope3Daccumulator and registered it in the FlinkCatalog. - Added Flink tests covering Box3D accessors/text and the new
ST_3DExtentbehavior (including null/empty handling).
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| flink/src/test/java/org/apache/sedona/flink/FunctionTest.java | Adds a test validating ST_AsText(Box3D) and all six Box3D accessors. |
| flink/src/test/java/org/apache/sedona/flink/AggregatorTest.java | Adds tests validating ST_3DExtent over mixed XYZ/XY inputs and all-empty/null inputs. |
| flink/src/main/java/org/apache/sedona/flink/expressions/Functions.java | Adds Box3D overloads for existing accessor scalar functions and ST_AsText. |
| flink/src/main/java/org/apache/sedona/flink/expressions/Aggregators.java | Introduces the ST_3DExtent aggregate implementation. |
| flink/src/main/java/org/apache/sedona/flink/expressions/Accumulators.java | Adds Envelope3D accumulator used by ST_3DExtent. |
| flink/src/main/java/org/apache/sedona/flink/Catalog.java | Registers ST_3DExtent for Flink SQL. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 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. |
There was a problem hiding this 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.
| Object o) { | ||
| assert (false); | ||
| } |
There was a problem hiding this 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.
Second Flink Box3D slice, building on the foundation in apache#3037. Adds the Box3D accessor overloads, the BOX3D text form, and the 3D extent aggregate — mirroring the Box2D Flink surface. - Functions: Box3D eval overloads on ST_XMin / ST_YMin / ST_ZMin / ST_XMax / ST_YMax / ST_ZMax (ST_ZMin / ST_ZMax previously had only the Geometry overload), and ST_AsText(box3d) → box3dAsText. - Aggregators.ST_3DExtent: AggregateFunction<Box3D, Envelope3D> mirroring ST_Extent. Per-row folding via Functions.box3D (missing Z → 0, empty → skipped); returns null on an all-empty/null input. - Accumulators.Envelope3D: six-double mutable accumulator. - Registered ST_3DExtent in Catalog (the accessors / ST_AsText gain overloads on already-registered classes, so no new registration). Note: ST_Expand has no Box3D overload — the common layer has no expand(Box3D, ...) and the Spark Box3D surface doesn't expose it either, so it stays out of scope. Tests: - FunctionTest.testBox3DAsTextAndAccessors: BOX3D text + all six accessors over a Box3D. - AggregatorTest.test3DExtent: mixed XYZ / XY-fold-to-0 rows; and test3DExtent_EmptyAndNullGeometries: all-empty/null → NULL. Out of scope (final Flink slice): Box3D ST_Intersects / ST_Contains overloads, ST_3DDWithin.
jiayuasu
force-pushed
the
feature/flink-box3d-accessors
branch
from
e1448ac to
940761d
Compare
Did you read the Contributor Guide?
Is this PR related to a ticket?
What changes were proposed in this PR?
Second Flink Box3D slice. With
Box3DTypeSerializer+ST_Box3D/ST_3DMakeBoxalready merged in #3037, this adds the accessor surface, the text form, and the aggregate — mirroring the Box2D Flink functions.evaloverloads onST_XMin/ST_YMin/ST_ZMin/ST_XMax/ST_YMax/ST_ZMax.ST_ZMin/ST_ZMaxpreviously had only the Geometry overload.ST_AsText(box3d)→box3dAsText(PostGIS-styleBOX3D(...)).Aggregators.ST_3DExtent—AggregateFunction<Box3D, Envelope3D>mirroringST_Extent. Per-row folding viaFunctions.box3D(missing Z → 0, empty → skipped); returns null on an all-empty/null input. NewAccumulators.Envelope3Dsix-double accumulator. Registered inCatalog.ST_Expandis intentionally not given a Box3D overload — the common layer has noexpand(Box3D, ...), and the Spark Box3D surface doesn't expose it either.How was this patch tested?
FunctionTest.testBox3DAsTextAndAccessors—BOX3Dtext + all six accessors over a Box3D. FullFunctionTest: 205 pass.AggregatorTest.test3DExtent(mixed XYZ / XY-fold-to-0 rows) andtest3DExtent_EmptyAndNullGeometries(all-empty/null → NULL). FullAggregatorTest+Box3DTypeSerializerTest: 20 pass.Did this PR include necessary documentation updates?
Out of scope (final Flink slice)
ST_Intersects/ST_Contains;ST_3DDWithin.