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