From f115d0bdf4115421844e8854f7e981a572b1bd85 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Wed, 16 Sep 2026 10:30:53 -0600 Subject: [PATCH 1/2] test: the decode arms could not see a decode (#1077 sweep) `native_fetch_projection.sh` asserted two properties about the projection path -- that the visibility-only caller decodes nothing, and that the reconstruct caller asks only for uncovered columns -- with two arms that counted a CALL SITE, with its argument text, pinned at a literal 1: grep -c 'PgColumnarRowIsLive(rel, snap, baseRow)' "1" grep -c 'PgColumnarReadRowByNumberCols(rel, snap, baseRow' "1" That asserts a particular call is still written the way it was written. Plant the regression these arms exist to catch -- a full `PgColumnarReadRowByNumber` beside the liveness check in the visibility path -- and BOTH stay green, because the call they count is still there and the decode next to it is invisible to them: state live cols full | OLD1 OLD2 | NEW clean 1 1 0 | PASS PASS | PASS REGRESSED, full decode added 1 1 1 | PASS PASS | RED honest 2nd narrow caller 2 1 0 | PASS PASS | PASS empty file 0 0 0 | RED RED | premises RED Unlike the `allColumns` pair repaired in 8e88f42, these fail in ONE direction only. A realistic second caller uses different variable names, so the exact-text count stays at 1 and the old arm is BLIND rather than falsely alarmed. Row 3 is a pass for the old arms by accident, not by correctness, and saying they share a defect with 8e88f42's pair would have been the easy sentence and is not true. THE PROPERTY IS A ZERO. Three entry points exist and only one decodes every column: PgColumnarRowIsLive answers visibility, decodes nothing PgColumnarReadRowByNumberCols decodes a given set of columns PgColumnarReadRowByNumber decodes EVERY column so the projection path is asserted never to call the third. More correct callers of the two narrow entry points move nothing; any full decode moves it off zero -- the opposite failure direction from a count pinned at 1. Three premises keep the zero from being vacuous. Each narrow entry point is called at all, and -- asserted rather than assumed, because the whole arm rests on it -- `PgColumnarReadRowByNumberCols(` does not match `PgColumnarReadRowByNumber(`. Rename the narrow entry point to a prefix of the wide one and the zero would silently start counting the honest caller. Removal proof, against the real suite rather than extracted expressions: mutation applied md5 c8324ce4f486 -> a2c26702a9cc, full decodes 0 -> 1 suite run 16 passed + 1 failed + 0 unrunnable + 0 skipped = 17 the one red neither caller decodes every column: no full decode in the projection path: got [1 full decode(s)] want [0 full decode(s)] all three premises stayed green, correctly restored md5 c8324ce4f486, git diff empty Five majors, own `make clean`, own build and own `.so` each, on this exact tree (the suite file is md5 8cb2aa5d5f82 in both the tested tree and this commit): PG15 17 PASS PG16 17 PASS PG17 17 PASS PG18 17 PASS PG19 17 PASS NO LEDGER CHANGE, checked rather than inherited from 8e88f42: `native_fetch_projection` has 0 rows in main's ledger, so the gate cannot refuse its checks, and neither removed name appears in the ledger at all -- nothing to prune. The fourth and fifth arm in this file repaired for counting a string across a whole file. A sweep found 12 of this shape; the remaining seven are in `native_fetch_cache.sh`, `native_fetch_position.sh`, `decode_interrupts.sh` and `native_saop_pushdown.sh`. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NhwXKAgSmYDUjteWkfajHK --- test/native_fetch_projection.sh | 46 +++++++++++++++++++++++++++++---- 1 file changed, 41 insertions(+), 5 deletions(-) diff --git a/test/native_fetch_projection.sh b/test/native_fetch_projection.sh index 18887ead..5f6b67c8 100755 --- a/test/native_fetch_projection.sh +++ b/test/native_fetch_projection.sh @@ -139,11 +139,47 @@ check "and its values are right with nothing decoded from the base" \ # every check above while giving back what the change was for. SRC="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)/src" -check "the visibility-only caller decodes nothing" \ - "$(grep -c 'PgColumnarRowIsLive(rel, snap, baseRow)' "$SRC/columnar_projection.c")" "1" - -check "the reconstruct caller asks only for uncovered columns" \ - "$(grep -c 'PgColumnarReadRowByNumberCols(rel, snap, baseRow' "$SRC/columnar_projection.c")" "1" +# THE ARM NAMED "DECODES NOTHING" COULD NOT SEE A DECODE. Both of these counted a +# CALL SITE, with its argument text, and pinned it at a literal 1 -- so they +# asserted that a particular call is still written the way it was written, which is +# not the property either name claims. Measured: plant the regression these arms +# exist to catch, a full `PgColumnarReadRowByNumber` beside the liveness check in +# the visibility path, and BOTH arms stay green, because the call they count is +# still there and the decode added next to it is invisible to them. +# +# The property is a ZERO, not a one. There are three entry points and only one of +# them decodes every column: +# +# PgColumnarRowIsLive answers visibility, decodes nothing +# PgColumnarReadRowByNumberCols decodes a given set of columns +# PgColumnarReadRowByNumber decodes EVERY column -- the thing to stay out +# +# So assert the file never calls the full decode. A want-zero count is also the +# shape a second honest caller cannot break: more correct callers of the two narrow +# entry points move nothing, while any full decode moves it off zero. That is the +# opposite failure direction from a count pinned at 1, which reddens on correct +# additions and stays green on wrong ones. +# +# The two premises keep the zero from being vacuous: a file that called NOTHING +# would also report zero full decodes. +_np_live="$(grep -c 'PgColumnarRowIsLive(' "$SRC/columnar_projection.c")" +_np_cols="$(grep -c 'PgColumnarReadRowByNumberCols(' "$SRC/columnar_projection.c")" +_np_full="$(grep -c 'PgColumnarReadRowByNumber(' "$SRC/columnar_projection.c")" + +check "premise: the visibility-only entry point is called at all" \ + "$([ "${_np_live:-0}" -ge 1 ] && echo yes || echo no)" "yes" + +check "premise: the column-set entry point is called at all" \ + "$([ "${_np_cols:-0}" -ge 1 ] && echo yes || echo no)" "yes" + +# `PgColumnarReadRowByNumberCols(` does not match `PgColumnarReadRowByNumber(`, so +# the narrow caller is not counted as a full decode. Asserted rather than assumed, +# because the whole arm rests on it. +check "premise: the column-set caller is not counted as a full decode" \ + "$(printf 'PgColumnarReadRowByNumberCols(a, b)\n' | grep -c 'PgColumnarReadRowByNumber(')" "0" + +check "neither caller decodes every column: no full decode in the projection path" \ + "$_np_full full decode(s)" "0 full decode(s)" # Scoped to the function rather than counting a string across the file: the # string appears legitimately elsewhere now that the index fetch also asks only From 7920784a9de7ef4c6a794bcb0c0948a80a54aecc Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Wed, 16 Sep 2026 12:24:35 -0600 Subject: [PATCH 2/2] Restore the CHANGELOG entry, and correct what the third premise guarantees TWO FIXES, one of them a hole in this PR that review did not look for. THE CHANGELOG ENTRY WAS NEVER COMMITTED. `f115d0b` contains only `test/native_fetch_projection.sh`. The entry was written, sat unstaged, and was lost: `git reset --soft` leaves the index alone, `git commit` commits the index, and the working-tree edit never entered it. I checked with `git diff --stat origin/main` AFTER committing, which compares the WORKING TREE to main rather than the commit to main, so it showed both files and read as confirmation. That check cannot see this class of mistake at all. The one that can is `git show --name-only`, which reads the commit. Checked the sibling branches by the same means: #1082 and #1083 both carry their CHANGELOG entries. Only this one was affected. AND THE THIRD PREMISE'S COMMENT OVERSTATED ITS JOB. It read "the whole arm rests on it". It does not. A broken prefix relationship cannot pass silently, because the other two arms already contradict each other under it: if `Cols(` matched the wide pattern then every narrow call would be counted twice, so `full >= cols`, and `cols >= 1` with `full == 0` is a contradiction. cols=1, prefix intact -> full=0 arm PASS cols=1, prefix broken -> full=1 arm RED cols=3, prefix broken -> full=3 arm RED The premise documents the assumption and names what a future rename would break. It is not the guarantee. Kept, with the comment now saying which it is. Correction from @OffgridwithJD's review. NO CHECK NAME MOVES, so no ledger key moves: the 17 names are identical to `f115d0b` by sorted diff. Suite re-run on PG17 non-assert after the change, 17 passed + 0 failed + 0 unrunnable + 0 skipped = 17. Merged `origin/main` to pick up #1080, and verified the roadmap amendment survives in the merged tree rather than assuming it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NhwXKAgSmYDUjteWkfajHK --- CHANGELOG.md | 48 +++++++++++++++++++++++++++++++++ test/native_fetch_projection.sh | 17 ++++++++++-- 2 files changed, 63 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b0448dd..508e1238 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -168,6 +168,54 @@ true until the next version shipped. ### Fixed +- The arm named "the visibility-only caller decodes nothing" passed on a tree + where it decoded (#1077 sweep). + + Two arms in `native_fetch_projection.sh` counted a CALL SITE, with its argument + text, pinned at a literal `1`: + + grep -c 'PgColumnarRowIsLive(rel, snap, baseRow)' "1" + grep -c 'PgColumnarReadRowByNumberCols(rel, snap, baseRow' "1" + + That asserts a particular call is still written the way it was written, which is + not the property either name claims. Plant the regression they exist to catch -- + a full `PgColumnarReadRowByNumber` beside the liveness check in the visibility + path -- and both stay green, because the call they count is still there and the + decode added next to it is invisible to them: + + state live cols full | OLD1 OLD2 | NEW + clean 1 1 0 | PASS PASS | PASS + REGRESSED, full decode added 1 1 1 | PASS PASS | RED + honest 2nd narrow caller 2 1 0 | PASS PASS | PASS + empty file 0 0 0 | RED RED | premises RED + + Unlike the `allColumns` pair repaired in `8e88f42`, these fail in ONE direction + only. A realistic second caller uses different variable names, so the exact-text + count stays at 1 and the old arm is BLIND rather than falsely alarmed. Row 3 is a + pass for the old arms by accident, not by correctness. + + THE PROPERTY IS A ZERO. Three entry points exist and only one decodes every + column, so the projection path is asserted never to call it. More correct callers + of the two narrow entry points move nothing; any full decode moves it off zero -- + the opposite failure direction from a count pinned at 1. + + Three premises keep the zero from being vacuous: each narrow entry point is + called at all, and that `PgColumnarReadRowByNumberCols(` does not match + `PgColumnarReadRowByNumber(`. That third one DOCUMENTS the assumption rather than + providing the guarantee, which the first version of its comment got wrong: a + broken prefix would make `full >= cols`, and `cols >= 1` with `full == 0` is a + contradiction, so the main arm reddens rather than hiding. Corrected in review by + @OffgridwithJD. + + Removal proof, against the real suite: md5 `c8324ce4f486` -> `a2c26702a9cc`, full + decodes 0 -> 1, `16 passed + 1 failed + 0 unrunnable + 0 skipped = 17`, one red + naming `got [1 full decode(s)] want [0 full decode(s)]`, all three premises green, + restored with an empty diff. + + The fourth and fifth arm in this file repaired for counting a string across a + whole file; `8e88f42` did the second and third and the `deltuples` comment + records the first. + - The projection guard fired on a correct caller and stayed green on a wrong one (#1077). diff --git a/test/native_fetch_projection.sh b/test/native_fetch_projection.sh index 5f6b67c8..28d1f85b 100755 --- a/test/native_fetch_projection.sh +++ b/test/native_fetch_projection.sh @@ -173,8 +173,21 @@ check "premise: the column-set entry point is called at all" \ "$([ "${_np_cols:-0}" -ge 1 ] && echo yes || echo no)" "yes" # `PgColumnarReadRowByNumberCols(` does not match `PgColumnarReadRowByNumber(`, so -# the narrow caller is not counted as a full decode. Asserted rather than assumed, -# because the whole arm rests on it. +# the narrow caller is not counted as a full decode. +# +# THIS PREMISE DOCUMENTS THE INTENT; IT IS NOT WHAT PROVIDES THE GUARANTEE, and +# the first version of this comment said it was. A broken prefix relationship +# cannot pass silently, because the other two arms already contradict each other +# under it: if `Cols(` matched the wide pattern then every narrow call would be +# counted twice, so `full >= cols`, and `cols >= 1` with `full == 0` is a +# contradiction. The arm reddens rather than hiding. +# +# cols=1, prefix intact -> full=0 arm PASS +# cols=1, prefix broken -> full=1 arm RED +# cols=3, prefix broken -> full=3 arm RED +# +# Kept because a reader should not have to derive that, and because it names the +# assumption a future rename would break. Correction from @OffgridwithJD's review. check "premise: the column-set caller is not counted as a full decode" \ "$(printf 'PgColumnarReadRowByNumberCols(a, b)\n' | grep -c 'PgColumnarReadRowByNumber(')" "0"