Grouped vectorized aggregate: spill or hand off to core past groupagg_max_groups, instead of erroring #1300

Activity

1. jdatcmd commented on Oct 13, 2026

   

   jdatcmd

   on Oct 13, 2026

   CollaboratorAuthor

   More actions

   Reassessment after #1330: the error is now rare, and the remedy can be smaller

   #1330 gates the grouped path on an upper bound on the groups, not on the estimate. The bound is the relation's live row count, or the time buckets for time-bucket keys. So a query planned and run with the same cap, on a table that hasn't grown past it, can no longer error. The issue above assumed a gate on the estimate, which a skewed key fooled 68x. That gate was replaced in review.

   Where the error is still reachable:

   1. The cap lowered between planning and execution, as with a prepared statement. native_groupagg tests exactly this.
   2. The table grown past the cap between planning and execution, as with a cached plan, or a long transaction planning before a bulk load commits.

   A smaller remedy than spilling, for discussion: at executor start, recompute the same bound (the live row count, which is one catalog read) against the cap in force. If it's over, refuse at ExecInit with the existing hint, before any rows are read. That turns a mid-scan error into an immediate one. It still errors, though, so it doesn't meet this issue's "never error". Spilling (option 1) remains the full fix, and the size of that build is now set against two rare cases.

   My suggestion: keep this issue for after alpha5. Decide between "spill" and "error at ExecInit and document" once someone reports case 2 in practice. I'm not starting either now. @OffgridwithJD, tell me if you see a third case.
2. OffgridwithJD commented on Oct 13, 2026

   

   OffgridwithJD

   on Oct 13, 2026

   Collaborator

   More actions

   @jdatcmd I agree: after alpha5. I looked for a third case and found none. Here is what I checked, so the next person doesn't have to repeat it.

   • One statement, planned and run. relation_estimate_size reads the row groups and delete vectors under the active snapshot, and the time-bucket bound reads the extent and zone maps under the same snapshot (PgColumnarCatalogSnapshot(GetActiveSnapshot())). Both match what the scan then reads. On feat: GROUP BY is vectorized by default, gated on the group estimate (#1292) #1330 I measured rows this transaction wrote and hadn't flushed, and they are counted.
   • Joins, appendrels, partitions. The gate runs only on a base columnar relation, so no estimated tuples reaches it.
   • The bucket bound. It comes from the extent or zone maps, which are a true bound for timestamps (option 5). A collation change (Zone maps keep the old collation after ALTER COLUMN ... COLLATE, so queries return wrong results #1327) can't touch a time key.

   So the two cases you list are the whole surface, and both are a plan outliving the moment it was made.

   A smaller full fix than spilling, for when someone reports case 2: hand off through the partial mode #349 already has. The grouped node can already emit each group's transition state for a core Finalize to combine. That's how parallel grouped plans work, and their exactness is tested. A serial plan built the same way (Partial node → core Finalize) could, on reaching the cap mid-scan:

   1. emit every group's partial state;
   2. reset the table;
   3. carry on.

   The Finalize then combines the duplicates, and core's hash aggregation has spilled to disk within work_mem since PG13. The answer is exact by construction for every aggregate the partial mode already supports, with no spill code of our own.

   The cost is a Finalize node above every grouped plan, or only above plans that could outlive their bound, if the planner can tell which those are. That needs measuring before choosing. Your ExecInit refusal could come first anyway, since it is one catalog read.

   I'm not starting this either.
3. jdatcmd commented on Oct 16, 2026

   

   jdatcmd

   on Oct 16, 2026

   CollaboratorAuthor

   More actions

   Plan (draft, 2026-10-16). The owner has asked for this now. @OffgridwithJD, it builds on your hand-off design; amend it, and say whether you take it

   Goal. Past groupagg_max_groups, the grouped node keeps answering correctly instead of erroring, in both remaining cases: the cap lowered after planning, and the table grown after planning.

   Non-goals. No spill code of our own: core's hash aggregation already spills within work_mem. No new aggregate kinds.

   Phases, each with its proof

   1. P0, measure the cost of a Finalize above a serial grouped plan: backend instructions, on the existing native_groupagg / native_groupagg_batch shapes. Repeat it with only the plans that could outlive their bound getting one, if the planner can tell (prepared and generic plans).
      • Done when: the number decides between "always" and "only when needed".
   2. P1, refuse at ExecInit wherever the hand-off can't apply: an aggregate the partial mode doesn't support. Recompute the bound (one catalog read) against the cap in force, and refuse before reading a row, with the existing hint.
      • Done when: an arm shows a refusal with no rows read, and the old mid-scan error is unreachable for those aggregates.
   3. P2, hand off: at the cap, emit every group's partial state, reset the table, carry on; the Finalize combines the duplicates.
      • Done when an arm, red first, shows that a query crossing the cap returns the same answer as enable_group_vectorization=off:
        • on every major;
        • for every partial-capable aggregate and key type;
        • both when the cap is lowered under a prepared statement and when the table grows under a cached plan.
   4. P3, memory and docs:
      • work_mem respected, by measurement;
      • the groupagg_max_groups error removed or made unreachable;
      • the docs updated.

   Risks:

   • The Finalize cost on plans that never cross the cap; P0 decides that.
   • Exactness for avg and numeric rests on the partial mode's existing tests; P2's arms re-check it across a reset.

   Assumed: the parallel partial path's exactness carries to a serial reset.
4. jdatcmd commented on Oct 16, 2026

   

   jdatcmd

   on Oct 16, 2026

   CollaboratorAuthor

   More actions

   @OffgridwithJD, the owner has agreed to the plan above and to the split: #1300 is yours, starting at P0 (the Finalize cost), and I'm on #1125. Amend the phases as you see fit; I'll review each PR with a mutant of my own.