Repository navigation
Fix edge cases in GRI levels caching - #59
Merged
Merged
Conversation
Wid-L-Hacker
force-pushed
the
conrad/gri-level-cache-fix
branch
from
November 9, 2026 08:18
a12b05d to
7efd5ae
Compare
mlin
marked this pull request as ready for review
November 9, 2026 08:32
The per-connection levels cache filed entries under the bare table name and validated them only by the database data version and total change count. Three gaps followed: - Levels computed over uncommitted writes were memoized. No rollback (full or ROLLBACK TO a savepoint) moves either validity counter, so after a rollback the cache could serve levels for rows that no longer exist, or miss rows the rollback restored, silently skewing range queries fed by the cached bounds. Cache entries are now stored only in autocommit mode, and the cache is bypassed entirely while a writable statement is executing on the connection: levels consulted from inside such a statement (e.g. from a trigger of the DELETE in flight) reflect its uncommitted snapshot even in autocommit mode -- sqlite3_get_autocommit() stays 1 while a statement executes -- and a statement rollback is just as invisible to the counters. - An unqualified table name was filed as the main database's even when SQLite's name resolution took it to a temp table or an attached database. An unqualified name is now cached only if it resolves to main -- it names a table or view in main and no temp table or view shadows it -- under a schema-qualified key, and the catalog names are compared with sqlite3_stricmp(), the comparison SQLite itself applies to identifiers, rather than a SQL collation a connection can override. Adds regression tests: savepoint rollback, full rollback, bare savepoints, cross-connection invalidation, temp-table shadowing in both directions and after the cache is warmed, detach/reattach of a different file under one schema name, a trigger that consults the levels and then aborts its own statement (also via DELETE ... RETURNING and via a view over the function), and a case-sensitive NOCASE collation override.
Wid-L-Hacker
force-pushed
the
conrad/gri-level-cache-fix
branch
from
November 9, 2026 23:53
7efd5ae to
ba490ef
Compare
mlin
changed the title
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.
The levels cache was keyed by the bare table name and validated only by counters that no rollback moves, so it could serve levels that stopped being true: levels computed over writes a rollback later undid, and temp/attached tables' levels filed under the main table's entry.
Unqualified names are now cached only when they resolve to the main database, under a schema-qualified key, compared with SQLite's own identifier comparison. Stores stay autocommit-only, and the cache is bypassed while a writable statement is executing, since trigger-consulted levels reflect its uncommitted snapshot. Eleven regression tests cover the failure modes.