From 76c1615bf47bcf0aa0c4f5e287430a48009ebbc8 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Mon, 27 Jul 2026 10:53:15 -0600 Subject: [PATCH] Cost the columnar scan properly when no seqscan path survives (#171) The custom scan inherits the sequential scan's cost, and when there was no seqscan path to inherit from it fell back to rel->rows -- an output row count used as a cost. That fallback was not the rare case it looks like. add_path frees a path it finds dominated, so the seqscan is already gone from rel->pathlist by the time this hook runs exactly when some index path beat it on both cost and pathkeys: precisely the selective lookups where using the index matters most. Before ANALYZE, the row estimate was a large default and the resulting "cost" was accidentally large enough to lose. Once ANALYZE supplied real statistics (#159), a selective predicate estimated one row, so a full scan of the table was priced at 1.00, beat an index scan of the same query costed at 174.29, and the planner stopped using the index. A point lookup went from 23.75 ms to 1251.88 ms the moment the table had statistics. The fallback now costs the work it actually does: every page read once and the restriction evaluated on every row, which is core's own seqscan formula. Where a seqscan path does survive, nothing changes. Two checks in test/analyze_stats.sh, both behavioural rather than assertions about a cost number. The second compares the same query against itself with the index denied, so it stays discriminating whatever the row group size is; a fixed row threshold would only separate the two cases for as long as a group happened to be larger than it. With the fix reverted both fail and nothing else does: the plan reads Custom Scan, and the index saves no work at all (9999 rows discarded either way). Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_012uKWWwBDt5TWWS5DR2tzDb --- src/columnar_customscan.c | 41 +++++++++++++++++++++++++++++++++-- test/analyze_stats.sh | 45 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 2 deletions(-) diff --git a/src/columnar_customscan.c b/src/columnar_customscan.c index 4ebcad7..b549fac 100644 --- a/src/columnar_customscan.c +++ b/src/columnar_customscan.c @@ -555,8 +555,45 @@ ColumnarSetRelPathlist(PlannerInfo *root, RelOptInfo *rel, Index rti, cpath->path.parallel_safe = false; cpath->path.parallel_workers = 0; cpath->path.rows = rel->rows; - cpath->path.startup_cost = seqpath ? seqpath->startup_cost : 0; - cpath->path.total_cost = seqpath ? seqpath->total_cost : rel->rows; + + /* + * Inherit the sequential scan's cost when there is one, since this path + * reads the same relation and the comparison against every other path + * should turn on what it does differently, not on a different cost model. + * + * When there is none, cost the work: every page read once and the + * restriction evaluated on every row. The fallback used to be rel->rows, + * which is an output row count and not a cost at all. + * + * That fallback was not the rare case it looks like. add_path frees a path + * it finds dominated, so the seqscan is gone from rel->pathlist by the time + * this hook runs exactly when some index path beat it on both cost and + * pathkeys -- which is to say, precisely on the selective lookups where + * using the index matters most. Before ANALYZE the row estimate was a large + * default and the resulting "cost" was accidentally large enough to lose. Once + * ANALYZE supplied real statistics (#159), a selective predicate estimated + * one row, so a full scan of the table was priced at 1.00, beat an index scan + * of the same query costed at 174.29, and the planner stopped using the index. + * 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.pathkeys = NIL; cpath->flags = 0; cpath->custom_paths = NIL; diff --git a/test/analyze_stats.sh b/test/analyze_stats.sh index 0381c29..567ae16 100755 --- a/test/analyze_stats.sh +++ b/test/analyze_stats.sh @@ -216,4 +216,49 @@ check "a selective equality is estimated from the data, not the 0.5% default" \ 'BEGIN { r = e / t; print (r > 0.2 && r < 0.5) ? "yes" : "no (" e " of " t ")" }')" \ "yes" +# --- 5. statistics must not cost the index out of a point lookup (#171) -------- + +# Collecting statistics made one query shape dramatically worse. The custom scan +# inherits the seqscan's cost, but add_path frees a dominated path, so when an +# index path beats the seqscan there is no seqscan left to inherit from -- and the +# fallback was rel->rows, an output row count used as a cost. With real statistics +# a selective predicate estimates one row, so a full scan was priced at 1.00 and +# won. Measured on a 6M-row table: 23.75 ms before ANALYZE, 1251.88 ms after. +# +# Both checks below are behavioural rather than assertions about a cost number, +# so neither can be satisfied by a differently-shaped wrong cost. + +psql_run "CREATE INDEX IF NOT EXISTS as_c_id ON as_c (id); ANALYZE as_c;" >/dev/null + +target=$((ROWS / 2)) +plan="$(q "EXPLAIN (COSTS off) SELECT * FROM as_c WHERE id = $target;")" +echo "-- point-lookup plan after ANALYZE: $(printf '%s' "$plan" | head -1)" + +check "a point lookup on an indexed column still uses the index after ANALYZE" \ + "$( grep -qE 'Index (Only )?Scan|Bitmap Heap Scan' <<<"$plan" \ + && echo yes || echo "no ($(printf '%s' "$plan" | head -1))")" \ + "yes" + +# The consequence, independent of plan shape: having the index available must +# actually save work. The comparison is against the same query with the index +# denied rather than against a fixed number of rows, so it stays discriminating +# whatever the row group size is -- a fixed threshold would only separate the two +# for as long as a group happens to be larger than it. +discarded() { # extra SET statements -> rows the filter threw away + local ea + ea="$(q "$1 EXPLAIN (ANALYZE, COSTS off, TIMING off, SUMMARY off) + SELECT * FROM as_c WHERE id = $target;")" + awk 'match($0, /Rows Removed by Filter: [0-9]+/) { + s = substr($0, RSTART, RLENGTH); sub(/[^0-9]+/, "", s); t += s } END { print t + 0 }' <<<"$ea" +} + +with_index="$(discarded "")" +no_index="$(discarded "SET enable_indexscan = off; SET enable_bitmapscan = off;")" +echo "-- rows discarded to return one row: ${with_index} with the index, ${no_index} without it" + +check "having an index available saves the point lookup real work" \ + "$(awk -v a="$with_index" -v b="$no_index" \ + 'BEGIN { print (b > 0 && a < b / 2) ? "yes" : "no (" a " with the index, " b " without)" }')" \ + "yes" + pgc_summary