From 5e1d9a0fca8cf5f70f7e483c87f808255506c1ba Mon Sep 17 00:00:00 2001 From: "Joshua (D) Drake" Date: Tue, 4 Aug 2026 07:51:42 -0600 Subject: [PATCH] fix: bound the index-fetch penalty by a multiple of one scan (#376) The penalty prices a fetch as a row-group decode times the rows the path returns. That is right when the plan above consumes the whole path and unbounded when it stops early. On the 100M fixture it reached 502,598,685,066 against an un-penalized 2,427,872, which is 207,000x, so a consumer reading 3,998 rows of 100,000,000 still lost to a full scan and a sort: 44,058 ms for the plan taken against 769 ms for the plan refused. Bound rather than model it better, because the two cases cannot be told apart from this hook. Both have a leading-key correlation of about zero. What differs is which groups the fetched rows land in, and that is not knowable before the consumer exists. Measured both ways on the same query text: a consumer that reads every row makes the penalty right by 36x, 4,710 ms against 170,965, and one that stops early makes it wrong by 57x. Past some multiple of one scan the number stops carrying information the planner can use. A path priced above the scan already loses to it, so further inflation only harms a consumer that fractions the path. The bound keeps the direction and drops the part that only does damage. The multiple is empirical rather than derived, and sits inside a window both measurements agree on: the early-stopping case needs at least 1.45x off, and the read-everything case tolerates about 16,000x before it picks the wrong plan. Twenty is near the conservative end, so the penalty keeps steering where it was steering correctly. Measured after the change. q7 on the 100M fixture takes the skip scan and runs 760 to 833 ms where it ran 44,058. The read-everything shape is unchanged and still refuses the index. The five earlier plan-shape guards are unchanged. Also removes a duplicated cost model: the full-scan fallback existed in two places in this file, and the bound needs a third caller, so it is now one function. Co-Authored-By: Claude Opus 5 (1M context) --- src/columnar_customscan.c | 103 +++++++++++++++++++++++++++++++------- test/analyze_stats.sh | 30 +++++++++++ 2 files changed, 114 insertions(+), 19 deletions(-) diff --git a/src/columnar_customscan.c b/src/columnar_customscan.c index b5ac376..4414fb2 100644 --- a/src/columnar_customscan.c +++ b/src/columnar_customscan.c @@ -726,6 +726,40 @@ columnar_index_fetch_penalty(RelOptInfo *rel, double rows, double rho, return groups_decoded * decode_per_group; } +/* + * How far above one full scan a fetching index path may be priced (issue #376). + * See columnar_penalize_index_fetches for why this is a bound rather than a + * better model, and for the two measurements that fix the window it sits in. + */ +#define COLUMNAR_INDEX_FETCH_PENALTY_MAX_SCANS 20.0 + +/* + * columnar_full_scan_cost + * What one full scan of this relation costs. + * + * The seqscan's own number when core still has one, and the same work priced + * directly when it does not -- add_path frees the seqscan whenever an index + * path dominates it, which is exactly the selective queries. One definition, + * two callers: the columnar path's own cost and the bound on the fetch + * penalty below. + */ +static Cost +columnar_full_scan_cost(RelOptInfo *rel, Path *seqpath) +{ + QualCost qcost; + double ntuples; + Cost run; + + if (seqpath != NULL) + return seqpath->total_cost; + + qcost = rel->baserestrictcost; + ntuples = (rel->tuples >= 0) ? rel->tuples : rel->rows; + run = seq_page_cost * (double) rel->pages; + run += (cpu_tuple_cost + qcost.per_tuple) * ntuples; + return qcost.startup + run; +} + /* * columnar_path_order_cmp * Order two paths the way add_path keeps rel->pathlist ordered. @@ -784,10 +818,12 @@ columnar_path_order_cmp(const ListCell *a, const ListCell *b) * planner models by fractioning (total - startup). */ static void -columnar_penalize_index_fetches(RelOptInfo *rel, Index rti, Oid relid) +columnar_penalize_index_fetches(RelOptInfo *rel, Index rti, Oid relid, + Cost fullScanCost) { int nproj; double decodedWidth; + Cost cap; bool mutated = false; ListCell *lc; @@ -796,6 +832,39 @@ columnar_penalize_index_fetches(RelOptInfo *rel, Index rti, Oid relid) columnar_scan_decode_shape(rel, rti, relid, &nproj, &decodedWidth); + /* + * The penalty is bounded by a multiple of one full scan (issue #376). + * + * The model prices a fetch as a row-group decode and multiplies by the rows + * the path returns. That is right when the plan above consumes the whole + * path, and it over-counts without limit when the plan above stops early: a + * DISTINCT ON reads one row per group, and a skip scan over the index reads + * 3,998 rows of 100,000,000. The un-bounded penalty reached 502,598,685,066 + * on that query -- 207,000 times the un-penalized path -- so even the 1/25000 + * of it that the skip scan fractions to still lost to a full scan and a sort. + * Measured: 44,058 ms for the plan chosen, 769 ms for the one refused. + * + * Why bound rather than model it better: the two cases cannot be told apart + * from this hook. Both have a leading-key correlation of about zero. What + * differs is which groups the fetched rows land in, and that is not knowable + * before the consumer exists. Measured both ways on the same shape -- a + * consumer that reads every row makes the penalty right by 36x (4,710 ms + * against 170,965), and one that stops early makes it wrong by 57x. + * + * Past some multiple of a full scan the number stops carrying information a + * planner can use. Any path priced above the scan already loses to it, so + * further inflation only harms a consumer that fractions the path. The bound + * keeps the direction and drops the part that only does damage. + * + * The multiple is empirical, not derived. It is chosen to sit inside a window + * both measurements agree on: the early-stopping case needs the penalty cut by + * at least 1.45x, and the read-everything case tolerates roughly 16,000x + * before it picks the wrong plan. Twenty is near the conservative end of that + * window, so the penalty keeps steering where it was steering correctly. + * test/analyze_stats.sh pins both directions. + */ + cap = COLUMNAR_INDEX_FETCH_PENALTY_MAX_SCANS * fullScanCost; + foreach(lc, rel->pathlist) { Path *p = (Path *) lfirst(lc); @@ -819,6 +888,15 @@ columnar_penalize_index_fetches(RelOptInfo *rel, Index rti, Oid relid) } /* T_IndexOnlyScan and the custom scans do no heap fetch */ + /* never price a fetching path above the bound (issue #376) */ + if (add > 0.0 && cap > 0.0) + { + if (p->total_cost >= cap) + add = 0.0; + else if (p->total_cost + add > cap) + add = cap - p->total_cost; + } + if (add > 0.0) { p->total_cost += add; @@ -897,7 +975,8 @@ ColumnarSetRelPathlist(PlannerInfo *root, RelOptInfo *rel, Index rti, * doing this after the add_path calls meant the columnar path was judged * against index costs that had not yet been penalized, and freed. */ - columnar_penalize_index_fetches(rel, rti, rte->relid); + columnar_penalize_index_fetches(rel, rti, rte->relid, + columnar_full_scan_cost(rel, seqpath)); cpath = makeNode(CustomPath); cpath->path.pathtype = T_CustomScan; @@ -930,23 +1009,9 @@ ColumnarSetRelPathlist(PlannerInfo *root, RelOptInfo *rel, Index rti, * That is issue #171: a point lookup went from 23.75 ms to 1251.88 ms the * moment the table had statistics. Regression-tested in test/analyze_stats.sh. */ - if (seqpath != NULL) - { - cpath->path.startup_cost = seqpath->startup_cost; - cpath->path.total_cost = seqpath->total_cost; - } - else - { - QualCost qcost = rel->baserestrictcost; - double ntuples = (rel->tuples >= 0) ? rel->tuples : rel->rows; - Cost run; - - run = seq_page_cost * (double) rel->pages; - run += (cpu_tuple_cost + qcost.per_tuple) * ntuples; - - cpath->path.startup_cost = qcost.startup; - cpath->path.total_cost = qcost.startup + run; - } + cpath->path.startup_cost = (seqpath != NULL) ? seqpath->startup_cost + : rel->baserestrictcost.startup; + cpath->path.total_cost = columnar_full_scan_cost(rel, seqpath); cpath->path.pathkeys = NIL; cpath->flags = 0; cpath->custom_paths = NIL; diff --git a/test/analyze_stats.sh b/test/analyze_stats.sh index c5e8b98..6c3f5bb 100755 --- a/test/analyze_stats.sh +++ b/test/analyze_stats.sh @@ -482,4 +482,34 @@ check "a late column's wide decode prefix costs it off the index (#363)" \ || echo "no ($(printf '%s' "$plan_late" | head -1))")" \ "yes" +# --- 9. the penalty is bounded by a multiple of one scan (#376) ------------------ +# +# The checks above assert direction. This one asserts magnitude, because #376 is not +# a wrong direction but an unbounded one. +# +# The model prices a fetch as a row-group decode times the rows the path returns. +# That is right when the plan above reads the whole path, and unbounded when it stops +# early. On the 100M fixture the penalty reached 502,598,685,066 against an +# un-penalized 2,427,872 -- 207,000x. A consumer reading 3,998 rows of 100,000,000 +# still lost to a full scan and a sort: 44,058 ms taken against 769 ms refused. +# +# A LIMIT is the shape core can show. Ten rows cost ten fetches, which is far less +# than sorting the table, so the index is the right plan. Before the bound the +# penalty was large enough to refuse it even for ten rows. +plan_lim="$(plan_of "${ord_setup} EXPLAIN (COSTS off) + SELECT * FROM o355 ORDER BY scat LIMIT 10;")" +echo "-- ORDER BY scat LIMIT 10, penalty on: $(printf '%s' "$plan_lim" | grep -m1 -E 'Scan|Sort')" +check "a small LIMIT still reaches the index through the fetch penalty (#376)" \ + "$(grep -q 'Index Scan using o355_scat' <<<"$plan_lim" && echo yes \ + || echo "no ($(printf '%s' "$plan_lim" | head -1))")" \ + "yes" + +# and the same query without a LIMIT must still be refused the index, so the bound +# has not simply switched the penalty off +plan_nolim="$(plan_of "${ord_setup} EXPLAIN (COSTS off) SELECT * FROM o355 ORDER BY scat;")" +check "the bound does not disable the penalty for a full ordered read (#376)" \ + "$( grep -qE 'Sort' <<<"$plan_nolim" && ! grep -q 'Index Scan using o355_scat' <<<"$plan_nolim" \ + && echo yes || echo "no ($(printf '%s' "$plan_nolim" | head -1))")" \ + "yes" + pgc_summary