fix: charge the grouped aggregate path for the folding it does (#349)

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;

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 <select-list>
+	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