Cost the columnar scan properly when no seqscan path survives (#171) - #173
Merged
Conversation
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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012uKWWwBDt5TWWS5DR2tzDb
This was referenced Jul 27, 2026
jdatcmd
pushed a commit
that referenced
this pull request
Jul 28, 2026
) Alpha gate 2 of 4. Audits docs/limitations.md against the current tree, corrects two sections that documented behaviour that has since been fixed, and adds the two missing pieces the pre-alpha review note named. Corrections (documenting a defect is not fixing it, and the reverse holds too -- a fixed defect must not stay listed as a limitation): - Constraints on the import path: removed the claim that a deferrable unique constraint is checked per row rather than deferred to commit. That was #168, fixed by the executor index-maintenance path (#180/#182); import_deferred.sh now asserts import_arrow and import_parquet defer correctly. - Planner statistics: removed the warning that ANALYZE can take a long time on wide tables, the SET STATISTICS 0 workaround, and the point-lookup plan regression. That was #171, both halves fixed (#173 planner, #175 sampler). Additions: - Release status: states what 1.0-dev means, what the extension is appropriate for today, and what hardening is gated before a first alpha (#214, #216, #217). Linked from README.md and docs/index.md. - Bulk load and import throughput: states the range by shape from #213 (0.67x to 8.83x, not a single multiplier), names encode_effort as the knob, and records that import has no overhead beyond the write path. The per-column numeric insert cost is deliberately held until its ablation lands, per the issue. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the first half of #171.
The defect
A point lookup on an indexed columnar table went from 23.75 ms to 1251.88 ms the moment the table had statistics. Collecting statistics, which #159 made work, made this query shape dramatically worse.
The custom scan path inherits the sequential scan's cost. 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 is not the rare case it looks like.
add_pathfrees a path it finds dominated, so the seqscan is already gone fromrel->pathlistby the time the 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. With real statistics a selective predicate estimates one row, so a full scan of the table was priced at
1.00:A seq scan of that same query costs
2513.00. The custom scan claimed1.00and won.The fix
The fallback now costs the work it actually does: every page read once, and the restriction evaluated on every row. That is core's own seqscan formula, which is why the two agree exactly where both can be observed. Where a seqscan path does survive, nothing changes.
After the fix, the same lookup plans
Index Scan using cz_id on cz (cost=0.42..8.44 rows=1).Tests
Two checks in
test/analyze_stats.sh, both behavioural rather than assertions about a cost number, so neither can be satisfied by a differently-shaped wrong cost.The second one compares the same query against itself with the index denied, rather than against a fixed number of rows. A fixed threshold would only separate the two cases for as long as a row group happened to be larger than it; this stays discriminating whatever the group size is.
Proven by removal. With the fix reverted, both fail and nothing else does:
With the fix:
Index Scan using as_c_id, and0 with the index, 9999 without it.Gate
The gate tree predates #170 landing; the two changes touch disjoint files (
columnar_customscan.chere,columnar_vector.cthere). I will run the full suite onmainafter this merges to cover the combination.Still open on #171
The second half -- ANALYZE itself taking tens of minutes on wide tables -- is not addressed here. It has not reproduced inside the benchmark and needs its own investigation. The issue stays open for it.