Make the grouped vectorized aggregate parallel-aware (#349) - #366
Merged
Conversation
The grouped vectorized node was serial by construction -- parallel_aware false, parallel_workers 0, and no DSM callbacks on columnar_groupagg_exec_methods -- so whenever it won it replaced a four-worker plan with a single-threaded one. #350 made the *choice* honest by charging the node for the folding it does; this makes the node itself parallel, so it wins on merit rather than needing to be priced to win. It is the #343/#346 ungrouped machinery with one structural difference: the ungrouped partial node emits one tuple per worker under an AGG_PLAIN Finalize, while the grouped one emits one tuple per group per worker under an AGG_HASHED / AGGSPLIT_FINAL_DESERIAL Finalize keyed on the grouping columns. Each worker claims distinct row groups through the same gap-23 counter, builds its own hash table over them, and emits (group keys, transition states). The per-aggregate transition state is unchanged, so combine and overflow parity carry over untouched. The partial path reuses core's UPPERREL_PARTIAL_GROUP_AGG target rather than building one, so the partial and final aggregates are structurally related and setrefs matches them up. The output map is rebuilt against that target, since its column order and aggregate split both differ from output_rel's. The serial node is still offered when the parallel arm is added, unlike the ungrouped arm which drops its serial node. That asymmetry is deliberate: #133 priced the ungrouped serial node at the cheap Gather cost so keeping it would out-cost a genuinely parallel plan, while #350 gave this node an honest per-row charge. Suppressing it here costs real time -- on a ten-aggregate windowed shape the parallel arm loses to core's own plan by a hair, and with the serial node gone the result is 8,945 ms against 7,493. Offering both makes enabling the GUC a strict addition to the planner's choices. Measured, 20M-row TSBS-shaped fixture, PG18 assert, 4 workers: shape core serial parallel G1 (1 metric, 12h) 6,144 ms 4,555 ms 4,587 ms (serial chosen) G2 (10 metrics, 12h) 12,410 ms 9,225 ms 7,493 ms (serial chosen) G3 (full scan, by host) 892 ms 896 ms 600 ms (parallel chosen) G3 takes the new path and wins. G1 does not, and not because the node is slow: forced onto G1 it runs in 1,184 ms against the serial node's 4,555. The core Finalize on top of our partial node is priced off dNumGroups, and estimate_num_groups cannot estimate a date_trunc() key -- 200,000 estimated against ~8,000 actual -- so the finalize is overpriced by that ratio and loses the comparison on its own. The serial node emits finished values and pays no finalize at all, so an inflated group estimate makes it win by construction. Confirmed by G3, whose plain-column key estimates accurately and where the parallel arm is chosen. The costing asymmetry is recorded in the design note as the follow-up; it changes plan choice on shapes beyond this one and deserves its own measurement. Both GUCs remain off by default, so no default behaviour changes. test/parallel_vector_agg.sh gains eleven grouped checks. Their premises are asserted against one EXPLAIN ANALYZE and each names a property core's own parallel grouped plan does not have, because checking merely for "a Gather" or "workers launched" passes on unmodified main -- both did until tightened. Proven by removal: three premise checks fail on pristine main while the value checks pass there. Also corrects a claim rather than leaving it: the leader-side flush in the new InitializeDSM is documented as defensive, not load-bearing. Removing it and the ungrouped one it mirrors leaves both H2 checks green under in-transaction INSERT and DELETE with a confirmed parallel plan, because the buffers are already flushed at the command boundary. It is kept for symmetry and cheapness; no test covers its removal and none now claims to. Gate: full suite matrix PG18 + PG19 assert, 112 suites each; ASAN+UBSAN over the aggregate suites. The 100M bench re-measurement asked for in #349 is still outstanding -- those fixtures no longer exist on the bench host. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 4, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The remaining scope of #349. Its other items are closed: item 2 (scan-key pushdown)
in #354, the costing regression in #350, and the measurement phase in the issue's own
thread.
The grouped vectorized node was serial by construction --
parallel_aware = false,parallel_workers = 0, and no DSM callbacks oncolumnar_groupagg_exec_methods--so whenever it won it replaced a four-worker plan with a single-threaded one. #350
made the choice honest; this makes the node itself parallel, so it wins on merit
rather than needing to be priced to win.
It is the #343/#346 machinery with one structural difference: the ungrouped partial
node emits one tuple per worker under an
AGG_PLAINFinalize; the grouped oneemits one tuple per group per worker under
AGG_HASHED/AGGSPLIT_FINAL_DESERIALkeyed on the grouping columns. Each worker claims distinctrow groups through the same gap-23 counter and builds its own hash table. The
per-aggregate transition state is unchanged, so combine and overflow parity carry
over untouched.
Design decisions worth a reviewer's eye
UPPERREL_PARTIAL_GROUP_AGGtarget rather thanbuilding one, so partial and final aggregates are structurally related and setrefs
matches them up. The output map is rebuilt against that target, since its column
order and aggregate split both differ from
output_rel's.ungrouped arm which drops its serial node. Deliberate: pgcolumnar.enable_metadata_count is an orphaned GUC, and count(*) loses to a parallel scan by default #133 priced the ungrouped
serial node at the cheap Gather cost so keeping it would out-cost a genuinely
parallel plan, while fix: charge the grouped aggregate path for the folding it does (#349) #350 gave this node an honest per-row charge. Suppressing it
here costs real time -- see the numbers below.
Measured (20M-row TSBS-shaped fixture, PG18 assert, 4 workers)
G3 takes the new path and wins. G1 does not, and not because the node is slow:
forced onto G1 it runs in 1,184 ms against the serial node's 4,555, a 3.9x win
the planner declines. The planner's own numbers:
Our partial node under its Gather costs 16,292 against the serial node's 20,042 -- it
wins on everything it does. The core Finalize adds 28,031, priced off
dNumGroups, andestimate_num_groupscannot estimate adate_trunc()key: 200,000estimated against ~8,000 actual. The finalize is overpriced by that ratio and loses
the comparison on its own.
The asymmetry is structural. The serial node emits finished values so it pays no
finalize at all, and #350 deliberately priced it per input row with no
per-output-group term. Any two-phase plan pays a group-count-driven finalize; the
serial node does not. Confirmed rather than guessed by G3, whose plain-column key
estimates accurately and where the parallel arm is chosen and is faster. Accurate
estimate -> chosen; inflated estimate -> declined.
That is why the serial node is not suppressed: doing so makes G1 win 4x, but on G2
the parallel arm loses to core by a hair and, with the serial node gone, the result
is 8,945 ms against 7,493. Offering both makes enabling the GUC a strict addition to
the planner's choices.
Follow-up this needs is the costing asymmetry, not more execution work: either
charge the serial node for the group hash table it builds, or cost the finalize off
something less brittle than
estimate_num_groupson an expression key. Both changeplan choice beyond this shape and deserve their own measurement.
Tests
test/parallel_vector_agg.shgains eleven grouped checks: plan premises, integeraggregates against a serial oracle, float against core's parallel aggregate
(reassociation), multi-key + WHERE, few-groups-many-workers, and in-transaction
deletes.
Premises are asserted against one
EXPLAIN ANALYZE, and each names a property core'sown parallel grouped plan does not have. Checking merely for "a Gather" or
"workers launched" passes on unmodified main, because core plans
Finalize GroupAggregate -> Gather Merge -> Sortover the same table -- both of myfirst-draft checks did exactly that until tightened.
Removal proof: three premise checks fail on pristine main while the value checks
pass there, so the premise checks are what detect the feature.
A claim I could not prove, corrected rather than left standing
I added a leader-side flush to the new
InitializeDSMand documented it as required.It is not. Removing it -- and the ungrouped one it mirrors -- leaves both H2 checks
green under in-transaction INSERT and DELETE with a confirmed parallel plan,
because the buffers are already flushed at the command boundary. The flush is kept
for symmetry and cheapness; the source and test comments now say it is defensive, that
no test covers its removal, and that none claims to.
Gate
ALL VERSIONS PASSED.parallel_vector_agg,ungrouped_vector_agg,native_groupagg,native_agg--DSM and cross-backend state is exactly what ASAN is for, and the parallel suites are
not in the default subset.
The 100M bench re-measurement asked for in #349 is outstanding: those fixtures no
longer exist on the bench host (
/root/bench-datais empty). This PR changes nodefault, so the issue's "measure before changing the default" bar is not crossed.
Design notes in
design/ISSUE_349_PARALLEL_GROUPAGG.md.🤖 Generated with Claude Code