Read a bloom filter for the columns a query filters on (#314) - #317
Merged
Conversation
A predicate probes one column, so a group that is examined needs the filters of the columns carrying predicates and no others. The reader fetched the whole group's filters and used at most one per predicate, so a single equality predicate on a twelve column table read twelve bloom bitmaps and probed one. The waste is worth removing because a bloom filter holds one bitmap per column sized by the group's distinct values, which makes it among the larger things in the metadata catalog. bloom_pkey is (storage_id, group_number, column_index), so naming the column turns the fetch into an exact index lookup rather than a range scan whose unwanted rows the caller discards. Measured on one group of 200,000 rows over 12 columns with one equality predicate: 715 buffers to 323. Deleting the bloom read outright gives 251, which is a floor and not a target, because the probed column's filter has to be read. Together with #310 the probe query falls from 9577 buffers to 1547. test/bloom_lazy.sh gains four checks. The property is that six columns' filters cost more to read than one column's: with the whole-group read those two queries were identical, because the work did not depend on the query at all. Measured 20 blocks against 120. While writing it I found the suite's existing #310 control was passing for the wrong reason. It used a column whose values were unique across the table, so its zone map excluded every group but one, and it was measuring the column count rather than the group count it claimed to. One column now repeats every 1000 rows so a single equality predicate keeps every group, and the control asserts that it does before relying on it. Proved by removal, both ways. Restoring the all-column read makes one and six predicates cost the same 21 blocks and turns the #314 checks red. Restoring the eager whole-group read turns all four red. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011miCFRSatixeNRw3w5yNq8
Collaborator
Author
|
Subsumed by #313, same as #314/#315 were. #313 already reads the bloom both lazily (only for groups the zone map didn't exclude) and per column ( Closing as superseded by #313. @jdatcmd heads-up: #313, #315, and #317 are the same fix arriving from multiple concurrent sessions of mine. Please review #313 — it's the complete, validated one; the others are subsets. I'm going to stop opening/closing duplicates in this area. |
This was referenced Aug 1, 2026
ChronicallyJD
added a commit
to ChronicallyJD/pgcolumnar
that referenced
this pull request
Aug 1, 2026
…pt#310) The entry for commandprompt#310 described the mechanism but measured it only on a 20-group synthetic shape, in buffer counts. That understates the effect and does not name its cause. Adds the figures from the profiled 100M TSBS-cpu run: a single bloom filter is 256 kB, the whole catalog is 3.5 GB, larger than the data it describes, and about 55 percent of the query's CPU sat in anonymous-page faults under the group-skip check from copying it per scan. The clustered hostname query falls from 4610 ms to 106 ms, a factor of 43, against the 30.6x measured on synthetic data. Measurement and profiling by ChronicallyJD in commandprompt#313. The code that PR proposed is already in main via commandprompt#315 and commandprompt#317, and is the same change; its evidence was better than what the merged commits carried, so this keeps it. Refs commandprompt#310, commandprompt#313, commandprompt#314. No issue is closed by this commit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011miCFRSatixeNRw3w5yNq8 Co-authored-by: ChronicallyJD <ChronicallyJD@users.noreply.github.com>
ChronicallyJD
pushed a commit
that referenced
this pull request
Aug 1, 2026
…d why This file has now been wrong four times, and the last three times the error was in the correction. An audit of the previous revision found two blockers in it: The paragraph correcting two "listed as not built, actually built" errors called the F1 delete-vector rename the one item still outstanding. It is built too, and the schema shows it directly: the catalog is named pgcolumnar.delete_vector today. The correcting paragraph committed the error it was correcting. The #289 entry said q6 is the only shape where columnar loses to heap. #289's own table shows q8 loses as well, 21,608 ms against heap's 15,315 ms. That sentence was load-bearing: it was the justification for singling q6 out. Also wrong and now fixed: "34x" did not follow from the two figures in its own sentence (1252/39 is 32.1); #317 was credited with closing #314, which was closed an hour before #317 was created; and the file claimed a PostgreSQL 13-19 matrix in three places when the matrix is 15-19. The common cause is structural, not clerical. Every revision restated measurements that live in the issues, and a restated measurement drifts the moment the issue moves. So the open list now carries no measurements at all. Each entry says what the work is, what is in flight, and which issue holds the current figures. Claims that cannot drift stay; claims that can are replaced by a pointer. Refs #289, #300, #291, #310. No issue is closed by this commit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011miCFRSatixeNRw3w5yNq8
ChronicallyJD
pushed a commit
that referenced
this pull request
Aug 2, 2026
…d why This file has now been wrong four times, and the last three times the error was in the correction. An audit of the previous revision found two blockers in it: The paragraph correcting two "listed as not built, actually built" errors called the F1 delete-vector rename the one item still outstanding. It is built too, and the schema shows it directly: the catalog is named pgcolumnar.delete_vector today. The correcting paragraph committed the error it was correcting. The #289 entry said q6 is the only shape where columnar loses to heap. #289's own table shows q8 loses as well, 21,608 ms against heap's 15,315 ms. That sentence was load-bearing: it was the justification for singling q6 out. Also wrong and now fixed: "34x" did not follow from the two figures in its own sentence (1252/39 is 32.1); #317 was credited with closing #314, which was closed an hour before #317 was created; and the file claimed a PostgreSQL 13-19 matrix in three places when the matrix is 15-19. The common cause is structural, not clerical. Every revision restated measurements that live in the issues, and a restated measurement drifts the moment the issue moves. So the open list now carries no measurements at all. Each entry says what the work is, what is in flight, and which issue holds the current figures. Claims that cannot drift stay; claims that can are replaced by a pointer. Refs #289, #300, #291, #310. No issue is closed by this commit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011miCFRSatixeNRw3w5yNq8
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 #314. Follows #315, and the two compound.
A predicate probes one column, so a group that is examined needs the filters of
the columns carrying predicates and no others. The reader fetched the whole
group's filters and used at most one per predicate, so a single equality
predicate on a twelve column table read twelve bloom bitmaps and probed one.
Why it is worth removing
A bloom filter holds one bitmap per column sized by the group's distinct values,
which makes it among the larger things in the metadata catalog.
bloom_pkeyis(storage_id, group_number, column_index), so naming the columnmakes the fetch an exact index lookup rather than a range scan whose unwanted
rows the caller discards. All three key columns are equality, so this is a probe,
not a narrowed range.
Measured
One group of 200,000 rows, twelve int columns, scanned whole with one equality
predicate:
The 251 is a floor and not a target: the probed column's filter has to be read.
Compounding with #315, on the 20-group probe from that PR:
Per-skipped-group cost is unchanged at about 64, as expected. #315 already
removed the bloom read for skipped groups; this is about the groups that are
kept.
Test
test/bloom_lazy.shgains four checks, taking it to 20.The property: six columns' filters cost more to read than one column's. With the
whole-group read those two queries were identical, because the work did not
depend on the query at all. Measured 20 blocks against 120. Block reads are
counted against
pgcolumnar.bloomwithpg_stat_get_blocks_fetched, so what isasserted is the thing that changed.
A correction to the suite this PR builds on
The existing #310 control, "a scan that keeps every group reads more", was
passing for the wrong reason. It used a column whose values are unique across the
table, so its zone map excluded every group but one: it claimed to vary the group
count and actually varied the column count. It went red under the #310 removal
proof, which is why it looked sound.
One column now repeats every 1000 rows, so a single equality predicate keeps
every group, and a new check asserts that the control query really does keep them
all before the property that depends on it is tested.
Worth stating plainly, because it is a limit of the removal-proof discipline
rather than a slip: a removal proof shows a check can go red, not that it goes
red for the stated reason. A fixture that is silently degenerate still responds
to the guard being removed, by a path nobody intended. Fixtures with a
precondition should assert the precondition as its own check.
Proved by removal, both ways
The two guards fire independently, so the suite covers both properties rather
than one standing in for the other.
Gates
Five-major matrix (15, 16, 17, 18.4, 19beta2).