feat: implement PRAGMA synchronous (FULL/NORMAL/OFF) (#645) - #650
Merged
Conversation
PRAGMA synchronous was silently ignored, so every commit unconditionally did the fsyncs that only match FULL (the default). Adds a full parser/AST/codegen/VDBE path (mirroring journal_mode's #388 shape, but bidirectional: the bare query form is implemented too, unlike journal_mode's) and wires Pager::flush_locked/flush_wal_locked to skip fsyncs per ADR-0036's policy table: Level Journal fsync (rollback) Main-file fsync (rollback) WAL frame fsync Full yes yes yes Normal yes no no Off no no no SynchronousMode lives in src/header.rs (not src/pager.rs, its otherwise natural home) so vdbe/pragma.rs can name it without violating spec 001-architecture Requirement 1 ("VDBE does not know file format" — vdbe/ may never `use crate::pager` directly). repl.rs's generic (non-SELECT) statement path previously discarded any result rows a program emitted — meaning PRAGMA integrity_check's output was silently dropped too, pre-existing and unrelated to this ticket but fixed alongside it since the synchronous query form needs the same row-printing to be testable via the CLI at all. MemoryVfs gained a sync_calls() counter (shared across clones/handles) so the new Pager fsync-gating tests can assert whether a commit actually fsynced, which an in-memory backend has no other way to show. spend: matched estimate (medium) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
3 tasks
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.
Summary
PRAGMA synchronouswas silently ignored (src/vdbe/pragma.rshad no handling), so every commit unconditionally did the fsyncs matching only stock SQLite'sFULLdefault, with no way to opt intoNORMAL/OFF.Adds a full parser → AST → codegen → VDBE path, mirroring
journal_mode's (pager: journal_mode=WAL switching #388) shape but bidirectional — the bare query form (PRAGMA synchronous, no=) is implemented too, unlikejournal_mode's (stillUnsupported).Pager::flush_locked/flush_wal_lockednow gate their three commit-time fsyncs on the activeSynchronousMode, per the policy table in ADR-0036:Full(default)NormalOffArchitecture note:
SynchronousModelives insrc/header.rs, notsrc/pager.rs(its otherwise natural home) —vdbe/pragma.rsneeds to name it directly, andvdbe/may neveruse crate::pagerdirectly (spec 001-architecture Requirement 1, enforced bytests/unit/layer_isolation.rs). Same reasonJournalModealready lives there.Incidental fix:
repl.rs's generic (non-SELECT) statement path was silently discarding any result rows a program emitted — meaningPRAGMA integrity_check's output was already being dropped, pre-existing and unrelated to this ticket, but fixed alongside it sincesynchronous's query form needed the same row-printing to be testable via the CLI at all.MemoryVfsgained async_calls()counter (shared across clones/handles) so the newPagerfsync-gating tests can assert whether a commit actually fsynced — an in-memory backend has no other way to observe that.Test plan
cargo test— full suite passes (975 lib tests + all integration suites, 0 failures)cargo clippy --all-targets/cargo fmt --check— cleanmake check-grammar-drift— clean (grammar EBNF extended withsynchronouscarve-out, tagged V7)make assurance— no dead links introducedON/EXTRA/out-of-range ints — explicitly out of scope per the ticket)tests/unit/pragma_synchronous_repl.rs) diffs ourreploutput against realsqlite3forPRAGMA synchronous = <level>; PRAGMA synchronous;across OFF/NORMAL/FULLRefs: #645
spend: matched estimate (medium)
🤖 Generated with Claude Code
Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com