fix(upgrade): __db_set_lastpgno stored page count, not last page number - #77
Merged
Conversation
__db_lastpgno() returns the page COUNT (file bytes / pagesize) -- its other caller __db_page_pass uses it correctly as a loop bound (for i=0; i<pgno_last). But __db_set_lastpgno assigned that count directly into meta.last_pgno, which the rest of the engine treats as the last valid page NUMBER (0-indexed). For an N-page file this left last_pgno = N instead of N-1 -- one too high. This is on the v8->v9 upgrade path (__db_upgrade case 8, a pure metadata pass). db_verify then rejects the upgraded file under HAVE_FTRUNCATE with "Page 0: last_pgno is not correct: N != N-1". The bug is byte-identical to upstream Berkeley DB 4.7/4.8, i.e. a long-standing latent defect; the hash upgrade path happened to mask it (its v8->v9 pass goes through mpool, which recomputes last_pgno on close). Fix: store the last page NUMBER (count - 1), mapping an impossible 0-page file to PGNO_INVALID rather than underflowing. Found by the access-method upgrade coverage work (PR #75, which exercises the per-version transforms via synthetic old-format fixtures). Verified: a v8 meta upgraded with db_upgrade now passes db_verify (was "last_pgno is not correct"); normal v9 upgrade + btree/hash test001 unaffected.
Coccinelle convention checksNo new violations. ✅ Resolved since baseline (2) -- update dist/cocci/baseline.txt to lock these in. |
ABI diff vs
|
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
1 participant
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.
Long-standing latent defect (byte-identical to upstream BDB 4.7/4.8), found by the access-method upgrade coverage work (PR #75).
__db_lastpgno()returns the page count (bytes/pagesize) — its other caller__db_page_passuses it correctly as a loop bound (for i=0; i<pgno_last). But__db_set_lastpgnoassigned that count straight intometa.last_pgno, which the engine treats as the last valid page number (0-indexed). For an N-page file this leftlast_pgno = Ninstead ofN-1.On the v8→v9 upgrade path (
__db_upgradecase 8),db_verifythen rejects the file underHAVE_FTRUNCATE:Page 0: last_pgno is not correct: N != N-1. The hash path masked it (its v8→v9 pass goes through mpool, which recomputeslast_pgnoon close).Fix: store the last page number (
count - 1), mapping an impossible 0-page file toPGNO_INVALIDrather than underflowing.Verified
db_upgradenow passesdb_verify(was "last_pgno is not correct")test001 btree/hashunaffected(The v7-flip synthetic fixture still fails verify — that's a known synthetic-fixture artifact per PR #75, not this bug; flipping a real v9 file to v7 doesn't produce a byte-accurate v7 page layout.)