Grouped vectorized aggregate (#289)

[ChronicallyJD]

Grouped vectorized aggregate (#289)

The existing vectorized aggregate fires only for a plain, ungrouped, unfiltered aggregate, which it answers from zone-map metadata. The moment a query adds GROUP BY (the TSBS double-groupby shape, q4/q5) it falls back to a scalar HashAggregate over the row-at-a-time scan. This adds the grouped sibling: a scanrelid==0 custom path that groups and aggregates inside one pass over the columnar reader.

What it does

Fires for SELECT <keys>, agg(col) … [WHERE …] GROUP BY <keys> over a single columnar relation when:

• every GROUP BY key is non-volatile, computable from this relation, has a hash function and an equality operator, and — if collatable — uses a deterministic collation;
• every output entry is either a supported aggregate or a bare reference to a group key (an output built on a key falls back);
• the estimated group count is within pgcolumnar.groupagg_max_groups.

Anything else adds no path and the ordinary Agg plan runs, so results are never at risk.

Each surviving row from ColumnarReadNextRow (WHERE pushed down for group/vector skipping) is rechecked against the full WHERE, its keys evaluated, and scattered into an open-addressing hash table. Grouping uses each key type's own hash and equality functions — not memcmp — so -0.0/NaN, numeric scale, and deterministic-collation text group exactly as core does. Per-group accumulators reuse the existing columnar_apply_one and fold in scan order, so results are byte-identical to the scalar Agg the planner would otherwise choose.

Extends the aggregate accumulators to sum/avg over int8/float/numeric (the ungrouped path still rejects these, unchanged).

Opt-in

Gated by pgcolumnar.enable_group_vectorization (default off). While off, planning and execution are unchanged. When on, it is priced to be chosen over Agg-over-scan (an accelerator you turn on), costed per output group rather than per input row.

Correctness — test/native_groupagg.sh (registered in the matrix)

• Heap-mirror oracle — exact aggregates (count, integer/numeric sums, min/max) compare byte-exact; float sums/averages compare rounded (float summation order is the executor, not a defect).
• Toggle-differential — the same query with the path off vs on over the same columnar table; exact aggregates must be byte-identical, which is what validates the order-preserving accumulators.
• Plan assertions that the node is chosen when supported and that over-cap cardinality, non-deterministic collation, an output built on a key, and a keys-only GROUP BY all fall back while still returning the oracle's answer.
• NULL keys, deletes, ADD COLUMN, a WHERE that both prunes groups and needs a residual recheck, and empty input.

Gate

• Full PG 18 + PG 19 assert matrix: ALL VERSIONS PASSED (224 suite runs, 0 failures; native_groupagg green on both).
• Preflight compile on all five majors (15–19): clean.
• Sanitizer gate (ASAN+UBSAN, pg18_san): native_groupagg passes — no leak / use-after-free / alignment fault in the new hash-table and memory-context code.

Performance

Interleaved A/B on the TSBS cpu set (100M rows, PG18 non-assert, serial, forced columnar scan), grouped path off vs on, warm median over 4 rounds. Both arms return the identical 48,000 groups:

query	off (scalar Agg)	on (grouped node)	speedup
q4 double-groupby-1 (1 avg)	14883 ms	12428 ms	1.20×
q5 double-groupby-all (10 avgs)	18342 ms	13290 ms	1.38×

q5 gains more: the wider the aggregate list, the more per-row nodeAgg plumbing the single-pass fold removes. This matches the honest estimate.

The larger ~4× lever for this shape is the follow-on: dictionary-coded grouping on the high-cardinality text key. This change is the executor foundation that makes that possible.

Cleanroom: order-preserving hand-written accumulators and an open-addressing hash over public PostgreSQL APIs; no core, TimescaleDB, Citus, or DuckDB source consulted.

🤖 Generated with Claude Code

[ChronicallyJD]

Review: do not merge yet. Two wrong-answer blockers, reproduced.

This is good work and the design is right: a real single-pass grouped path, opt-in,
with per-type hash and equality rather than memcmp, and a test suite that is
genuinely strong (more on that below). The problems are specific and fixable, but
two of them produce silently wrong answers with the GUC on.

Method: a 7-dimension adversarial code review (each finding then attacked by an
independent verifier trying to refute it), plus my own empirical probes on PG17
against the PR branch. Everything marked reproduced below I ran myself.

---

Blocker 1: sum(real) returns 0 (reproduced)

columnar_agg_finalize returns Float8GetDatum(spec->fsum) for
COLUMNAR_AGG_SUM_FLOAT unconditionally (src/columnar_vector.c:1335). But core's
sum(real) returns real, not double precision, so a float8 Datum is handed back
where the tuple expects float4.

Heap oracle vs the grouped path, same data, GUC on:

sum(f4)  heap: h0|188250  h1|187125  h2|187500  h3|187875
         col : h0|0       h1|0       h2|0       h3|0

Every group returns 0. avg(f4), sum(f8) and avg(f8) are fine, so it is
specifically the float4 return type. This also disproves the PR body's
"byte-identical to the scalar Agg".

Blocker 2: gating (pseudoconstant) WHERE clauses are silently dropped (reproduced)

extract_actual_clauses(..., false) at src/columnar_vector.c:998 excludes
pseudoconstant quals, and nothing re-adds them, so a one-time filter is lost:

SELECT host, count(*) FROM t WHERE (SELECT false) GROUP BY host;
  heap: (no rows)              -- correct, the gate is false
  col : h0|100 h1|100 h2|100   -- all rows counted, WHERE ignored

The query returns data it must not return. WHERE (SELECT true) is fine, so it is
the gating path specifically.

Blocker 3: the suite section that should have caught Blocker 1 is vacuous (reproduced)

Section 4, "heap oracle: averages and float sums, rounded"
(test/native_groupagg.sh:141), wraps every aggregate in round(...). That makes
the output an expression over an aggregate, which the path rejects, so the node
never fires for those two checks and both arms run the scalar Agg:

avg(f8)                      -> NODE
round(avg(f8)::numeric,6)    -> fallback     (same aggregate, only the wrapper differs)

$EXACT contains no avg and no float sum, so section 4 is the only coverage
for every average accumulator and every float-sum accumulator this PR adds, which
is precisely the new work in the "extends the accumulators" line.

Proven a second way. I injected a silent defect into the grouped fold (drop one row
in 500) and re-ran the suite:

• 17 of 31 checks went red -- the suite is genuinely powerful for what it covers;
• oracle rounded avg/float: GROUP BY host, ... GROUP BY hour, host, and
  empty table: grouped scan yields 0 rows stayed green under a defect that
  corrupts every group.

Fix: compare the aggregates bare and round in the harness, and assert
pgc_is_groupvec = yes for those queries.

---

The cap: documented behaviour does not match the code (reproduced)

The GUC description reads "Plan-time cap on the estimated group count the grouped
vectorized aggregate will accept before falling back." Both halves are wrong.
With groupagg_max_groups=100 against 20,000 actual groups:

	measured
plan	node is chosen (estimate 20000 > cap 100)
run	ERROR mentioning groupagg_max_groups

So it is enforced at execution, not plan time, and it errors rather than falling
back. The suite's own comment is the accurate description; the GUC string and the
PR body are not. Default is 1,000,000 so the practical risk is low, but a DBA
reading pg_settings would expect a silent fallback and get a failed query.

---

Other findings from the review (not individually reproduced by me)

Each survived an independent refutation attempt. Flagging them as reported rather
than verified, since I only ran the ones above.

Correctness

• src/columnar_vector.c:462 -- stripping RelabelType from a GROUP BY key discards
  the cast's result type and collation. This also defeats the
  deterministic-collation check, so an explicit COLLATE with a nondeterministic
  collation can group wrongly. Two dimensions found this independently.
• src/columnar_vector.c:1209 -- float sum/avg silently overflow to Infinity where
  core raises "value out of range: overflow".
• src/columnar_vector.c:2004 / :2011 -- WHERE clauses referencing system columns or
  whole-row Vars are neither rejected nor projected.
• src/columnar_vector.c:908 -- a legacy inheritance parent is accepted and only the
  parent's own storage is scanned, dropping every child row.
• src/columnar_vector.c:1256 -- min/max keep the first value on a tie; core's
  *_larger/*_smaller keep the last. Visible for numeric 1.0 vs 1.00. I tried to
  reproduce this and could not, because the ::text I used to expose dscale makes the
  query fall back. Worth checking directly.

Memory (these bound how large a scan can get before it hurts)

• src/columnar_vector.c:1232 -- the numeric/int8 sum and avg accumulators allocate one
  or two numerics per scanned row into specContext, which is not reset until end
  of scan. That is O(rows), not O(groups), on exactly the 100M-row shape this feature
  targets.
• src/columnar_vector.c:2249 -- group-key expressions use ExecEvalExpr instead of
  ExecEvalExprSwitchContext, leaking one allocation per row into es_query_cxt.
• src/columnar_vector.c:2077 -- table growth is a single palloc, so it hits the
  internal alloc-size limit before the 1<<30 ceiling.

Planner / EXPLAIN

• src/columnar_vector.c:1023 -- the path is priced from cheapest_total_path, which can
  be an index scan, but always executes a full columnar scan.
• src/columnar_vector.c:1947 -- plain EXPLAIN always reports
  Columnar Pushed-Down Filters: 0 and shows neither the filter nor the group keys.

Test suite

• The 15 oracle and toggle comparisons never assert their own premise: none verifies
  the grouped node was chosen for that query shape. I checked, and all the $EXACT
  ones do fire today -- but nothing keeps them firing, which is how section 4 rotted.
• test/native_groupagg.sh:218 -- the non-deterministic-collation answer check cannot
  fail: the fixture has no keys differing only by case.
• test/native_groupagg.sh:232 -- "GROUP BY with no aggregate falls back" is
  short-circuited earlier than the guard it is credited with covering.

---

What is good, and worth saying

• The plan assertions use a marker line unique to this node, and the comment explains
  that this is proof of the node rather than an absence test a fallback would also
  pass. That is the right instinct and it is why the vacuity is confined to section 4.
• The pipefail note at line 196 catches a trap that would have turned the over-cap
  check into a false negative.
• The GUC really is default off, so none of this is live today.
• The single-pass design, per-type hash/equality, and the toggle-differential oracle
  are all sound.

To merge

Blockers 1, 2 and 3 need fixing, and the cap's description needs to match its
behaviour. The RelabelType and per-row allocation findings I would want addressed
before this is ever turned on by default, but they need not block the merge of an
opt-in path if you would rather land the foundation and follow up.

Happy to be wrong on any of the not-reproduced items -- push back with a repro and I
will retract.

[ChronicallyJD]

All findings addressed — thank you, this was a genuinely good review

The vacuous section 4 is the miss I most want to own: the suite looked thorough at 31 checks while giving zero real coverage to every average and float-sum accumulator, and it hid a wrong-answer bug. That's exactly the failure the rest of the suite was built to avoid. Fixed at the root, not just patched.

Pushed as 387fe73. Point by point:

Blockers (all reproduced, all fixed)

• sum(real) returned 0. It now accumulates in real (matching float4pl step for step) and returns real; sum(double precision) and the avg running sum go through overflow-checked helpers so a finite+finite → ∞ raises "value out of range: overflow" like core, instead of carrying an Infinity the scalar Agg never would. Named regression regress B1 asserts the node fires, the result matches heap, and it is nonzero.
• Gating (pseudoconstant) WHERE dropped. The per-row residual recheck can't honor a one-time filter, so the path now falls back when baserestrictinfo holds any pseudoconstant clause. regress B2 checks WHERE (SELECT false) returns no rows and the node falls back.
• Vacuous section 4. Rewritten: the float/avg accumulators are now covered by toggle-differentials of the bare aggregates, and toggle_diff/oracle both assert pgc_is_groupvec = yes — the premise no comparison used to state. Your injected-defect method was the right test of the test; every oracle/toggle now states its premise.

Cap description

Rewritten to say it is an execution-time cap on the actual group count that errors (with a hint), matching the code rather than the old "plan-time … falling back".

Before-default-on and the reported findings — also fixed

• RelabelType no longer stripped from a group key, so an explicit COLLATE keeps its collation and the determinism check is honest (output-matching stays consistent — neither side strips now).
• O(rows) numeric memory → O(groups): the running sum and each per-row int8→numeric intermediate are freed.
• Group keys evaluate in the per-tuple context (no per-row leak); the table uses a huge zeroed allocation so it can reach its ceiling; the path is costed from a full columnar scan, not a cheaper index path; EXPLAIN reports the real pushed-down filter count; min/max keep the later value on a tie (numeric 1.0 vs 1.00) matching core's larger/smaller; a WHERE on a system/whole-row column and a legacy inheritance parent fall back instead of scanning wrong data.

Adversarial re-verification caught four more edge cases (also fixed)

Before claiming done I ran the fixes back through an adversarial pass. It surfaced four things worth having, now fixed and covered:

• avg(double precision) overflow parity. Core's float8_accum keeps the Youngs-Cramer Sxx and raises overflow when either the sum or Sxx goes finite→∞; my simple running-sum check missed the Sxx case (avg(-1e308, 1e308) → core errors, I returned 0). Now tracks Sxx purely to reproduce the error; the returned average is still Sx/N.
• Signed zero. sum of a lone -0.0 printed 0, not -0: core assigns the first value directly, I folded it into +0.0. Now the first value is assigned directly.
• A latent (unreachable) rescan hazard: the ungrouped ReScanAggScan didn't clear the new float/numeric accumulator fields; it can't reach them today (that path rejects the extended kinds) but I reset them anyway.
• The non-deterministic-collation fixture was still vacuous — g%20 tied case to suffix so nothing case-folded. Rewritten to decouple them (20 byte-distinct keys → 10 case-folded).

Re-gate

• native_groupagg.sh 31 → 62 checks, incl. named regressions for both wrong-answer blockers, the signed-zero and avg-overflow parity cases, and a numeric display-scale tie; every oracle/toggle asserts the node fires.
• PG18 + PG19 assert matrix green (ALL VERSIONS PASSED, 0 fail); ASan+UBSan clean (the memory fixes' surface).

The EXPLAIN-shows-the-filter-expressions nicety I left as the count only; say the word if you'd like the clause text too. Anything I marked fixed that you can still break, send the repro and I'll chase it.

[ChronicallyJD]

Merged. I re-verified rather than took the summary, using the same reproductions
that found the blockers.

Blocker 1, sum(real). Fixed, and fixed properly rather than by falling back:

sum(f4)  plan=NODE  MATCHES heap      (was: heap 188250 / col 0)
avg(f4)  plan=NODE  MATCHES heap
sum(f8)  plan=NODE  MATCHES heap
avg(f8)  plan=NODE  MATCHES heap

Blocker 2, gating WHERE. WHERE (SELECT false) now falls back and matches the
heap oracle. WHERE (SELECT true) falls back too, which is the conservative
reading of "any pseudoconstant clause" and is the right call.

Blocker 3, the vacuous section. This is the one I most wanted to check, since a
fix here is easy to claim and hard to see. I re-ran the same injected defect (drop
one row in 500 from the grouped fold) against the new suite:

	old suite	new suite
checks	31	62
failures under the injected defect	17	21
avg/float checks catching it	0	toggle float/avg x3, plus regress B1

The three toggle float/avg checks and the named regress B1 all go red now, where
the old oracle rounded avg/float pair stayed green under a defect that corrupted
every group. The coverage is real.

The four you caught yourself. All match heap with the node firing:

sum(f8) of a lone -0.0            plan=NODE  MATCHES
avg(v) of -1e308, 1e308           plan=NODE  MATCHES  (both ERROR: value out of range: overflow)
sum(v) of 1e308, 1e308            plan=NODE  MATCHES  (both ERROR)
min(n), max(n) on 1.0 vs 1.00     plan=NODE  MATCHES

Worth noting how nearly I missed verifying these: my first probe wrapped the
aggregates in ::text to expose display scale, which is exactly the shape that
disables the path. All four came back "MATCHES" with plan=fallback — the same
vacuity, reproduced in my own check an hour after reviewing yours for it. Casting
the whole row outside the grouped query fixed it.

Finding the Sxx overflow case in avg(float8) before anyone asked is the part of
your adversarial pass I'd call out: avg(-1e308, 1e308) erroring in core and
returning 0 here is not a case a reviewer would have constructed.

CI green on all 11 checks. #289 stays open for the remaining lever.