fix: a bigint column against an integer literal now prunes chunk groups (#477) - #478
Merged
Conversation
…ps (#477) Two columns, identical values, same operator, same layout: idi > 16000 7 chunk groups removed of 10 seq > 16000 0 seq > 16000::bigint 7 The only difference is that the bigint column met an int4 literal. columnar_reader.c dropped the scan key: /* avoid cross-type comparisons that our column cmp proc cannot do */ if (OidIsValid(key->sk_subtype) && key->sk_subtype != att->atttypid) continue; That was right about the proc and wrong about the remedy. btint8cmp handed an int4 Datum reads a value that is not there, so refusing to use it was correct -- but refusing meant no SkipPredicate was built at all, and every bigint column compared against an unadorned integer literal read every chunk group. That is what ordinary SQL looks like, not an edge case. btree opfamilies already carry the answer: integer_ops supplies btint84cmp for (int8, int4). The comparison proc is now resolved for the PAIR of types from the column's default opfamily, and where the family has none the key is still skipped -- the old behaviour, still correct, now the fallback rather than the rule. Two details ride with it. The equality branch compared cmp(const, min) and cmp(const, max), constant first. That is equivalent for a same-type proc and wrong for a cross-type one, whose argument order is part of its signature, so it is rewritten column-first as "skip if min > const or max < const". The bloom probe stays DISABLED for cross-type equality. The filter is built by hashing column-type values, so hashing an int4 constant with the int8 hash proc probes a slot the writer never set and skips a group that may hold the row. That is a wrong answer, where min/max pruning is only ever conservative -- the one place in this change whose failure mode is not merely a missed optimisation. EXPLAIN gave no way to see any of this. "Columnar Pushed-Down Filters" counts scan keys handed to the reader, not predicates able to exclude, so it read 1 while zero groups were skipped. That is also how it survived in our own gate. test/zonemap_cost.sh declares `seq bigint` and queries `WHERE <col> > 160000`, so its CORRELATED arm -- the one whose whole purpose is to prune -- removed zero chunk groups, and #460's cost discount was validated against it. The suite asserted a cost relation between two priced plans, which is true or false whether or not the pruning being priced occurs. It now asserts the physical premise first, from EXPLAIN ANALYZE's own counter: that arm removes 16 groups of 20, and the scattered control removes 0. Found while building the fixture for #461, which is blocked on this: a column that prunes nothing cannot demonstrate that a pruning column is under-credited. Proven by removal: restoring the cross-type refusal fails three native_skip checks and the new zonemap_cost premise, with seq back to 0 groups removed. Full 132-suite matrix green on PG18 and PG19. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013sY1RQR5MjZNUixA2um31r
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013sY1RQR5MjZNUixA2um31r
This was referenced Aug 7, 2026
Closed
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 #477. Found while building the fixture for #461, which is blocked on this.
The measurement
Two columns, identical values, same operator, same layout:
idi > 16000seq > 16000seq > 16000::bigintThe only difference is that the
bigintcolumn met anint4literal.Cause, and why the old code was half right
columnar_reader.cdropped the scan key:Right about the proc, wrong about the remedy.
btint8cmphanded anint4Datumreads a value that is not there, so refusing to use it was correct. But refusing
meant no skip predicate was built at all, and every
bigintcolumn comparedagainst a bare integer literal read every chunk group. That is what ordinary SQL
looks like.
btree opfamilies already carry the answer:
integer_opssuppliesbtint84cmpfor
(int8, int4). The proc is now resolved for the pair of types; where afamily has none, the key is still skipped, which is the old behaviour kept as the
fallback rather than the rule.
Two details that are not obvious
The equality branch had to be rewritten column-first. It compared
cmp(const, min)andcmp(const, max). That is equivalent for a same-type procand wrong for a cross-type one, whose argument order is part of its signature.
Now
skip if min > const or max < const.The bloom probe stays disabled for cross-type equality. The filter is built
by hashing column-type values, so hashing an
int4constant with theint8hashproc probes a slot the writer never set and would skip a group that holds the
row. That is a wrong answer, where min/max pruning is only ever conservative.
It is the one place in this change whose failure mode is not a missed
optimisation, which is why equality parity and an absent-value check are both
asserted.
It was invisible from EXPLAIN, and that is how it survived our own gate
Columnar Pushed-Down Filterscounts scan keys handed to the reader, notpredicates able to exclude anything, so it read
1while zero groups wereskipped.
test/zonemap_cost.shhas been in that state since it was written. Itdeclares
seq bigintand queriesWHERE <col> > 160000, so its correlatedarm -- the one whose entire purpose is to prune -- removed zero chunk groups.
#460's discount was validated against it. The suite asserted a cost relation
between two priced plans, and that is true or false whether or not the pruning it
prices actually happens.
It now asserts the physical premise first, from
EXPLAIN ANALYZE's own counter:#460 is not wrong to discount; the layout genuinely is prunable. Its evidence was.
Verification
native_skip.shzonemap_cost.shseq = 0seq = 16RED proven before implementing, and proven again by restoring the cross-type
refusal afterwards. Full 132-suite matrix green on PG18 and PG19 (130 ran / 2
skipped on 18, 132 / 0 on 19).
Scope
Deliberately only the executor's pruning. #461's cost model is the follow-on and
now has a base where pruning demonstrably occurs. I have not touched the
Columnar Pushed-Down Filterscounter, though it is arguably the reporting halfof this defect -- a filter that cannot exclude anything still counts as pushed
down. Worth its own issue if you agree.
🤖 Generated with Claude Code
https://claude.ai/code/session_013sY1RQR5MjZNUixA2um31r