From c13b931607ddbad5242ae0de93056bd04463e4f5 Mon Sep 17 00:00:00 2001 From: OffgridwithJD Date: Wed, 16 Sep 2026 18:54:40 +0000 Subject: [PATCH] The natts comparison is not against the row group (#1081) #1081 replaced a count of `entry->fileOffset != rg->fileOffset` with a per-field membership loop, so dropping any one of the four compared fields reddens by name. The loop matched `entry->$_f != rg->$_f` OR `entry->$_f != natts` for EVERY field, because `natts` is the one compared against the scan's own column count rather than against the row group. Harmless today -- `entry->firstRowNumber != natts` appears nowhere -- and still wrong as a claim: each arm would accept a comparison its own name denies. Three fields now match only the `rg` form; `natts` has its own arm and its own name. Reported by @jdatcmd reviewing #1081. ALL FOUR VERIFIED BY REMOVAL on the real suite, each reddening only its own arm: firstRowNumber removed FAIL ... firstRowNumber against the row group rowCount removed FAIL ... rowCount against the row group fileOffset removed FAIL ... fileOffset against the row group natts removed FAIL ... natts against the scan's column count restored 26 passed, source byte-identical FIRSTROWNUMBER IS THE ONE #1081 SHIPPED UNPROVEN, and the reason is worth keeping. Its mutation never applied: that line begins `(entry->` rather than `entry->`, so the pattern missed, and the harness's applied-assertion REPORTED it instead of letting a clean run count as a pass. A mutation that does not apply looks exactly like an arm that does not fire. Five majors: PG15 PG16 PG17 PG18 PG19, native_fetch_cache PASSED. No ledger change: one check name is added and none removed, and the suite has zero rows in the ledger -- it is one of the 249 uncovered suites. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_012RSw4qMHS7ByE7PY8Ns4cs --- CHANGELOG.md | 29 +++++++++++++++++++++++++++++ test/native_fetch_cache.sh | 16 ++++++++++++++-- 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 01597056..d15593a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,35 @@ true until the next version shipped. ## [Unreleased] +### Fixed + +- One geometry arm accepted a comparison the property does not describe (#1081). + + #1081 replaced a count of `entry->fileOffset != rg->fileOffset` with a per-field + membership loop, so that dropping any one of the four compared fields reddens by + name. The loop matched `entry->$_f != rg->$_f` OR `entry->$_f != natts` for every + field, because `natts` is the one field compared against the scan's own column + count rather than against the row group. + + Harmless today -- `entry->firstRowNumber != natts` appears nowhere -- and still + wrong as a claim: each arm would accept a comparison its own name denies. Three + fields now match only the `rg` form, and `natts` has its own arm and its own name. + + Reported by @jdatcmd on #1081's review. + + All four verified by removal on the real suite, each reddening only its own arm: + + firstRowNumber removed FAIL a hit re-checks the group's firstRowNumber ... + rowCount removed FAIL a hit re-checks the group's rowCount ... + fileOffset removed FAIL a hit re-checks the group's fileOffset ... + natts removed FAIL a hit re-checks the group's natts against the + scan's column count + restored 26 passed, source byte-identical + + `firstRowNumber` is the one #1081 shipped unproven. Its mutation did not apply -- + that line starts `(entry->` rather than `entry->`, so the pattern missed -- and the + harness's applied-assertion reported it rather than counting a clean run as a pass. + ### Added - Six more guards counted their callers instead of pinning their property (#1078's diff --git a/test/native_fetch_cache.sh b/test/native_fetch_cache.sh index 950f7983..e936e050 100755 --- a/test/native_fetch_cache.sh +++ b/test/native_fetch_cache.sh @@ -237,13 +237,25 @@ check "an entry from an earlier command is rejected" \ # The recheck compares four fields. Counting one of them asserted nothing about # the other three, and a reflow that dropped one would have passed. -for _f in firstRowNumber rowCount fileOffset natts; do +# +# THREE COMPARE AGAINST THE ROW GROUP AND ONE DOES NOT. `natts` is compared against +# the scan's own column count rather than against `rg`, so it needs its own pattern. +# Reported by @jdatcmd: the first version put `!= natts` in the alternation for ALL +# four, which is harmless -- `entry->firstRowNumber != natts` appears nowhere -- and +# is still wrong as a claim, because each arm would then accept a comparison the +# property does not describe. +for _f in firstRowNumber rowCount fileOffset; do check "a hit re-checks the group's $_f against the row group" \ "$(case "$_nfc_row" in \ - *"entry->$_f != rg->$_f"*|*"entry->$_f != natts"*) echo yes ;; \ + *"entry->$_f != rg->$_f"*) echo yes ;; \ *) echo no ;; esac)" "yes" done +check "a hit re-checks the group's natts against the scan's column count" \ + "$(case "$_nfc_row" in \ + *"entry->natts != natts"*) echo yes ;; \ + *) echo no ;; esac)" "yes" + # Named call sites rather than a count of 2: a third honest caller is not a defect, # and losing either of these two is. #