Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 14 additions & 2 deletions test/native_fetch_cache.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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.
#
Expand Down
Loading