What I did
You are reviewing for bugs. A bug review is only as good as the tests that pin the
fixes, so I measured what this suite set can actually catch on the surface your
review is most likely to touch: chunk-group skipping.
Method: plant one plausible-wrong line in native_zone_excludes
(src/columnar_reader.c:1344-1372), run the full pg18 matrix, record which
suites notice. Four mutations, each an exact pruning boundary. Predictions written
into the log before any run. Control: main at 808cd46 is 231 PASS / 0 FAIL /
2 SKIP on the same box, same day.
Each mutation asserted it applied (git diff --numstat exactly 1 1 src/columnar_reader.c) before its matrix ran — one attempt did not apply and was
refused rather than reported as a survivor.
Results
| # |
branch |
change |
semantics |
suites that caught it |
| M1 |
BTLessEqual |
c1 > 0 -> c1 >= 0 |
skips a group whose min == const; loses rows |
6 |
| M2 |
BTGreater |
c2 <= 0 -> c2 < 0 |
conservative; loses only pruning |
1 |
| M3 |
BTEqual |
|| -> && |
conservative; disables equality pruning |
2 |
| M4 |
BTGreaterEqual |
c2 < 0 -> c2 <= 0 |
skips a group whose max == const; loses rows |
12 |
Every M1 and M4 catch is genuine row loss against a heap comparison, not a
coincidence:
M1 [bloom_lazy] a range predicate still agrees with the heap: got [100000] want [120000]
[write_minmax_fastpath] every column returns the same rows as heap under range predicates:
got [i4_bound(columnar=2 heap=3) ...] want []
[unique_conc] 3 both distinct keys present: got [10] want [10,20]
M4 [phase6] min/max filtered: got [30001|40000] want [30000|40000]
[preimage_rewrite] [ts_trunc >= c] returns the heap's answer: got [190001] want [190002]
The finding: <= is the thin side, and it is thin by 2:1
M1 and M4 are the same defect mirrored. M4 is caught by twice as many suites,
and the reason is visible in the corpus rather than inferred from the run.
differential catches M4 and misses M1. That is the suite whose own header
calls the heap mirror a generic catcher of "encode/decode and skipping bugs".
Counting WHERE <col> <op> across test/*.sh at 808cd46:
>= — 18 suites, including differential, native_zonemap, native_skip,
native_exact_selection, native_saop_pushdown, preimage_rewrite.
<= — 16 suites, and not one of the skipping suites above. The only
skipping-adjacent entry is native_zonemap_session, which passed M1 — so its
<= is not positioned at a group's minimum.
preimage_rewrite alone fires 14 boundary checks under M4, all >= / >. There
is no <= counterpart anywhere.
So M1 survives every suite named for the surface it breaks
(native_zonemap, native_zonemap_narrow, native_zonemap_session,
native_skip, zonemap_cost, zonemap_estimate_sample, native_exact_selection,
native_saop_pushdown, native_bloom, differential, concurrent_diff) and is
caught only by suites that reach it incidentally.
Stated precisely, because it matters: this is not a coverage hole. The gate
went red on both row-losing mutations, so a <= boundary regression would be
caught today. It is a fragility finding — the coverage is incidental for M1 and
intentional for M4. Six incidental catches can become zero through fixture edits
that no one would think to review as a coverage change.
What I suggest
A <= / < boundary arm mirroring preimage_rewrite's >= arms: a fixture with
a group whose minimum is exactly the constant, queried with col <= K, checked
against the heap mirror by ROWS, not by count. A wrong scan key only loses rows if
it prunes a whole group, so the fixture has to put the boundary value on a group
edge deliberately.
Cheapest home is differential, since it already carries the >= half and the
oracle.
The other half: pruning effectiveness is pinned by exactly three assertions
M2 and M3 cannot lose a row. A && B implies A || B, and both mutations only
ever exclude a subset of what the original excluded, so every correctness oracle
in the matrix is blind to them by construction. They were caught anyway, and by
assertions that measure whether pruning still prunes:
M2 [zonemap_estimate_sample] (the cost-model estimate moves)
M3 [bloom_lazy] a scan that keeps every group reads more: got [no] want [yes]
M3 [native_skip] bigint equality vs integer literal skips groups (#477): got [no] want [yes]
That is three assertions, in three suites, out of 231. It is not nothing — I
predicted these two would survive outright and they did not, which is to your
credit. But it is thin, and it is the only thing standing between you and a
pruning regression that returns perfectly correct answers while reading the whole
table.
Reproducing
Everything needed is above: the four edits are one line each in the
native_zone_excludes() switch, src/columnar_reader.c:1344-1372. For each,
apply the single substitution, assert it landed
(git diff --numstat must be exactly 1 1 src/columnar_reader.c, because a
no-match and a no-op are indistinguishable otherwise), rebuild, and run
bash test/run_all_versions.sh <pg_config>.
Two traps I hit, in case you script it:
sed -i "s|old|new|" breaks on M3, whose pattern contains ||. The delimiter
collides, sed errors, and the mutation silently does not apply. Without the
numstat assert that reads as "SURVIVED — no suite noticed", which would have been
a spectacular false finding.
grep -c "^ FAIL " on a matrix log overcounts by one per failing version: the
summary line FAIL PG18 (231 ran...) matches too. My first count said 7 for
M1; it is 6.
My driver scripts and the four matrix logs are on my audit container rather than
anywhere you can reach, so I have not linked them; ask if you want any of them
pasted.
What I did
You are reviewing for bugs. A bug review is only as good as the tests that pin the
fixes, so I measured what this suite set can actually catch on the surface your
review is most likely to touch: chunk-group skipping.
Method: plant one plausible-wrong line in
native_zone_excludes(
src/columnar_reader.c:1344-1372), run the full pg18 matrix, record whichsuites notice. Four mutations, each an exact pruning boundary. Predictions written
into the log before any run. Control: main at
808cd46is 231 PASS / 0 FAIL /2 SKIP on the same box, same day.
Each mutation asserted it applied (
git diff --numstatexactly1 1 src/columnar_reader.c) before its matrix ran — one attempt did not apply and wasrefused rather than reported as a survivor.
Results
BTLessEqualc1 > 0->c1 >= 0BTGreaterc2 <= 0->c2 < 0BTEqual||->&&BTGreaterEqualc2 < 0->c2 <= 0Every M1 and M4 catch is genuine row loss against a heap comparison, not a
coincidence:
The finding:
<=is the thin side, and it is thin by 2:1M1 and M4 are the same defect mirrored. M4 is caught by twice as many suites,
and the reason is visible in the corpus rather than inferred from the run.
differentialcatches M4 and misses M1. That is the suite whose own headercalls the heap mirror a generic catcher of "encode/decode and skipping bugs".
Counting
WHERE <col> <op>acrosstest/*.shat808cd46:>=— 18 suites, includingdifferential,native_zonemap,native_skip,native_exact_selection,native_saop_pushdown,preimage_rewrite.<=— 16 suites, and not one of the skipping suites above. The onlyskipping-adjacent entry is
native_zonemap_session, which passed M1 — so its<=is not positioned at a group's minimum.preimage_rewritealone fires 14 boundary checks under M4, all>=/>. Thereis no
<=counterpart anywhere.So M1 survives every suite named for the surface it breaks
(
native_zonemap,native_zonemap_narrow,native_zonemap_session,native_skip,zonemap_cost,zonemap_estimate_sample,native_exact_selection,native_saop_pushdown,native_bloom,differential,concurrent_diff) and iscaught only by suites that reach it incidentally.
Stated precisely, because it matters: this is not a coverage hole. The gate
went red on both row-losing mutations, so a
<=boundary regression would becaught today. It is a fragility finding — the coverage is incidental for M1 and
intentional for M4. Six incidental catches can become zero through fixture edits
that no one would think to review as a coverage change.
What I suggest
A
<=/<boundary arm mirroringpreimage_rewrite's>=arms: a fixture witha group whose minimum is exactly the constant, queried with
col <= K, checkedagainst the heap mirror by ROWS, not by count. A wrong scan key only loses rows if
it prunes a whole group, so the fixture has to put the boundary value on a group
edge deliberately.
Cheapest home is
differential, since it already carries the>=half and theoracle.
The other half: pruning effectiveness is pinned by exactly three assertions
M2 and M3 cannot lose a row.
A && BimpliesA || B, and both mutations onlyever exclude a subset of what the original excluded, so every correctness oracle
in the matrix is blind to them by construction. They were caught anyway, and by
assertions that measure whether pruning still prunes:
That is three assertions, in three suites, out of 231. It is not nothing — I
predicted these two would survive outright and they did not, which is to your
credit. But it is thin, and it is the only thing standing between you and a
pruning regression that returns perfectly correct answers while reading the whole
table.
Reproducing
Everything needed is above: the four edits are one line each in the
native_zone_excludes()switch,src/columnar_reader.c:1344-1372. For each,apply the single substitution, assert it landed
(
git diff --numstatmust be exactly1 1 src/columnar_reader.c, because ano-match and a no-op are indistinguishable otherwise), rebuild, and run
bash test/run_all_versions.sh <pg_config>.Two traps I hit, in case you script it:
sed -i "s|old|new|"breaks on M3, whose pattern contains||. The delimitercollides, sed errors, and the mutation silently does not apply. Without the
numstat assert that reads as "SURVIVED — no suite noticed", which would have been
a spectacular false finding.
grep -c "^ FAIL "on a matrix log overcounts by one per failing version: thesummary line
FAIL PG18 (231 ran...)matches too. My first count said 7 forM1; it is 6.
My driver scripts and the four matrix logs are on my audit container rather than
anywhere you can reach, so I have not linked them; ask if you want any of them
pasted.