From 9c738aca0e37fe88f01b808a703b5d6bdbd9d955 Mon Sep 17 00:00:00 2001 From: Ilja Heitlager Date: Sat, 29 Aug 2026 22:02:35 +0200 Subject: [PATCH] feat: implement PRAGMA synchronous (FULL/NORMAL/OFF) (#645) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../0036-pragma-synchronous-fsync-policy.md | 122 +++++++++++++ .openspec/adr/index.md | 1 + .openspec/grammar/sqlite.ebnf | 30 ++-- Cargo.toml | 4 + src/bin/sqlite-rs/repl.rs | 19 ++- src/codegen/pragma.rs | 72 +++++++- src/header.rs | 24 +++ src/pager.rs | 160 +++++++++++++++++- src/parser/ast.rs | 36 +++- src/parser/grammar.rs | 43 +++++ src/vdbe.rs | 5 +- src/vdbe/exec.rs | 3 +- src/vdbe/explain.rs | 2 + src/vdbe/pragma.rs | 93 +++++++++- src/vdbe/program.rs | 11 ++ src/vfs/memory.rs | 31 +++- tests/unit/pragma_parser.rs | 63 ++++++- tests/unit/pragma_synchronous_repl.rs | 160 ++++++++++++++++++ 18 files changed, 843 insertions(+), 36 deletions(-) create mode 100644 .openspec/adr/0036-pragma-synchronous-fsync-policy.md create mode 100644 tests/unit/pragma_synchronous_repl.rs diff --git a/.openspec/adr/0036-pragma-synchronous-fsync-policy.md b/.openspec/adr/0036-pragma-synchronous-fsync-policy.md new file mode 100644 index 00000000..138cab39 --- /dev/null +++ b/.openspec/adr/0036-pragma-synchronous-fsync-policy.md @@ -0,0 +1,122 @@ +# 0036: `PRAGMA synchronous` fsync-skip policy, and why `SynchronousMode` lives in `header.rs` + +Date: 2026-08-29 + +## Context + +#645: `PRAGMA synchronous` was silently ignored — every commit +unconditionally did the fsyncs that only match stock SQLite's `FULL` +(the default), with no way to opt into `NORMAL`/`OFF`. Implementing it +for real means deciding, precisely, which of `Pager`'s three +commit-time fsyncs each level skips: + +- The rollback-journal fsync (`flush_locked`, before writing any dirty + page into the main file) — the safety ordering `recover_hot_journal` + depends on to replay a hot journal after a crash mid-write. +- The rollback main-file fsync (`flush_locked`, after writing every + dirty page) — the second half of the two-fsync rollback-journal + commit protocol. +- The WAL frame fsync (`flush_wal_locked`, before publishing the new + `mxFrame`) — WAL mode's own single commit-time fsync. + +Stock SQLite's documentation gives a clear, if slightly asymmetric, +answer: `NORMAL` still fsyncs "at the most critical moments" in +rollback-journal mode, but in WAL mode it only syncs at checkpoint +boundaries, not on every commit. `OFF` never fsyncs at all. Chasing +every historical nuance of `getSafetyLevel`'s masking of arbitrary +out-of-range integer values, `EXTRA`, and the `ON` boolean alias was +explicitly out of scope (#645's acceptance criteria only names +`FULL`/`NORMAL`/`OFF`). + +A second, unrelated question came up during implementation: `vdbe/` +may never `use crate::pager` directly (spec 001-architecture +Requirement 1, "VDBE does not know file format", enforced by +`tests/unit/layer_isolation.rs`). `synchronous`'s state is exactly the +kind of thing `vdbe/pragma.rs` needs to name directly (to convert +between the wire-level `i32` opcode operand and a real enum, and to +report the current value back as a query result) — the same situation +`JournalMode` was already in, and already solved by living in +`src/header.rs` rather than `src/pager.rs`. + +## Decision + +**Fsync policy** (`Pager::flush_locked`/`flush_wal_locked`): + +| Level | Journal fsync (rollback) | Main-file fsync (rollback) | WAL frame fsync | +|----------|:---:|:---:|:---:| +| `Full` | yes | yes | yes | +| `Normal` | yes | no | no | +| `Off` | no | no | no | + +`Normal` keeps the rollback-journal fsync because it's what makes +`recover_hot_journal` safe at all — without it, a crash between the +journal write and the main-file write could leave a journal on disk +whose own bytes never made it to a stable state, defeating the +recovery it exists to enable. It drops the main-file fsync (the thing +`FULL` adds on top) and the WAL frame fsync, matching stock SQLite's +documented "still consistent, less durable" `NORMAL` semantics for +both journal modes. + +Two other fsync call sites are deliberately left ungated: +`Pager::set_journal_mode`'s own page-1 write-back (a mode switch, not +part of any user transaction) and `checkpoint::checkpoint_passive`'s +post-backfill fsync (checkpoints sync the main file under `NORMAL` +too, per stock SQLite's docs — gating it would need threading +`synchronous` through a free function that doesn't otherwise touch +`Pager` state, for a case `#645`'s acceptance criteria doesn't ask +for). + +**Where `SynchronousMode` lives**: `src/header.rs`, next to +`JournalMode`, even though — unlike `JournalMode` — it has no on-disk +representation at all (stock SQLite never persists `synchronous`; a +fresh connection always starts at `Full`). `header.rs` is the +established "vdbe-safe vocabulary" module for exactly this situation: +an enum `vdbe/pragma.rs` must reference by name (to build/read it) but +that `src/pager.rs` itself would otherwise be the natural home for. + +## Alternatives rejected + +- **Cache the fsync policy as three precomputed booleans** (`sync_journal`, + `sync_main_file`, `sync_wal_frame`) instead of a `SynchronousMode` + enum matched at each call site. Rejected: the enum is the thing + `PRAGMA synchronous` (query form) needs to report back verbatim + (`0`/`1`/`2`), and three booleans would just be a less legible + re-encoding of the same three-level table above, computed twice. +- **Also gate `checkpoint_passive`'s fsync and the mode-switch page-1 + write-back** on `synchronous`. Deferred: neither is a per-commit hot + path, `#645`'s acceptance criteria only asks about "the correct + fsync call pattern" for commits, and stock SQLite's own checkpoint + fsync isn't skipped by `NORMAL` anyway — only `OFF` would change + anything there, a narrower case not worth the extra plumbing yet. +- **A general per-connection settings struct** (rather than fields + directly on `Pager`, mirroring `journal_mode`) to hold `synchronous` + and future PRAGMA-set state. Rejected as premature: there is + currently exactly one other stateful PRAGMA (`journal_mode`), it + already lives as a bare `Pager` field, and introducing a wrapper + struct for two fields is speculative until a third stateful PRAGMA + actually arrives. +- **`Pager::set_synchronous` re-checking for a pending transaction** + (mirroring `set_journal_mode`'s `PendingTransaction` guard). Rejected: + stock SQLite explicitly allows changing `synchronous` mid-transaction + (it only affects fsync behavior at the *next* commit, unlike + `journal_mode`, which needs a clean transaction boundary to safely + rewrite page 1's version bytes and swap journal implementations). + +## Consequences + +- `Pager::synchronous`/`Pager::set_synchronous` are the only two new + public `Pager` methods; no existing call site changes shape. +- `vdbe/pragma.rs::synchronous` is the first pragma executor to both + set state (mirroring `set_journal_mode`) and emit a result row + (mirroring `integrity_check`) from the same opcode, keyed off a + sentinel `P1` value (`SYNCHRONOUS_QUERY = -1`) rather than a second + opcode — kept as one opcode since the two forms share the same + "resolve the writer, if any" prologue. +- A future PRAGMA that also needs bidirectional get/set (unlike + `journal_mode`'s still-write-only shape) has a second precedent to + follow beyond `integrity_check`'s read-only one. +- `src/vfs/memory.rs`'s `MemoryVfs` gained a `sync_calls()` counter + (shared across clones and every file handle opened from it) purely + so `src/pager.rs`'s new fsync-gating tests can assert *whether* a + commit fsynced — an in-memory backend has no other way to observe + that. diff --git a/.openspec/adr/index.md b/.openspec/adr/index.md index 98cb39cf..0a93e2ea 100644 --- a/.openspec/adr/index.md +++ b/.openspec/adr/index.md @@ -39,3 +39,4 @@ Specs record what the system must do; ADRs record **why it is shaped this way** | [0033](0033-constant-propagation-and-or-to-in-extend-fast-paths-in-place.md) | Constant propagation and OR-to-IN extend existing equality fast paths in place; only genuine range seeks wait for a new opcode | 2026-08-28 | | [0034](0034-index-range-seeks.md) | Real-index range seeks for `BETWEEN`/`IN`/`LIKE`-prefix (`SeekIndexGE`/`IdxCompareGT`) | 2026-08-28 | | [0035](0035-wal-resume-hint-cache-supersedes-0026.md) | `Pager`-cached WAL resume hint supersedes ADR-0026's per-flush rescan | 2026-08-29 | +| [0036](0036-pragma-synchronous-fsync-policy.md) | `PRAGMA synchronous` fsync-skip policy, and why `SynchronousMode` lives in `header.rs` | 2026-08-29 | diff --git a/.openspec/grammar/sqlite.ebnf b/.openspec/grammar/sqlite.ebnf index 207f6858..609b8c39 100644 --- a/.openspec/grammar/sqlite.ebnf +++ b/.openspec/grammar/sqlite.ebnf @@ -345,26 +345,36 @@ drop-view-stmt ::= "DROP" "VIEW" [ "IF" "EXISTS" ] view-name ; view-name ::= identifier ; (* V6 *) (* ===================== PRAGMA journal_mode / integrity_check / quick_check - * (V6/V7 carve-outs) ===================== + * / synchronous (V6/V7 carve-outs) ===================== * [parse.y:1715 cmd ::= PRAGMA nm dbnm EQ nmnum, :1717 cmd ::= PRAGMA nm - * dbnm] -- `nmnum` (parse.y:1723) is where `DELETE` is folded in as a - * bare keyword value alongside a plain `nm` (identifier) value like - * `WAL`, which is why the `journal_mode` value alternation below needs - * its own two literal cases rather than one `identifier` production. + * dbnm] -- `nmnum` (parse.y:1723) is where `DELETE`/`FULL` are folded in + * as bare keyword values alongside a plain `nm` (identifier) value like + * `WAL`/`OFF`/`NORMAL`, which is why the `journal_mode`/`synchronous` + * value alternations below need their own literal cases for those two + * rather than one `identifier` production. * `integrity_check`/`quick_check` (#540, #541) are the bare * `PRAGMA nm dbnm` form (no `EQ`) with no result-set arguments -- * stock SQLite's `PRAGMA integrity_check(N)` (bounding the number of * errors reported) and any `dbnm` (schema-qualified) prefix are out of - * this carve-out's scope. General PRAGMA support -- every other pragma - * name, and journal_mode's other stock-SQLite values (MEMORY/OFF/ - * TRUNCATE/PERSIST) -- stays deferred to V7 (see Future blocks below); - * #388/#540/#541 carve out only these cases. + * this carve-out's scope. `synchronous` (#645) is the first pragma here + * that's genuinely bidirectional: `PRAGMA synchronous` (no `EQ`, same + * :1717 production as `integrity_check`) queries the connection's + * current level instead of changing it -- `journal_mode`'s own bare + * query form stays unimplemented (`Unsupported`). General PRAGMA + * support -- every other pragma name, journal_mode's other + * stock-SQLite values (MEMORY/OFF/TRUNCATE/PERSIST), and synchronous's + * `EXTRA`/`ON`/boolean aliases and out-of-0-2-range integers (with + * their own legacy masking quirks) -- stays deferred to V7 (see Future + * blocks below); #388/#540/#541/#645 carve out only these cases. *) pragma-stmt ::= "PRAGMA" "journal_mode" "=" ( "WAL" | "DELETE" ) - | "PRAGMA" ( "integrity_check" | "quick_check" ) ; + | "PRAGMA" ( "integrity_check" | "quick_check" ) + | "PRAGMA" "synchronous" [ "=" ( "OFF" | "NORMAL" | "FULL" + | "0" | "1" | "2" ) ] ; (* V6 [parse.y:1715 cmd] -- #388 *) (* V7 [parse.y:1717 cmd] -- #540, #541 *) + (* V7 [parse.y:1715,1717 cmd] -- #645 *) (* ===================== ANALYZE (V7 carve-out) ===================== * [parse.y:1875 cmd ::= ANALYZE, :1876 cmd ::= ANALYZE nm dbnm] -- diff --git a/Cargo.toml b/Cargo.toml index 7bfe3165..48e5f3ac 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -76,6 +76,10 @@ path = "tests/unit/transaction_parser.rs" name = "unit_pragma_parser" path = "tests/unit/pragma_parser.rs" +[[test]] +name = "unit_pragma_synchronous_repl" +path = "tests/unit/pragma_synchronous_repl.rs" + [[test]] name = "unit_introspection_pragmas" path = "tests/unit/introspection_pragmas.rs" diff --git a/src/bin/sqlite-rs/repl.rs b/src/bin/sqlite-rs/repl.rs index f81d832f..b6fdcd7d 100644 --- a/src/bin/sqlite-rs/repl.rs +++ b/src/bin/sqlite-rs/repl.rs @@ -376,7 +376,24 @@ fn run_one_statement( } }; match execute_transaction_step(&program, Rc::clone(pager), header, state.autocommit) { - Ok((_, ac)) => state.autocommit = ac, + Ok((rows, ac)) => { + state.autocommit = ac; + // #645: a non-`SELECT` statement can still emit result rows + // (e.g. `PRAGMA synchronous`'s bare query form, or + // `PRAGMA integrity_check`) — print them the same way the + // `SELECT` branch above does, rather than silently dropping + // them as this branch did before. No column names to derive + // here (this path has no `Select` AST to read them from), + // so `.headers on` renders a blank header line for these. + if !rows.is_empty() { + let mut stdout = io::BufWriter::new(io::stdout().lock()); + if let Err(e) = print_rows(&mut stdout, state.mode, state.headers, &[], &rows) { + eprintln!("Error: {e}"); + return; + } + stdout.flush().ok(); + } + } Err(e) => eprintln!("Error: {e}"), } } diff --git a/src/codegen/pragma.rs b/src/codegen/pragma.rs index da9ee48b..27045d4d 100644 --- a/src/codegen/pragma.rs +++ b/src/codegen/pragma.rs @@ -1,20 +1,25 @@ // Copyright 2026 Schuberg Philis // SPDX-License-Identifier: Apache-2.0 -//! `Pragma` AST -> `Program` compilation: `journal_mode` (#388) and -//! `integrity_check`/`quick_check` (#540, #541). Mirrors -//! `src/codegen/transaction.rs`'s shape: one control opcode per pragma, -//! operands carrying whatever the executor needs. +//! `Pragma` AST -> `Program` compilation: `journal_mode` (#388), +//! `integrity_check`/`quick_check` (#540, #541), and `synchronous` +//! (#645). Mirrors `src/codegen/transaction.rs`'s shape: one control +//! opcode per pragma, operands carrying whatever the executor needs. use crate::codegen::Emitter; -use crate::parser::ast::{Pragma, PragmaJournalMode}; -use crate::vdbe::{Instruction, Opcode, Program, JOURNAL_MODE_DELETE, JOURNAL_MODE_WAL}; +use crate::parser::ast::{Pragma, PragmaJournalMode, PragmaSynchronous}; +use crate::vdbe::{ + Instruction, Opcode, Program, JOURNAL_MODE_DELETE, JOURNAL_MODE_WAL, SYNCHRONOUS_FULL, + SYNCHRONOUS_NORMAL, SYNCHRONOUS_OFF, SYNCHRONOUS_QUERY, +}; /// Compiles a `PRAGMA` statement into an `Init -> -> Halt` program. /// `journal_mode` emits `SetJournalMode` (`P1` carries the target mode, /// no result rows); `integrity_check`/`quick_check` emit /// `IntegrityCheck` (`P1` = 1 for the `quick_check` reduced pass, 0 for /// the full `integrity_check`), which produces a result set of `TEXT` -/// rows. +/// rows; `synchronous` emits `Synchronous` (`P1` carries the target +/// level, or `SYNCHRONOUS_QUERY` for the bare query form, which +/// produces a single `INTEGER` result row instead of a side effect). pub fn compile_pragma(pragma: &Pragma) -> Program { let mut em = Emitter::new(); let init_addr = em.emit(Instruction::new(Opcode::Init, 0, 0, 0)); @@ -38,6 +43,15 @@ pub fn compile_pragma(pragma: &Pragma) -> Program { 0, )); } + Pragma::Synchronous { level, .. } => { + let p1 = match level { + None => SYNCHRONOUS_QUERY, + Some(PragmaSynchronous::Off) => SYNCHRONOUS_OFF, + Some(PragmaSynchronous::Normal) => SYNCHRONOUS_NORMAL, + Some(PragmaSynchronous::Full) => SYNCHRONOUS_FULL, + }; + em.emit(Instruction::new(Opcode::Synchronous, p1, 0, 0)); + } } em.emit(Instruction::new(Opcode::Halt, 0, 0, 0)); em.finish() @@ -108,4 +122,48 @@ mod tests { ); assert_eq!(program.instructions[1].p1, 1); } + + #[test] + fn synchronous_query_compiles_p1_sentinel() { + let pragma = match parse_pragma("PRAGMA synchronous") { + ParseOutcome::Accepted(p) => p, + other => panic!("expected Accepted, got {other:?}"), + }; + let program = compile_pragma(&pragma); + assert_eq!( + opcodes(&program), + vec![Opcode::Init, Opcode::Synchronous, Opcode::Halt] + ); + assert_eq!(program.instructions[1].p1, SYNCHRONOUS_QUERY); + } + + #[test] + fn synchronous_off_compiles_p1_zero() { + let pragma = match parse_pragma("PRAGMA synchronous = OFF") { + ParseOutcome::Accepted(p) => p, + other => panic!("expected Accepted, got {other:?}"), + }; + let program = compile_pragma(&pragma); + assert_eq!(program.instructions[1].p1, SYNCHRONOUS_OFF); + } + + #[test] + fn synchronous_normal_compiles_p1_one() { + let pragma = match parse_pragma("PRAGMA synchronous = NORMAL") { + ParseOutcome::Accepted(p) => p, + other => panic!("expected Accepted, got {other:?}"), + }; + let program = compile_pragma(&pragma); + assert_eq!(program.instructions[1].p1, SYNCHRONOUS_NORMAL); + } + + #[test] + fn synchronous_full_via_integer_compiles_p1_two() { + let pragma = match parse_pragma("PRAGMA synchronous = 2") { + ParseOutcome::Accepted(p) => p, + other => panic!("expected Accepted, got {other:?}"), + }; + let program = compile_pragma(&pragma); + assert_eq!(program.instructions[1].p1, SYNCHRONOUS_FULL); + } } diff --git a/src/header.rs b/src/header.rs index 83f36270..b7062179 100644 --- a/src/header.rs +++ b/src/header.rs @@ -111,6 +111,30 @@ pub enum JournalMode { Wal, } +/// `PRAGMA synchronous` (#645): how aggressively `Pager::flush` fsyncs +/// on commit. Unlike [`JournalMode`], this is never read from or +/// written to the header bytes — stock SQLite keeps it purely as +/// per-connection state, defaulting to `Full` on every fresh +/// connection, and so does `Pager`. Lives here (rather than +/// `src/pager.rs`, its otherwise-natural home) only so `vdbe/pragma.rs` +/// can name it without importing `crate::pager` directly, which spec +/// 001-architecture Requirement 1 ("VDBE does not know file format") +/// forbids — see ADR-0036. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +pub enum SynchronousMode { + /// No fsyncs at all on commit — fastest, least crash-safe. + Off = 0, + /// Skips the fsync(s) stock `FULL` performs that aren't needed for + /// basic crash *consistency* (as opposed to guaranteeing durability + /// of the most recent commits after a power loss). + Normal = 1, + /// fsyncs at every point needed to guarantee a commit survives a + /// crash or power loss — this pager's behavior before #645, and + /// still the default. + #[default] + Full = 2, +} + /// The parsed 100-byte SQLite database header. /// /// See the page-1 trap note in the module doc: this struct describes only diff --git a/src/pager.rs b/src/pager.rs index b46a4bfb..4fa37546 100644 --- a/src/pager.rs +++ b/src/pager.rs @@ -43,7 +43,7 @@ use std::hash::{BuildHasherDefault, Hasher}; use std::path::{Path, PathBuf}; use std::rc::Rc; -use crate::header::JournalMode; +use crate::header::{JournalMode, SynchronousMode}; use crate::vfs::{ companion_path, AnyVfs, AnyVfsFile, AnyWalShm, FileLock, PageError, PageSource, Vfs, VfsError, WritablePageSource, @@ -334,6 +334,10 @@ pub struct Pager { /// and WAL write paths, so it's tracked here rather than re-read off /// page 1 on every flush. journal_mode: JournalMode, + /// `PRAGMA synchronous` (#645) — defaults to `Full` on every fresh + /// [`Pager::open`], same as stock SQLite; never read from or + /// written to the database file. See [`SynchronousMode`]. + synchronous: SynchronousMode, } /// Byte offsets of the three header fields ([`crate::header::DatabaseHeader`]) @@ -451,6 +455,7 @@ impl Pager { db_path: path.to_path_buf(), journal_path, journal_mode, + synchronous: SynchronousMode::default(), }) } @@ -603,7 +608,13 @@ impl Pager { .write_record(index as u32, page_num, &original) .map_err(journal_to_pager_error)?; } - writer.sync().map_err(journal_to_pager_error)?; + // `PRAGMA synchronous` (#645): the journal fsync is skipped + // only at `Off` — `Normal` keeps it, since it's what lets a + // crash mid-write-to-the-main-file recover via + // `recover_hot_journal` at all (ADR-0036). + if self.synchronous != SynchronousMode::Off { + writer.sync().map_err(journal_to_pager_error)?; + } } for page_num in page_nums { @@ -611,7 +622,12 @@ impl Pager { self.source.write_page(page_num, bytes)?; } } - self.source.sync()?; + // `PRAGMA synchronous` (#645): the main-file fsync (the second + // of the two-fsync rollback-journal commit protocol) is the one + // `Normal` relaxes relative to `Full` (ADR-0036). + if self.synchronous == SynchronousMode::Full { + self.source.sync()?; + } if !to_journal.is_empty() { self.vfs.delete(&self.journal_path)?; @@ -713,7 +729,13 @@ impl Pager { .map_err(to_pager_error)?; } } - writer.sync().map_err(to_pager_error)?; + // `PRAGMA synchronous` (#645): `Full` fsyncs the WAL on every + // commit; `Normal`/`Off` don't — matching stock SQLite's + // documented WAL+NORMAL behavior of only syncing at + // checkpoint boundaries (ADR-0036). + if self.synchronous == SynchronousMode::Full { + writer.sync().map_err(to_pager_error)?; + } self.wal_resume = Some(writer.resume_hint()); let new_mx_frame = writer.frame_count(); @@ -804,6 +826,22 @@ impl Pager { Ok(()) } + /// The active `PRAGMA synchronous` level (#645), consulted by + /// [`Pager::flush_locked`]/[`Pager::flush_wal_locked`] to decide + /// which commit-time fsyncs to skip. + pub fn synchronous(&self) -> SynchronousMode { + self.synchronous + } + + /// Sets the active `PRAGMA synchronous` level (#645). Unlike + /// [`Pager::set_journal_mode`], this has no on-disk representation + /// to flip and no pending-transaction restriction — stock SQLite + /// allows changing it at any time, including mid-transaction, since + /// it only affects fsync behavior at the *next* commit. + pub fn set_synchronous(&mut self, mode: SynchronousMode) { + self.synchronous = mode; + } + /// Recovers [`Pager::flush_wal_locked`] from a `-wal` that vanished out /// from under it (#422) — e.g. a concurrent `sqlite3` connection /// auto-checkpointed and deleted `-wal`/`-shm` on close. Creates a @@ -1648,6 +1686,120 @@ mod tests { assert_eq!(pager.read_page(1).unwrap(), Rc::from(vec![7u8; 512])); } + #[test] + fn synchronous_defaults_to_full() { + let mut vfs = MemoryVfs::new(); + vfs.insert("/test.db", vec![1u8; 512]); + let pager = Pager::open(&vfs, Path::new("/test.db"), 512).unwrap(); + assert_eq!(pager.synchronous(), SynchronousMode::Full); + } + + #[test] + fn set_synchronous_roundtrips_and_is_never_a_pending_transaction_error() { + let mut vfs = MemoryVfs::new(); + let mut contents = vec![1u8; 512]; + contents.extend(vec![2u8; 512]); + vfs.insert("/test.db", contents); + let mut pager = Pager::open(&vfs, Path::new("/test.db"), 512).unwrap(); + // Unlike `set_journal_mode`, allowed even with a dirty page — + // stock SQLite lets `synchronous` change mid-transaction (#645). + pager.get_page_mut(2).unwrap().fill(9u8); + pager.set_synchronous(SynchronousMode::Off); + assert_eq!(pager.synchronous(), SynchronousMode::Off); + } + + /// #645/ADR-0036: `Full` (the default) fsyncs both the journal and + /// the main file on a rollback-journal commit — the two-fsync + /// protocol this pager used unconditionally before `synchronous` + /// existed. + #[test] + fn synchronous_full_syncs_journal_and_main_file_on_rollback_commit() { + let mut vfs = MemoryVfs::new(); + let mut contents = vec![1u8; 512]; + contents.extend(vec![2u8; 512]); + vfs.insert("/test.db", contents); + let mut pager = Pager::open(&vfs, Path::new("/test.db"), 512).unwrap(); + + pager.get_page_mut(2).unwrap().fill(9u8); + let before = vfs.sync_calls(); + pager.flush().unwrap(); + assert_eq!( + vfs.sync_calls() - before, + 2, + "journal fsync + main-file fsync" + ); + } + + /// #645/ADR-0036: `Normal` keeps the journal fsync (still needed for + /// `recover_hot_journal` to be safe) but skips the main-file fsync. + #[test] + fn synchronous_normal_skips_main_file_sync_on_rollback_commit() { + let mut vfs = MemoryVfs::new(); + let mut contents = vec![1u8; 512]; + contents.extend(vec![2u8; 512]); + vfs.insert("/test.db", contents); + let mut pager = Pager::open(&vfs, Path::new("/test.db"), 512).unwrap(); + pager.set_synchronous(SynchronousMode::Normal); + + pager.get_page_mut(2).unwrap().fill(9u8); + let before = vfs.sync_calls(); + pager.flush().unwrap(); + assert_eq!(vfs.sync_calls() - before, 1, "journal fsync only"); + } + + /// #645/ADR-0036: `Off` skips every commit-time fsync. + #[test] + fn synchronous_off_skips_all_syncs_on_rollback_commit() { + let mut vfs = MemoryVfs::new(); + let mut contents = vec![1u8; 512]; + contents.extend(vec![2u8; 512]); + vfs.insert("/test.db", contents); + let mut pager = Pager::open(&vfs, Path::new("/test.db"), 512).unwrap(); + pager.set_synchronous(SynchronousMode::Off); + + pager.get_page_mut(2).unwrap().fill(9u8); + let before = vfs.sync_calls(); + pager.flush().unwrap(); + assert_eq!(vfs.sync_calls() - before, 0); + } + + /// #645/ADR-0036: `Full` fsyncs the WAL on every commit. + #[test] + fn synchronous_full_syncs_wal_frame_on_commit() { + let mut vfs = MemoryVfs::new(); + let mut contents = vec![1u8; 512]; + write_be_u32(&mut contents, PAGE_COUNT_OFFSET, 2).unwrap(); + contents.extend(vec![2u8; 512]); + vfs.insert("/test.db", contents); + let mut pager = Pager::open(&vfs, Path::new("/test.db"), 512).unwrap(); + pager.set_journal_mode(JournalMode::Wal).unwrap(); + + pager.get_page_mut(2).unwrap().fill(9u8); + let before = vfs.sync_calls(); + pager.flush().unwrap(); + assert_eq!(vfs.sync_calls() - before, 1, "WAL frame fsync"); + } + + /// #645/ADR-0036: `Normal`/`Off` in WAL mode skip the per-commit + /// frame fsync — matching stock SQLite's documented behavior of only + /// syncing WAL+NORMAL at checkpoint boundaries. + #[test] + fn synchronous_normal_skips_wal_frame_sync_on_commit() { + let mut vfs = MemoryVfs::new(); + let mut contents = vec![1u8; 512]; + write_be_u32(&mut contents, PAGE_COUNT_OFFSET, 2).unwrap(); + contents.extend(vec![2u8; 512]); + vfs.insert("/test.db", contents); + let mut pager = Pager::open(&vfs, Path::new("/test.db"), 512).unwrap(); + pager.set_journal_mode(JournalMode::Wal).unwrap(); + pager.set_synchronous(SynchronousMode::Normal); + + pager.get_page_mut(2).unwrap().fill(9u8); + let before = vfs.sync_calls(); + pager.flush().unwrap(); + assert_eq!(vfs.sync_calls() - before, 0); + } + /// #388: `PRAGMA journal_mode=WAL` creates a fresh `-wal`/`-shm` and /// flips page 1's write/read-version bytes (18/19) to `2, 2`. #[test] diff --git a/src/parser/ast.rs b/src/parser/ast.rs index 70841970..36a21d75 100644 --- a/src/parser/ast.rs +++ b/src/parser/ast.rs @@ -787,10 +787,25 @@ pub enum PragmaJournalMode { Delete, } +/// The three `synchronous` levels `pragma-stmt` (grammar V7 carve-out, +/// #645) accepts — stock SQLite's `synchronous` pragma also takes +/// `EXTRA` and the `ON`/boolean aliases, plus arbitrary integers beyond +/// 0-2 (with its own legacy masking quirks), all deferred to general +/// PRAGMA support. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum PragmaSynchronous { + /// `PRAGMA synchronous = OFF` (or `= 0`). + Off, + /// `PRAGMA synchronous = NORMAL` (or `= 1`). + Normal, + /// `PRAGMA synchronous = FULL` (or `= 2`). + Full, +} + /// A parsed `PRAGMA` statement: the narrow V6 `journal_mode` carve-out -/// (#388) plus the V7 `integrity_check`/`quick_check` carve-out (#540, -/// #541). Every other pragma name stays `Unsupported` at the parser -/// (see `parse_pragma_stmt`). +/// (#388), the V7 `integrity_check`/`quick_check` carve-out (#540, +/// #541), and the V7 `synchronous` carve-out (#645). Every other pragma +/// name stays `Unsupported` at the parser (see `parse_pragma_stmt`). #[derive(Debug, Clone, PartialEq)] pub enum Pragma { /// `PRAGMA journal_mode = WAL|DELETE` (#388); see [`PragmaJournalMode`]. @@ -809,13 +824,26 @@ pub enum Pragma { /// The source span covering the whole statement. span: Span, }, + /// `PRAGMA synchronous [= OFF|NORMAL|FULL|0|1|2]` (#645). `level = + /// None` is the bare query form (`PRAGMA synchronous`, no `=`), + /// which reports the connection's current level as a result row + /// instead of changing it — unlike `journal_mode`, whose bare query + /// form the parser still rejects as `Unsupported`. + Synchronous { + /// The requested level, or `None` to query the current one. + level: Option, + /// The source span covering the whole statement. + span: Span, + }, } impl Pragma { /// The source span covering the whole statement, whichever variant. pub fn span(&self) -> Span { match self { - Pragma::JournalMode { span, .. } | Pragma::IntegrityCheck { span, .. } => *span, + Pragma::JournalMode { span, .. } + | Pragma::IntegrityCheck { span, .. } + | Pragma::Synchronous { span, .. } => *span, } } } diff --git a/src/parser/grammar.rs b/src/parser/grammar.rs index 6b35dfc1..b2f37820 100644 --- a/src/parser/grammar.rs +++ b/src/parser/grammar.rs @@ -941,6 +941,22 @@ impl Parser { span: join_span(start, name_span), }); } + if name.eq_ignore_ascii_case("synchronous") { + // Unlike `journal_mode`, the bare query form (no `=`) *is* + // implemented (#645) -- it reports the connection's current + // level rather than changing it. + if !self.eat_punct(&TokenKind::Eq) { + return Ok(Pragma::Synchronous { + level: None, + span: join_span(start, name_span), + }); + } + let (level, end) = self.pragma_synchronous_value()?; + return Ok(Pragma::Synchronous { + level: Some(level), + span: join_span(start, end), + }); + } if !name.eq_ignore_ascii_case("journal_mode") { return self.unsupported(format!("pragma {name:?} not yet supported")); } @@ -978,6 +994,33 @@ impl Parser { } } + /// `OFF`/`NORMAL`/`FULL` (case-insensitive identifiers) or the + /// equivalent `0`/`1`/`2` integer literal (#645). Stock SQLite also + /// accepts `EXTRA`, `ON`/boolean aliases, and out-of-range integers + /// (with its own legacy masking quirks) -- all deferred, same as + /// `journal_mode`'s own narrower-than-stock carve-out. + fn pragma_synchronous_value(&mut self) -> PResult<(PragmaSynchronous, Span)> { + match self.peek().kind.clone() { + TokenKind::Identifier(text) if text.eq_ignore_ascii_case("off") => { + Ok((PragmaSynchronous::Off, self.advance_span())) + } + TokenKind::Identifier(text) if text.eq_ignore_ascii_case("normal") => { + Ok((PragmaSynchronous::Normal, self.advance_span())) + } + // `FULL` is a reserved keyword (used in `FULL [OUTER] JOIN`), + // never tokenized as a plain identifier -- same reason + // `journal_mode`'s `DELETE` value needs its own keyword + // match arm rather than falling out of `identifier()`. + TokenKind::Keyword(Keyword::FULL) => Ok((PragmaSynchronous::Full, self.advance_span())), + TokenKind::Integer(0) => Ok((PragmaSynchronous::Off, self.advance_span())), + TokenKind::Integer(1) => Ok((PragmaSynchronous::Normal, self.advance_span())), + TokenKind::Integer(2) => Ok((PragmaSynchronous::Full, self.advance_span())), + _ => self.unsupported( + "unsupported synchronous value (only OFF/NORMAL/FULL/0/1/2 are supported)", + ), + } + } + /// `analyze-stmt` (#461, grammar V7 carve-out): `ANALYZE` or `ANALYZE /// table-name`. Only a single bare identifier (a table name) is /// accepted; a qualified `schema-name.table-name` form parses far diff --git a/src/vdbe.rs b/src/vdbe.rs index b628dd2d..39e3dad1 100644 --- a/src/vdbe.rs +++ b/src/vdbe.rs @@ -40,7 +40,10 @@ pub use exec::{ }; pub use explain::{explain, ExplainRow}; pub use functions::{call as call_function, like_match, FunctionError}; -pub use pragma::{JOURNAL_MODE_DELETE, JOURNAL_MODE_WAL}; +pub use pragma::{ + JOURNAL_MODE_DELETE, JOURNAL_MODE_WAL, SYNCHRONOUS_FULL, SYNCHRONOUS_NORMAL, SYNCHRONOUS_OFF, + SYNCHRONOUS_QUERY, +}; pub use program::{ AnalyzeIndexTarget, AnalyzeTarget, GroupKeyColumn, Instruction, Opcode, Program, SortKeyColumn, P4, diff --git a/src/vdbe/exec.rs b/src/vdbe/exec.rs index 11603747..8cef6e72 100644 --- a/src/vdbe/exec.rs +++ b/src/vdbe/exec.rs @@ -714,7 +714,7 @@ fn dispatch(vm: &mut Vm, pc: usize, instr: &Instruction) -> Result control::init(instr), @@ -727,6 +727,7 @@ fn dispatch(vm: &mut Vm, pc: usize, instr: &Instruction) -> Result control::auto_commit(vm, instr), SetJournalMode => pragma::set_journal_mode(vm, instr), IntegrityCheck => pragma::integrity_check(vm, instr), + Synchronous => pragma::synchronous(vm, instr), IfNot => control::if_not(vm, instr), IfNotZero => control::if_not_zero(vm, instr), IfPos => control::if_pos(vm, instr), diff --git a/src/vdbe/explain.rs b/src/vdbe/explain.rs index 66dc7d21..1a278f99 100644 --- a/src/vdbe/explain.rs +++ b/src/vdbe/explain.rs @@ -145,6 +145,7 @@ fn opcode_name(opcode: Opcode) -> &'static str { Opcode::AutoCommit => "AutoCommit", Opcode::SetJournalMode => "SetJournalMode", Opcode::IntegrityCheck => "IntegrityCheck", + Opcode::Synchronous => "Synchronous", Opcode::IfNot => "IfNot", Opcode::IfNotZero => "IfNotZero", Opcode::IfPos => "IfPos", @@ -366,6 +367,7 @@ mod tests { (Opcode::AutoCommit, "AutoCommit"), (Opcode::SetJournalMode, "SetJournalMode"), (Opcode::IntegrityCheck, "IntegrityCheck"), + (Opcode::Synchronous, "Synchronous"), (Opcode::IfNot, "IfNot"), (Opcode::IfNotZero, "IfNotZero"), (Opcode::IfPos, "IfPos"), diff --git a/src/vdbe/pragma.rs b/src/vdbe/pragma.rs index 1f3f80b9..ebc5478e 100644 --- a/src/vdbe/pragma.rs +++ b/src/vdbe/pragma.rs @@ -8,7 +8,7 @@ //! mode switch mid-transaction errors with a clear message rather than //! being silently applied. -use crate::header::JournalMode; +use crate::header::{JournalMode, SynchronousMode}; use crate::integrity::run_integrity_check; use crate::record::Value; use crate::vdbe::exec::{ExecError, Step, Vm}; @@ -21,6 +21,19 @@ pub const JOURNAL_MODE_DELETE: i32 = 0; /// See [`JOURNAL_MODE_DELETE`]. pub const JOURNAL_MODE_WAL: i32 = 1; +/// `Instruction::p1` values `compile_pragma`/`synchronous` (#645) use to +/// carry the target [`SynchronousMode`] (or the bare-query-form +/// sentinel) through the `Synchronous` opcode. +pub const SYNCHRONOUS_OFF: i32 = 0; +/// See [`SYNCHRONOUS_OFF`]. +pub const SYNCHRONOUS_NORMAL: i32 = 1; +/// See [`SYNCHRONOUS_OFF`]. +pub const SYNCHRONOUS_FULL: i32 = 2; +/// `PRAGMA synchronous` (bare, no `=`): query the current level rather +/// than set it. Distinct from every real level (`0`/`1`/`2`), never +/// mistakable for one. +pub const SYNCHRONOUS_QUERY: i32 = -1; + /// `SetJournalMode`: switches the pager's on-disk journal mode via /// [`crate::pager::Pager::set_journal_mode`]. Errors with /// [`ExecError::JournalModeChangeDuringTransaction`] rather than @@ -50,6 +63,39 @@ pub fn set_journal_mode(vm: &mut Vm, instr: &Instruction) -> Result Result { + let writer = vm.db().ok().and_then(|db| db.writer.clone()); + if instr.p1 == SYNCHRONOUS_QUERY { + let mode = writer.map_or(SynchronousMode::default(), |writer| { + writer.borrow().synchronous() + }); + vm.emit_row(vec![Value::Integer(mode as i64)]); + return Ok(Step::Next); + } + let mode = match instr.p1 { + SYNCHRONOUS_OFF => SynchronousMode::Off, + SYNCHRONOUS_NORMAL => SynchronousMode::Normal, + _ => SynchronousMode::Full, + }; + if let Some(writer) = writer { + writer.borrow_mut().set_synchronous(mode); + } + Ok(Step::Next) +} + /// `IntegrityCheck` (#540, #541): `P1` is 1 for `quick_check`, 0 for the /// full `integrity_check`. Runs [`run_integrity_check`] against the /// attached database's `source`/`header` and emits one `TEXT` result row @@ -122,4 +168,49 @@ mod tests { let instr = Instruction::new(Opcode::SetJournalMode, JOURNAL_MODE_WAL, 0, 0); assert_eq!(set_journal_mode(&mut vm, &instr).unwrap(), Step::Next); } + + #[test] + fn synchronous_defaults_to_full_on_query() { + let mut vm = writable_vm(); + let instr = Instruction::new(Opcode::Synchronous, SYNCHRONOUS_QUERY, 0, 0); + assert_eq!(synchronous(&mut vm, &instr).unwrap(), Step::Next); + assert_eq!(vm.rows().to_vec(), vec![vec![Value::Integer(2)]]); + } + + #[test] + fn synchronous_set_then_query_round_trips() { + let mut vm = writable_vm(); + let set_instr = Instruction::new(Opcode::Synchronous, SYNCHRONOUS_OFF, 0, 0); + assert_eq!(synchronous(&mut vm, &set_instr).unwrap(), Step::Next); + let writer = vm.db().unwrap().writer.clone().unwrap(); + assert_eq!(writer.borrow().synchronous(), SynchronousMode::Off); + + let query_instr = Instruction::new(Opcode::Synchronous, SYNCHRONOUS_QUERY, 0, 0); + assert_eq!(synchronous(&mut vm, &query_instr).unwrap(), Step::Next); + assert_eq!(vm.rows().to_vec(), vec![vec![Value::Integer(0)]]); + } + + #[test] + fn synchronous_normal_sets_level_one() { + let mut vm = writable_vm(); + let instr = Instruction::new(Opcode::Synchronous, SYNCHRONOUS_NORMAL, 0, 0); + assert_eq!(synchronous(&mut vm, &instr).unwrap(), Step::Next); + let writer = vm.db().unwrap().writer.clone().unwrap(); + assert_eq!(writer.borrow().synchronous(), SynchronousMode::Normal); + } + + #[test] + fn synchronous_query_with_no_writable_db_reports_default_full() { + let mut vm = VmType::new(); + let instr = Instruction::new(Opcode::Synchronous, SYNCHRONOUS_QUERY, 0, 0); + assert_eq!(synchronous(&mut vm, &instr).unwrap(), Step::Next); + assert_eq!(vm.rows().to_vec(), vec![vec![Value::Integer(2)]]); + } + + #[test] + fn synchronous_set_with_no_writable_db_is_a_no_op() { + let mut vm = VmType::new(); + let instr = Instruction::new(Opcode::Synchronous, SYNCHRONOUS_OFF, 0, 0); + assert_eq!(synchronous(&mut vm, &instr).unwrap(), Step::Next); + } } diff --git a/src/vdbe/program.rs b/src/vdbe/program.rs index 383dd34b..0763430a 100644 --- a/src/vdbe/program.rs +++ b/src/vdbe/program.rs @@ -59,6 +59,16 @@ pub enum Opcode { /// 0 for the full `integrity_check`. Emits one `TEXT` result row per /// problem found, or a single `"ok"` row if none. IntegrityCheck, + // #645: `PRAGMA synchronous [= OFF|NORMAL|FULL]` -- postdates the V2 + // oracle harvest, so excluded from `ALL` but fully dispatched and + // exhaustiveness-checked, like `SetJournalMode`/`IntegrityCheck` + // above. + /// `PRAGMA synchronous [= OFF|NORMAL|FULL]`: `P1` carries the target + /// level (`crate::vdbe::pragma::{SYNCHRONOUS_OFF, SYNCHRONOUS_NORMAL, + /// SYNCHRONOUS_FULL}`), or `crate::vdbe::pragma::SYNCHRONOUS_QUERY` + /// for the bare query form, which emits the connection's current + /// level as a single `INTEGER` result row instead of changing it. + Synchronous, /// Jumps to `P2` if register `P1` is falsy (zero/false, per SQLite's /// truthiness rules). IfNot, @@ -517,6 +527,7 @@ fn _exhaustive(o: Opcode) { | Opcode::AutoCommit | Opcode::SetJournalMode | Opcode::IntegrityCheck + | Opcode::Synchronous | Opcode::IfNot | Opcode::IfNotZero | Opcode::IfPos diff --git a/src/vfs/memory.rs b/src/vfs/memory.rs index 41b50c8d..90aaf282 100644 --- a/src/vfs/memory.rs +++ b/src/vfs/memory.rs @@ -4,6 +4,7 @@ use std::collections::HashMap; use std::path::{Path, PathBuf}; +use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::{Arc, Mutex}; use super::{FileLock, Result, SharedLockGuard, Vfs, VfsError, VfsFile}; @@ -22,6 +23,13 @@ type FileTable = Arc>>>>>; #[derive(Debug, Default, Clone)] pub struct MemoryVfs { files: FileTable, + /// Total `VfsFile::sync` calls across every file handle this `Vfs` + /// (or a clone of it) has opened — `Arc`-shared like `files`, so it + /// stays visible from the original handle after `Pager::open` + /// clones it. Exists purely so `PRAGMA synchronous` (#645) tests + /// can assert *whether* a commit fsynced, since an in-memory + /// backend has no real fsync effect to observe otherwise. + sync_calls: Arc, } impl MemoryVfs { @@ -39,6 +47,14 @@ impl MemoryVfs { files.insert(path.into(), Arc::new(Mutex::new(contents))); } + /// Total `VfsFile::sync` calls across every file this `Vfs` (or a + /// clone of it) has opened so far — never reset, so callers + /// snapshot the count before and after the operation under test and + /// compare the delta (#645). + pub fn sync_calls(&self) -> usize { + self.sync_calls.load(Ordering::SeqCst) + } + fn handle(&self, path: &Path) -> Result>>> { let files = self.files.lock().map_err(|_| poisoned(path))?; files.get(path).cloned().ok_or_else(|| VfsError::NotFound { @@ -49,11 +65,17 @@ impl MemoryVfs { impl Vfs for MemoryVfs { fn open_read(&self, path: &Path) -> Result> { - Ok(Box::new(MemoryVfsFile(self.handle(path)?))) + Ok(Box::new(MemoryVfsFile( + self.handle(path)?, + self.sync_calls.clone(), + ))) } fn open_write(&self, path: &Path) -> Result> { - Ok(Box::new(MemoryVfsFile(self.handle(path)?))) + Ok(Box::new(MemoryVfsFile( + self.handle(path)?, + self.sync_calls.clone(), + ))) } fn exists(&self, path: &Path) -> Result { @@ -69,7 +91,7 @@ impl Vfs for MemoryVfs { .or_insert_with(|| Arc::new(Mutex::new(Vec::new()))) .clone() }; - Ok(Box::new(MemoryVfsFile(handle))) + Ok(Box::new(MemoryVfsFile(handle, self.sync_calls.clone()))) } fn delete(&self, path: &Path) -> Result<()> { @@ -79,7 +101,7 @@ impl Vfs for MemoryVfs { } } -struct MemoryVfsFile(Arc>>); +struct MemoryVfsFile(Arc>>, Arc); /// The in-memory backend's `Mutex`es are only ever contended within a /// single test process and never cross a panic boundary while held, so a @@ -141,6 +163,7 @@ impl VfsFile for MemoryVfsFile { } fn sync(&self) -> Result<()> { + self.1.fetch_add(1, Ordering::SeqCst); Ok(()) } } diff --git a/tests/unit/pragma_parser.rs b/tests/unit/pragma_parser.rs index a8297b0a..c4873465 100644 --- a/tests/unit/pragma_parser.rs +++ b/tests/unit/pragma_parser.rs @@ -1,9 +1,9 @@ // Copyright 2026 Schuberg Philis // SPDX-License-Identifier: Apache-2.0 //! Unit tests for the PRAGMA parser carve-outs: `journal_mode` (#388), -//! `integrity_check`/`quick_check` (#540, #541). Any other pragma name -//! or value is `Unsupported`, not a hard parse error -- mirrors `WITH -//! RECURSIVE`'s precedent. +//! `integrity_check`/`quick_check` (#540, #541), `synchronous` (#645). +//! Any other pragma name or value is `Unsupported`, not a hard parse +//! error -- mirrors `WITH RECURSIVE`'s precedent. #![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] @@ -31,6 +31,13 @@ fn journal_mode(p: &Pragma) -> PragmaJournalMode { } } +fn synchronous_level(p: &Pragma) -> Option { + match p { + Pragma::Synchronous { level, .. } => *level, + other => panic!("expected Synchronous, got {other:?}"), + } +} + // ---- accepted: journal_mode ------------------------------------------------ #[test] @@ -98,6 +105,48 @@ fn test_accept_integrity_check_is_case_insensitive() { ); } +// ---- accepted: synchronous -------------------------------------------------- + +#[test] +fn test_accept_synchronous_query_form() { + let p = accept("PRAGMA synchronous"); + assert_eq!(synchronous_level(&p), None); +} + +#[test] +fn test_accept_synchronous_off() { + let p = accept("PRAGMA synchronous = OFF"); + assert_eq!(synchronous_level(&p), Some(PragmaSynchronous::Off)); +} + +#[test] +fn test_accept_synchronous_normal() { + let p = accept("PRAGMA synchronous = NORMAL"); + assert_eq!(synchronous_level(&p), Some(PragmaSynchronous::Normal)); +} + +#[test] +fn test_accept_synchronous_full() { + let p = accept("PRAGMA synchronous = FULL"); + assert_eq!(synchronous_level(&p), Some(PragmaSynchronous::Full)); +} + +#[test] +fn test_accept_synchronous_integer_values() { + let p = accept("PRAGMA synchronous = 0"); + assert_eq!(synchronous_level(&p), Some(PragmaSynchronous::Off)); + let p = accept("PRAGMA synchronous = 1"); + assert_eq!(synchronous_level(&p), Some(PragmaSynchronous::Normal)); + let p = accept("PRAGMA synchronous = 2"); + assert_eq!(synchronous_level(&p), Some(PragmaSynchronous::Full)); +} + +#[test] +fn test_accept_synchronous_is_case_insensitive() { + let p = accept("pragma SYNCHRONOUS = full"); + assert_eq!(synchronous_level(&p), Some(PragmaSynchronous::Full)); +} + // ---- unsupported ------------------------------------------------------------ #[test] @@ -129,3 +178,11 @@ fn test_unsupported_bare_journal_mode_query_form() { fn test_unsupported_integrity_check_with_arg() { unsupported("PRAGMA integrity_check(10)"); } + +#[test] +fn test_unsupported_synchronous_value() { + unsupported("PRAGMA synchronous = ON"); + unsupported("PRAGMA synchronous = EXTRA"); + unsupported("PRAGMA synchronous = 3"); + unsupported("PRAGMA synchronous = 4"); +} diff --git a/tests/unit/pragma_synchronous_repl.rs b/tests/unit/pragma_synchronous_repl.rs new file mode 100644 index 00000000..13208cec --- /dev/null +++ b/tests/unit/pragma_synchronous_repl.rs @@ -0,0 +1,160 @@ +// Copyright 2026 Schuberg Philis +// SPDX-License-Identifier: Apache-2.0 +#![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] + +//! CLI-surface oracle-parity test for `PRAGMA synchronous` (#645): +//! `PRAGMA synchronous = ; PRAGMA synchronous;` round-trips to +//! the numeric value just set, matching stock `sqlite3`. Driven through +//! `repl` (not `exec`, which deliberately never prints result rows — +//! see its module doc — and not `query`, which only accepts a single +//! `SELECT`/introspection-pragma statement) via the same piped-stdin +//! pattern `tests/unit/repl_dot_commands.rs` uses. + +#[path = "../corpus/oracle.rs"] +#[allow(dead_code)] +mod oracle; + +use std::io::Write; +use std::path::{Path, PathBuf}; +use std::process::{Command, Output, Stdio}; +use std::sync::atomic::{AtomicU64, Ordering}; + +use oracle::{pinned_oracle, run_oracle, skip_no_oracle}; + +const CLI: &str = env!("CARGO_BIN_EXE_sqlite-rs"); + +fn scratch_db(label: &str) -> PathBuf { + static COUNTER: AtomicU64 = AtomicU64::new(0); + let n = COUNTER.fetch_add(1, Ordering::Relaxed); + let dir = std::env::temp_dir().join(format!( + "sqlite-rs-pragma-synchronous-{label}-{}-{n}", + std::process::id() + )); + std::fs::remove_dir_all(&dir).ok(); + std::fs::create_dir_all(&dir).unwrap(); + dir.join("scratch.db") +} + +/// A scratch db seeded with one bootstrap table — `PRAGMA synchronous` +/// itself needs no schema, but `repl`/`exec` (unlike stock `sqlite3`) +/// never create the file, so it must already exist. +fn seed_db(label: &str) -> PathBuf { + let db = scratch_db(label); + if let Some(oracle) = pinned_oracle() { + let status = Command::new(&oracle) + .arg(&db) + .arg("CREATE TABLE seed_bootstrap(x)") + .status() + .unwrap(); + assert!(status.success(), "seeding via oracle failed"); + } else { + let out = Command::new(CLI) + .arg("exec") + .arg(&db) + .arg("CREATE TABLE seed_bootstrap(x)") + .output() + .unwrap(); + assert!(out.status.success(), "seeding via our own exec failed"); + } + db +} + +fn run_repl_script(db: &Path, script: &str) -> Output { + let mut child = Command::new(CLI) + .arg("repl") + .arg(db) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .unwrap_or_else(|e| panic!("spawning {CLI} repl {}: {e}", db.display())); + child + .stdin + .take() + .expect("piped stdin") + .write_all(script.as_bytes()) + .expect("writing repl script"); + child.wait_with_output().expect("waiting for repl") +} + +fn stdout_of(db: &Path, script: &str) -> String { + let out = run_repl_script(db, script); + assert!( + out.status.success(), + "repl script failed: stderr={}", + String::from_utf8_lossy(&out.stderr) + ); + String::from_utf8_lossy(&out.stdout).into_owned() +} + +/// Strips the interactive `sqlite> ` prompts `repl` prints between +/// statements, so its output can be diffed against a non-interactive +/// oracle invocation (which never emits a prompt at all). +fn strip_prompts(s: &str) -> String { + s.replace("sqlite> ", "").replace("sqlite>", "") +} + +#[test] +fn synchronous_query_defaults_to_full() { + let db = seed_db("default"); + let out = stdout_of(&db, "PRAGMA synchronous;\n.quit\n"); + assert!(out.contains('2'), "{out}"); +} + +#[test] +fn synchronous_off_then_query_round_trips() { + let db = seed_db("off"); + let out = stdout_of( + &db, + "PRAGMA synchronous = OFF;\nPRAGMA synchronous;\n.quit\n", + ); + assert!(out.contains('0'), "{out}"); +} + +#[test] +fn synchronous_normal_then_query_round_trips() { + let db = seed_db("normal"); + let out = stdout_of( + &db, + "PRAGMA synchronous = NORMAL;\nPRAGMA synchronous;\n.quit\n", + ); + assert!(out.contains('1'), "{out}"); +} + +#[test] +fn synchronous_full_then_query_round_trips() { + let db = seed_db("full"); + let out = stdout_of( + &db, + "PRAGMA synchronous = FULL;\nPRAGMA synchronous;\n.quit\n", + ); + assert!(out.contains('2'), "{out}"); +} + +/// Diffs the round-trip against the real `sqlite3` oracle: same +/// sequence of statements, same reported numeric level. +#[test] +fn synchronous_round_trip_matches_oracle() { + let Some(oracle) = pinned_oracle() else { + skip_no_oracle("synchronous_round_trip_matches_oracle"); + return; + }; + let db = seed_db("oracle-diff"); + for (level, expected) in [("OFF", "0"), ("NORMAL", "1"), ("FULL", "2")] { + let ours = stdout_of( + &db, + &format!("PRAGMA synchronous = {level};\nPRAGMA synchronous;\n.quit\n"), + ); + let theirs = run_oracle( + &oracle, + &db, + &[], + &format!("PRAGMA synchronous = {level}; PRAGMA synchronous;"), + ); + assert_eq!( + strip_prompts(&ours).trim(), + theirs.trim(), + "level {level}: expected {expected}" + ); + } +}