From 7ea27dafc81efe9214be9c5f3e36bc38db398a9d Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Mon, 3 Aug 2026 07:12:53 -0600 Subject: [PATCH] fix: charge the grouped aggregate path for the folding it does (#349) ColumnarTryGroupAggPath priced itself at the cheapest non-index scan plus cpu_tuple_cost, and charged nothing for the folding it performs. Every competing plan pays a per-row aggregation cost; this one paid none, so it won by construction against anything -- including a parallel plan measured 1.9x faster than itself. Measured on the 100M-row TSBS bench, GROUP BY hostname over the full scan with 4000 groups: enabling the path took the query from 5,953 ms to 11,478 ms, because it replaced a four-worker Finalize GroupAggregate with a single serial node. Confirmed over three alternating runs (6134/5972/5985 against 11473/11616/11445) with near-identical buffers, so the cost is CPU, not I/O. The node is serial, so it cannot be priced low and left to win; it has to compete honestly. It folds vectors rather than advancing a row-wise Agg, which is cheaper per row, but it does that on every row without dividing the work across workers. Charging cpu_operator_cost per input row per aggregate, the same rate core charges a transition, expresses exactly that, and is conservative because the fold is cheaper than the advance it replaces. Against the planner's own estimates on that fixture, per row per aggregate: any charge above ~0.0013 correctly loses the full-scan case and any below ~0.0107 correctly wins the windowed ones. The default cpu_operator_cost of 0.0025 sits inside with margin at both ends, so this is not tuned to the fixture. Charging per input row rather than per output group also avoids the failure the previous comment records, where an earlier per-group charge let autoanalyze's n_distinct estimate flip the choice on large inputs so the node sometimes did not run at all. Per input row does not depend on n_distinct. It also makes the estimate respond to the number of aggregates, which it did not: ten aggregates were priced identically to one while costing 2.35x as much to run. Verified on the bench, plan choice and runtime together: shape chosen before after 1 metric, 12h window vectorized serial 3,604 ms 3,175 ms 10 metrics, 12h window vectorized serial 8,474 ms 6,842 ms full scan, 4000 groups core parallel 11,478 ms 5,380 ms The regression is gone and both windowed shapes keep their wins. test/native_groupagg.sh gains the property that was observably wrong and that does not depend on the planner choosing any particular plan at fixture scale: the estimate must respond to how much folding the node will do. Removal proof: with the charge reverted, one aggregate and ten are both priced at 20601 and the check fails; with it, 25601 against 70601. This does not make the node parallel-aware, which remains the larger win -- it stops the node from winning plans it should lose. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_011miCFRSatixeNRw3w5yNq8 --- src/columnar_vector.c | 49 ++++++++++++++++++++++++++++++++--------- test/native_groupagg.sh | 36 ++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 11 deletions(-) diff --git a/src/columnar_vector.c b/src/columnar_vector.c index 4a9b991..0d4d6cd 100644 --- a/src/columnar_vector.c +++ b/src/columnar_vector.c @@ -1371,19 +1371,46 @@ ColumnarTryGroupAggPath(PlannerInfo *root, RelOptInfo *input_rel, cpath->path.rows = (dNumGroups < 1.0) ? 1.0 : dNumGroups; /* - * Price just above the scan the reader already does. The fold is one hash - * probe per row inside that scan, always cheaper than a separate Agg node's - * per-row advance over the same scan, so a negligible fixed bump keeps this - * reliably below the ordinary Agg-over-scan plan whenever the feature is - * enabled. This is an opt-in accelerator: when it is on it should be the - * plan, not a coin-flip against a HashAggregate whose cost is close. An - * earlier version charged per output group, which let autoanalyze flip the - * choice on large inputs -- so the node sometimes did not run at all. One row - * per group comes out only after the whole scan is folded, so there is no - * cheap partial start-up. + * Price the scan this node performs, plus the folding it does over that + * scan. + * + * Charging only the scan, as this did, makes the node unpriceable-against: + * every competing plan pays a per-row aggregation cost and this one paid + * none, so it won by construction against anything, including a parallel + * plan several times faster than itself. That is not a conservative bias, it + * is a blind spot -- and it cost ~1.9x on a full-scan GROUP BY with few + * groups, where the four-worker plan this displaced was the better one + * (issue #349). + * + * The node is serial, so it cannot simply be priced low and left to win. It + * has to compete honestly: it folds vectors instead of advancing a row-wise + * Agg, which is cheaper per row, but it does that work on every row without + * dividing it across workers. Charging cpu_operator_cost per row per + * aggregate -- the same rate core charges a transition -- expresses exactly + * that. It is conservative, since the fold is cheaper per row than the + * row-wise advance it replaces, and it leaves the node ahead wherever it + * actually is ahead. + * + * Measured against the planner's own numbers on a 100M-row TSBS fixture, per + * row per aggregate: any charge above ~0.0013 correctly loses the full-scan + * case, and any charge below ~0.0107 correctly wins the windowed ones. The + * default cpu_operator_cost of 0.0025 sits inside that range with margin at + * both ends, so this is not tuned to the fixture. + * + * Charging per input row also makes the estimate respond to the number of + * aggregates, which the previous cost did not: ten aggregates were priced + * identically to one, while costing 2.35x as much to run. + * + * An earlier version charged per output GROUP, which let autoanalyze's group + * estimate flip the choice on large inputs so the node sometimes did not run + * at all. Per input row avoids that: it does not depend on n_distinct. + * + * One row per group comes out only after the whole scan is folded, so there + * is no cheap partial start-up. */ { - Cost cost = cheapest->total_cost + cpu_tuple_cost; + Cost cost = cheapest->total_cost + cpu_tuple_cost + + cpu_operator_cost * cheapest->rows * (naggs > 0 ? naggs : 1); cpath->path.startup_cost = cost; cpath->path.total_cost = cost; diff --git a/test/native_groupagg.sh b/test/native_groupagg.sh index 74b55e5..fab035e 100644 --- a/test/native_groupagg.sh +++ b/test/native_groupagg.sh @@ -336,4 +336,40 @@ psql_run "CREATE TABLE t_empty (k text, v int) USING pgcolumnar;" check "empty table: grouped scan yields 0 rows" \ "$(q "SELECT count(*) FROM (SELECT k, count(*) FROM t_empty GROUP BY k) s")" 0 +# ---- 9. the path pays for the folding it does (#349) ----------------------- + +# The grouped path used to price itself at the scan cost plus a fixed bump, and +# charged nothing for folding. Every competing plan pays a per-row aggregation +# cost and this one paid none, so it won by construction -- including against a +# parallel plan measured 1.9x faster than itself on a full-scan GROUP BY. +# +# The property asserted here is the one that was observably wrong and that does +# not depend on the planner picking any particular plan at fixture scale: the +# estimate must respond to how much folding the node will do. Ten aggregates +# over the same rows must not be priced the same as one. +psql_run "DROP TABLE IF EXISTS t_cost; + CREATE TABLE t_cost (host text, a int, b int, c int, d int, e int, + f int, g2 int, h int, i int, j int) + USING pgcolumnar; + INSERT INTO t_cost + SELECT 'h' || (n % 50), n, n, n, n, n, n, n, n, n, n + FROM generate_series(1, 20000) n; + ANALYZE t_cost;" >/dev/null + +# cost of the Custom Scan top node, with the grouped path forced on +gcost() { # gcost + env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres \ + -d "$PGC_DB" -Atq \ + -c "SET pgcolumnar.enable_group_vectorization=on" \ + -c "EXPLAIN (COSTS ON) SELECT host, $1 FROM t_cost GROUP BY host" 2>&1 | + head -1 | grep -oE '\.\.[0-9]+\.[0-9]+' | tr -d '.' | head -1 +} +C1="$(gcost 'avg(a)')" +C10="$(gcost 'avg(a),avg(b),avg(c),avg(d),avg(e),avg(f),avg(g2),avg(h),avg(i),avg(j)')" +echo " grouped path cost: 1 aggregate $C1, 10 aggregates $C10" +check "premise: the grouped path is costed at all (non-empty estimate)" \ + "$( [ -n "$C1" ] && [ -n "$C10" ] && echo yes || echo no )" yes +check "ten aggregates cost more than one (#349)" \ + "$( [ "$C10" -gt "$C1" ] 2>/dev/null && echo yes || echo no )" yes + pgc_summary