Spell Q5's hull aggregation as ST_Collect_Agg in the base dialect

[james-willis]

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:

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,
with Python and Flink bindings and its own docs page.
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

[james-willis]

Withdrawing this. Further testing undercut the premise it was built on.

1. The SedonaDB override is not redundant — it is load-bearing. DuckDB inherits the
base Q5, and DuckDB spatial has no ST_Collect_Agg (duckdb_functions() lists
ST_Collect as a scalar plus ST_Envelope_Agg and ST_Union_Agg). So moving the base to
the aggregate form does not delete an override, it relocates one: minus SedonaDB, plus
DuckDB. Net count unchanged.

2. ST_Collect_Agg lands in Sedona Spark 1.8.1, not earlier. Measured, one Spark
3.5.3 session per version:

Sedona Spark	ST_Collect(ARRAY_AGG(...))	ST_Collect_Agg(...)
1.7.2	works	UNRESOLVED_ROUTINE
1.8.0	works	UNRESOLVED_ROUTINE
1.8.1	works	works
1.9.1	works	works

The base class is the documented Spark/Sedona dialect and surfaces in docs/queries.md
and the notebooks, but nothing executes it — get_sql_queries() maps only duckdb and
sedonadb, and there is no Spark runner in run_benchmark.py. So the change buys no
measured benefit and would narrow the documented reference query to Sedona >= 1.8.1, in a
repo that pins no Sedona version. That trade is not worth it.

Everything else here did check out — at SF1 and SF10 both spellings are bit-identical in
Sedona Spark 1.9.1 and match benchmark/answers/sf{1,10}/q5.csv — but "same answer,
narrower compatibility, same override count" is not a good enough reason to touch it.

One finding worth separating out, unrelated to the spelling debate: docs/queries.md,
docs/queries.zh.md and notebooks/queries.ipynb show Q5 as
ST_Collect(ST_GeomFromWKB(t.t_dropoffloc)) inside an sd.sql(...) call, and SedonaDB
0.4.1 rejects that outright with Invalid function 'st_collect' — the documented query
cannot be run as written. That looks like a real docs bug and I am happy to send a
docs-only fix if maintainers want one.

[james-willis]

Superseded by #142, which now carries this change together with the documentation fixes. The two touch the same three docs files, so keeping them separate guaranteed a conflict. Leaving this closed; the history lives in #142.