james-willis opened a new pull request, #141: URL: https://github.com/apache/sedona-spatialbench/pull/141
## What Q5 computes a convex hull over each customer-month's dropoff locations. The base class — documented as the Sedona/Spark SQL dialect — spells that as a scalar over a materialized array: ```sql ST_Area(ST_ConvexHull(ST_Collect(ARRAY_AGG(ST_GeomFromWKB(t.t_dropoffloc))))) ``` and `SedonaDBSpatialBenchBenchmark` overrides Q5 **solely** to spell it as the native aggregate `ST_Collect_Agg`. `ST_Collect_Agg` is a first-class Sedona Spark function, not a SedonaDB-ism — registered in [`st_aggregates.scala`](https://github.com/apache/sedona/blob/sedona-1.9.1/spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/st_aggregates.scala), with Python and Flink bindings and its own [docs page](https://sedona.apache.org/latest/api/sql/Aggregate-Functions/ST_Collect_Agg/). So the base dialect can say it directly and SedonaDB needs no override at all. `ARRAY_AGG` also builds a full array of geometries per group before `ST_Collect` runs, where a native aggregate accumulates into one collection incrementally. (Motivation, not a benchmark claim — I have not measured it.) ## The catch: DuckDB DuckDB inherits the base Q5, and **DuckDB spatial has no `ST_Collect_Agg`**. I checked this by running it rather than reading docs: ``` duckdb 1.5.5, spatial eb1e57c ST_Collect(ARRAY_AGG(...)) -> OK ST_Collect_Agg(...) -> Catalog Error: Scalar Function with name st_collect_agg does not exist! duckdb_functions(): ST_Collect (scalar), ST_Envelope_Agg (aggregate), ST_Union_Agg (aggregate) ``` Its `ST_Collect` is a scalar over a `GEOMETRY[]`. So the `ARRAY_AGG` spelling **moves into a DuckDB override** rather than disappearing. Net effect: each override now sits where the dialect genuinely differs. | class | Q5 spelling | before → after | |---|---|---| | `SpatialBenchBenchmark` (base) | `ST_Collect_Agg` | changed | | `DuckDBSpatialBenchBenchmark` | `ST_Collect(ARRAY_AGG(…))` | **override added** (same SQL the base had) | | `SedonaDBSpatialBenchBenchmark` | inherits base | **override removed** | | `DatabricksSpatialBenchBenchmark` | `ST_Union_Agg` | unchanged (comment reworded) | ## Verification **No engine's executable SQL changes except the base dialect's.** Comparing every query before and after with comments and whitespace normalized away: ``` SedonaDB queries whose SQL changed: NONE DuckDB queries whose SQL changed: NONE Databricks queries whose SQL changed: NONE SedonaSpark (base) queries whose SQL changed: ['q5'] ``` so the committed answers cannot move. Confirmed by running it anyway, against `benchmark/answers/sf{1,10}/q5.csv` (CI tolerance is `rtol=1e-6`): | engine | scale | result | |---|---|---| | Sedona Spark 1.9.1 / Spark 3.5.3 — **both** spellings | SF1 | bit-identical to each other; both match, max rel diff 3.6e-16 | | Sedona Spark 1.9.1 / Spark 3.5.3 — **both** spellings | SF10 | bit-identical to each other; both match, max rel diff 4.1e-16 | | SedonaDB 0.4.1 — new base Q5 | SF1 | matches, max rel diff 1.2e-16 | | DuckDB 1.5.5 — new DuckDB Q5 | SF1 | matches, max rel diff 2.4e-16 | `SHOW FUNCTIONS` on Sedona Spark 1.9.1 lists `st_collect_agg` alongside `st_collect` and `st_union_agg`, so both spellings resolve there. SedonaDB rejects the *old* base Q5 (`Invalid function 'st_collect'`) — which is exactly why the override existed. SF10 used the committed `trip` data plus a `customer` table regenerated with `spatialbench-cli` at SF10; the generator reproduces the existing SF1 `customer` table row for row, so it is the same data. `benchmark/answers/` is untouched. ## Also updated `notebooks/queries.ipynb` and the two rendered docs pages showed Q5 as `ST_Collect(ST_GeomFromWKB(...))` inside an `sd.sql(...)` call — a spelling current SedonaDB rejects outright, so the documented query could not be run as shown. They now match the base. The stored notebook output is unchanged because the result is unchanged. ## Deliberately not touched The geography suite carries the same pattern, but it has no DuckDB implementation, is not run by the benchmark workflow, and only borrows the base class's query-collection reflection rather than its SQL — I verified nothing here leaks into it. Worth a follow-up if maintainers want the suites symmetric. --- 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
