Repository navigation
fix: GET of a never-put memo index is an error (#47) - #59
Merged
Merged
Conversation
jensens
force-pushed
the
fix/memo-gap
branch
from
September 30, 2026 15:47
3d3396a to
6864844
Compare
The memo is a `Vec<Option<PickleValue>>`: a put above the current length grows it with `None`, every store writes `Some`, and `memo_get` maps a `None` to the `memo index N not found` error the out-of-range case already raised. The dirty-flush sites write `Some`. No new bookkeeping; the sample database decodes identically and the FileStorage decode A/B against main is within noise (0.97). Assisted-by: Claude Fable 5.1
…ts (#47) Fresh review: ready to merge (80,000 differential memo streams against CPython match on every non-MEMOIZE case, sample identical, Option niche at 56 bytes). Minors folded in: `decode_zodb_record_for_pg` joins the parametrized test, a trailing byte after STOP pins the gap on the MemoNeeds::All path, and the size comments say 56 bytes. Assisted-by: Claude Fable 5.1
jensens
force-pushed
the
fix/memo-gap
branch
from
September 30, 2026 17:13
6864844 to
7d63c47
Compare
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
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.
Fixes #47
A
GET/BINGET/LONG_BINGETof a memo index that was never put yieldedNonewhen a higher index had been put before:memo_putgrows the memo vector with placeholders, and a gap below the highest index read as one. CPython raisesMemo value not found at index N.What changes
The memo is a
Vec<Option<PickleValue>>: growth fills withNone, every store writesSome, andmemo_getmaps aNoneto thememo index N not founderror the out-of-range case already raised. The dirty-flush sites (a memoized container mutated after the put) writeSome.Option<PickleValue>uses the enum's niche, so the element stays 56 bytes and the vector does not grow; the FileStorage decode and PG pipeline A/B againstmainare within the noise floor.No pickler writes such a stream; this is corruption handling, and the pre-scan optimisation (puts of indices never read are skipped) is unaffected because the gap error is path-independent.
Evidence
STOP), in protocol 0 text opcodes, a dirty entry after a gap put, every Python entry point, and the memo shared across the two pickles of a ZODB record (a class pickle with its own puts makes index 0 valid in the state pickle).main.MEMOIZEthe branch now matches CPython's error decision exactly (832 streams thatmaindecoded to a value are errors in both), and it reports the same index. The remaining divergence,MEMOIZEafter a re-put of a filled slot, predates this branch (Decode regression since 1.6.0 (memo dirty-sync clones every container); pre-scan memo reads to skip unread puts #22) and is filed as a follow-up issue.Assisted-by: Claude Fable 5.1