From fa622071ecd9ea9ba68d6f9688614a1c6292fcf2 Mon Sep 17 00:00:00 2001 From: Jacob Verhoeks Date: Fri, 28 Aug 2026 14:33:15 +0200 Subject: [PATCH 1/5] feature: spec extension for sqe --- ...-owns-the-connection-driver-out-of-tree.md | 103 ++++++ .openspec/specs/012-embedding-api/spec.md | 347 ++++++++++++++++++ 2 files changed, 450 insertions(+) create mode 100644 .openspec/adr/0033-embedding-api-owns-the-connection-driver-out-of-tree.md create mode 100644 .openspec/specs/012-embedding-api/spec.md diff --git a/.openspec/adr/0033-embedding-api-owns-the-connection-driver-out-of-tree.md b/.openspec/adr/0033-embedding-api-owns-the-connection-driver-out-of-tree.md new file mode 100644 index 00000000..258aeef2 --- /dev/null +++ b/.openspec/adr/0033-embedding-api-owns-the-connection-driver-out-of-tree.md @@ -0,0 +1,103 @@ +# 0033 — The embedding API owns the connection; the `sqlx` driver stays out of tree + +**Status:** Proposed · **Date:** 2026-08-28 + +## Context + +`examples/README.md`: "this crate exposes its parser/codegen/VM pipeline +directly rather than an ergonomic `Connection`/`prepare`/`bind` wrapper, so each +example wires those pieces together the same way the `sqlite-rs` CLI binary +does." + +The pieces are built. `Vm::bind_params` and the `Variable` opcode (spec 009, +ADR-0015), `compile_statement`, the autocommit state `execute_transaction_step` +threads, and `Pager`'s file locks (spec 007). Creating a database shows the +shape of what is left: the primitive is public +(`DatabaseHeader::new_empty_page1`) and the CLI calls it, but no API offers it. +Missing, then: a facade, a `Send + Sync` boundary, and one counter. + +The counter is the only genuine capability gap. Nothing in `src/vdbe/` reports +rows changed, so a caller cannot tell a conditional `UPDATE` that matched from +one that did not, and optimistic concurrency is built on exactly that. The +driving consumer (SQE, which stores Iceberg catalog pointers in SQLite) is +working around it with SELECT-then-UPDATE in a transaction, sound only while a +single writer is guaranteed. + +Spec 012 defines the surface and makes the counter its Requirement 1. This ADR +records what that closes. + +## Decision + +**A native `Connection`/`Statement`/`Transaction` API in `src/api.rs`** with a +rows-affected count, open-or-create modes, positional `?` binding, explicit +transactions, a stated durability contract, and a retryable busy error distinct +from fatal ones. Named parameters are rejected at prepare time rather than +reaching execution as ADR-0015's always-NULL stub, which a public facade would +otherwise make reachable and unattributable. + +**A `Send + Sync` handle over a connection-owned worker thread.** `Rc` is not +`Send` and ownership does not change that, so making the connection type itself +`Send` requires an `Arc` refactor of `Pager`/`PageSource`. The connection +instead creates its `Rc` graph on a thread it owns and never lets it leave. +`sqlx`'s own SQLite driver does this for a C `sqlite3*` +(`sqlx-sqlite-0.9.0/src/connection/worker.rs`), which is evidence the shape is +the standard answer rather than a concession. + +**The `sqlx` driver lives in a separate crate** (`sqlx-sqlite-rs`) and is a +`SHOULD`, not a `MUST`. This crate's `[dependencies]` stays empty. SQE showed +the driver is not the only path by implementing its catalog trait directly over +the public internals; the driver remains right for consumers who will not write +one. + +## Alternatives rejected + +**A C ABI shim** exporting `sqlite3_open`/`sqlite3_prepare_v2`/`sqlite3_step` +so `libsqlite3-sys` links here. Every existing consumer in every language would +work unchanged. Rejected: it reintroduces the `unsafe` boundary the crate exists +to remove, needs a carve-out larger than `src/sys/`, returns the error surface +to null-pointer semantics, and forces the C threading contract on a design that +gets to choose one. Defensible later, on top of spec 012; a bad substitute for +it. + +**Cloning rusqlite's API.** Rejected: `.openspec/README.md` already commits to +"inspired by rusqlite but not a wrapper", and rusqlite's shape is dictated by C +ownership (`Connection` is `!Sync` because `sqlite3*` is, statements borrow +through a `RefCell` cache, `close` hands the connection back on failure). +Copying it imports constraints this crate does not have. + +**Making `Pager` `Arc`/`Mutex`.** Rejected on the grounds ADR-0013 and ADR-0017 +already established: one `Vm` shares a page source across N cursors via cheap +`Rc` clones, so `Arc` taxes the Tier 0 read path to serve a requirement that +lives at the connection boundary. + +**Putting the `sqlx` driver in this repository**, feature-gated. Rejected: an +empty `[dependencies]` table is one of this crate's two headline properties, a +feature-gated dependency still appears in `Cargo.lock`, the SBOM and +`cargo deny`, and it would couple this crate's cadence to `sqlx`'s. A separate +crate can track `sqlx` 0.9, 0.10 and 1.0 while this API stays still. + +**Async connections.** Rejected for now, not on principle: `sqlx` drivers run +blocking work on their own executor, so async buys the driving consumer nothing, +and an async pager is a storage decision spec 012 must not pre-empt. + +## Consequences + +- Spec 012 needs its own value block. It sits outside the V1--V12 ladder (those + deliver SQL surface, it delivers consumability) and is a prerequisite for V7's + stated demo, so it belongs before V8. One minor per phase (ADR-0006) implies + its own minor. +- `foreign_keys = ON` will be accepted before it is enforced (V8), because + `sqlx` issues it unconditionally. That divergence needs its own ADR under + ADR-0004, not a silent no-op. +- The rows-affected count is the one item a consumer cannot work around + cleanly. Shipping the facade without it leaves every compare-and-swap consumer + reinventing a single-writer workaround, and the ones who get it wrong lose + writes silently. +- Two SQL-semantics prerequisites surface alongside this work and belong to + V3/V7: `CREATE TABLE IF NOT EXISTS` is ignored after parsing, and a composite + `PRIMARY KEY`/`UNIQUE` table constraint is neither enforced nor backed by an + auto-created `sqlite_autoindex_*`. The second also affects this crate's own + byte-compatibility claim, which makes it the highest-value item in the set. +- Consumer statement sets become fixture families under spec 004's harness + (spec 012/Req-6), so "an application can use this" is measured rather than + asserted. diff --git a/.openspec/specs/012-embedding-api/spec.md b/.openspec/specs/012-embedding-api/spec.md new file mode 100644 index 00000000..1c111c8e --- /dev/null +++ b/.openspec/specs/012-embedding-api/spec.md @@ -0,0 +1,347 @@ +--- +domain: embedding-api +version: 0.1.0 +status: draft +date: 2026-08-28 +--- + +# 012 — Embedding API + +The public surface an application links against. Everything below is additive: +a rows-affected count, a connection and statement facade, a `Send + Sync` +handle, a durability contract, and a stability policy. No storage behavior, no +SQL surface, no opcode. + +The engine primitives are already public and nearly sufficient -- +`examples/query.rs` opens a database, compiles once and binds `?1` per +execution; `examples/crud.rs` writes inside a transaction -- so a consumer +willing to write glue can embed this crate today, and one does (SQE, an Iceberg +query engine that stores catalog pointers in SQLite; cited as *SQE* where its +measured need pins a decision). Requirement 1 is the one item on this list a +consumer cannot work around. + +Decisions and rejected alternatives: ADR-0033. + +## Scope and inheritance + +This spec defines only the surface. Every concern below is already specified +elsewhere and is not restated here. + +| Concern | Defined in | +|---------|-----------| +| File locking, WAL reader marks, hot-journal handling, `VfsError::Locked` | spec 007 | +| Bound parameters, the `Variable` opcode, register allocation | spec 009, ADR-0015 | +| Write opcodes and their semantics | spec 010 | +| Value affinity, comparison, storage classes | spec 008 | +| Oracle diff harness, fixture families, corpus layout | spec 004, spec 005 | +| PRAGMA catalogue and priority tiers | plan.md, V7 | +| Recording a deliberate divergence from stock SQLite | ADR-0004 | +| `Rc`/`RefCell` page-source ownership, why `Vm` is not generic | ADR-0013, ADR-0017 | + +Nothing here is in the tier model: the tiers rank SQL capability, this spec +ranks consumability. It sits outside the V1--V12 ladder because every block in +plan.md delivers SQL surface and this one delivers an API, and it is a +prerequisite for V7's stated demo ("point an existing tool ... at sqlite-rs and +have it work"), so it belongs as its own block before V8. + +## The consumer this is drawn from + +SQE uses this crate as a pointer store, not a query engine. Two tables written +only by its Iceberg catalog layer: `iceberg_tables` maps `(catalog_name, +table_namespace, table_name)` to a `metadata_location` string, +`iceberg_namespace_properties` maps a namespace property key to a value. One +row per Iceberg table, written at commit frequency, read by primary key. User +SQL never reaches it. + +Two consequences that shape the requirements. Throughput is irrelevant, so +serialized access is acceptable and Requirement 4 is about reachability rather +than parallelism. And correctness is absolute, because each row points at a +table that may hold terabytes: an unenforced uniqueness constraint, a lost +compare-and-swap or a non-durable commit makes a table unreachable rather than +slow. Requirements 1 and 5, plus the composite-key prerequisite, are the +correctness core; the rest is safety and ergonomics. + +## What is missing today + +Each line is checkable against the tree at 0.18.5. + +1. **A rows-affected count.** Nothing in `src/vdbe/` reports how many rows an + `INSERT`/`UPDATE`/`DELETE` changed. The only capability gap here. +2. **A facade.** No `Connection`, `Statement` or `Transaction`; the caller + assembles pager, header, `Program` and a positional `Vec` by hand. +3. **A `Send + Sync` handle.** `Rc` and `Rc>` + are `!Send`, and ownership does not change that, so an async trait cannot + hold the engine at all. +4. **A creation API.** `DatabaseHeader::new_empty_page1` is public + (`src/header.rs:295`) but no API offers it, so `examples/README.md` records + that the examples copy `fixtures/empty.db` instead. +5. **Named parameters.** `:name`, `@name`, `$name` reach the always-NULL stub + ADR-0015 left in place, which a public facade would make reachable. +6. **A durability contract.** `Pager` syncs (`src/pager.rs:589,597,697,782`) + but nothing states what is guaranteed, and `synchronous` has no handler. + +## Prerequisites owned elsewhere + +Two SQL-semantics gaps block a consumer and belong to V3/V7, not to this spec. +They are listed so nobody plans around the wrong gap. + +- **`CREATE TABLE IF NOT EXISTS` is ignored after parsing.** `if_not_exists` + appears only in `src/parser/grammar.rs` and `src/parser/printer.rs`. +- **A composite `PRIMARY KEY`/`UNIQUE` table constraint is not enforced and no + `sqlite_autoindex_*` is created** (`src/codegen/stmt/insert.rs` documents + this). Two consequences: duplicates are accepted where stock SQLite raises a + constraint error, and the schema diverges from what the oracle writes for the + same DDL, which Requirement 6's acceptance check will surface. + +## Requirements + +### Requirement 1: A Rows-Affected Count [MUST] + +The API MUST report how many rows the last `INSERT`, `UPDATE` or `DELETE` +changed, as `sqlite3_changes()` does, following SQLite's rules: a statement +returning no rows does not reset it, and it counts rows changed rather than +examined. + +`execute_transaction_step` returns rows and the new autocommit flag, so a +caller cannot distinguish an `UPDATE` that matched from one that did not. Every +optimistic-concurrency scheme is built on that distinction. *SQE* swaps a +table's metadata pointer with a conditional `UPDATE` and treats zero rows +affected as a lost race; without the count that becomes SELECT-then-UPDATE in a +transaction, sound only while the consumer guarantees a single writer, and every +consumer reinvents it. + +**Implementation:** `src/api.rs::Connection::changes` (planned) + +**Tests:** `tests/unit/api_changes_test.rs` (planned) + +#### Scenario: A conditional update reports whether it matched + +- GIVEN a row with `metadata_location = 'a'` +- WHEN `UPDATE t SET metadata_location = 'b' WHERE metadata_location = 'a'` + runs, then the identical statement runs again +- THEN the first reports one row changed, the second reports zero, and the + pinned oracle agrees with both + +**Tests:** `tests/unit/api_changes_test.rs::conditional_update_reports_match` (planned) + +#### Scenario: A SELECT does not clobber the count + +- GIVEN a `DELETE` that removed two rows +- WHEN a `SELECT` returning no rows runs next +- THEN the count still reports two + +**Tests:** `tests/unit/api_changes_test.rs::select_does_not_clobber_count` (planned) + +### Requirement 2: Connection, Open or Create [MUST] + +The API MUST open an existing database and, when asked, create a valid empty +one. Modes MUST distinguish read-only, read-write and read-write-create, and +MUST NOT create a file when create was not requested. Locks are spec 007's; the +handle MUST release them on drop, which `Pager` already does. + +*SQE* opens its catalog as `sqlite://?mode=rwc` and expects the file to +appear on first use; a first-run laptop has no `empty.db` to copy. + +**Implementation:** `src/api.rs::Connection::open`, `::open_with` (planned) + +**Tests:** `tests/unit/api_connection_test.rs` (planned) + +#### Scenario: Create produces a database the oracle reads + +- GIVEN a path with no file at it +- WHEN opened `ReadWriteCreate` +- THEN a valid database exists and the pinned oracle reports an empty schema + +**Tests:** `tests/unit/api_connection_test.rs::create_then_oracle_reads_empty_schema` (planned) + +#### Scenario: Without create, nothing is written + +- GIVEN a path with no file at it +- WHEN opened `ReadWrite` +- THEN it fails and no file exists afterwards + +**Tests:** `tests/unit/api_connection_test.rs::readwrite_does_not_create` (planned) + +### Requirement 3: Statement Handle [MUST] + +The API MUST expose a statement that owns its compiled `Program` and its +parameter slots: bind positional `?`/`?NNN` (spec 009's `Variable` opcode), +then read rows as typed values by index and by name over spec 008's storage +classes, without the caller naming `Program`, registers or cursors. Named +parameter forms MUST be rejected at prepare time rather than reaching execution +as ADR-0015's always-NULL stub. + +The value is not speed. *SQE* issues about a dozen statements at commit +frequency, so compiling once saves nothing measurable; a handle owning its slots +is what stops a transposed argument list writing a valid row that points at the +wrong table. + +**Implementation:** `src/api.rs::Statement`, `::Row` (planned) + +**Tests:** `tests/unit/api_statement_test.rs` (planned) + +#### Scenario: One compile, many bindings + +- GIVEN a prepared `SELECT name FROM t WHERE id = ?1` +- WHEN executed with 1, 2 and 3 bound +- THEN each returns that row's name and compilation happened once + +**Tests:** `tests/unit/api_statement_test.rs::compile_once_bind_many` (planned) + +#### Scenario: A named parameter is refused, not silently NULL + +- GIVEN `SELECT * FROM t WHERE id = :id` +- WHEN prepared +- THEN preparation fails naming the unsupported form + +**Tests:** `tests/unit/api_statement_test.rs::named_param_is_refused_at_prepare` (planned) + +### Requirement 4: A `Send + Sync` Handle Over an Owned Worker Thread [MUST] + +The handle MUST be `Send + Sync` so a pool, an async task or a trait demanding +those bounds can hold it, while the engine state stays `Rc`/`RefCell` per +ADR-0017. + +Both cannot hold by making the connection type `Send`: `Rc` is not `Send` and +ownership does not change that, so the compiler rejects the shape. Of the two +achievable designs -- an `Arc`/lock refactor of `Pager` and `PageSource`, which +ADR-0013 and ADR-0017 rejected on read-path cost, or a worker thread owned by +the connection -- this requirement specifies the second. `sqlx`'s own SQLite +driver does the same for a C `sqlite3*` +(`sqlx-sqlite-0.9.0/src/connection/worker.rs`: a spawned thread behind a `flume` +channel, one per connection), so implementing it here gives every consumer once +what each would otherwise write. + +The thread MUST terminate on drop, and a request after it dies MUST error rather +than block. Coordination between connections in one process is spec 007's file +locks, as between processes; no shared cache, no global state. + +**Implementation:** `src/api.rs::Connection` (planned) + +**Tests:** `tests/unit/api_threading_test.rs` (planned) + +#### Scenario: The handle is shared across threads + +- GIVEN a connection opened on thread A +- WHEN its handle is cloned into several threads that each run a query +- THEN every query succeeds, a static assertion proves the handle is + `Send + Sync`, and all engine access happened on the connection's thread + +**Tests:** `tests/unit/api_threading_test.rs::handle_is_send_sync` (planned) + +#### Scenario: The thread is released, and a dead engine errors + +- GIVEN a loop that opens and drops connections +- WHEN it finishes +- THEN the thread count is unchanged, and a request on a dropped connection's + handle errors instead of blocking + +**Tests:** `tests/unit/api_threading_test.rs::worker_thread_joins_on_drop` (planned) + +### Requirement 5: Transactions and a Stated Durability Contract [MUST] + +The API MUST expose `BEGIN`/`COMMIT`/`ROLLBACK` (deferred, immediate, +exclusive), MUST thread the autocommit state `execute_transaction_step` already +returns so a multi-statement transaction is one unit, and MUST roll back a +transaction handle dropped without commit. + +It MUST also state what is durable at commit and honor `PRAGMA synchronous` at +least to distinguish FULL from OFF. Sync points exist +(`src/pager.rs:589,597,697,782,811,845`); what is missing is a documented +guarantee and any way to trade it. A consumer storing pointers to data it cannot +otherwise find needs that in writing: *SQE*'s file holds the metadata pointer +for every table in a warehouse, so a commit returning before it is durable turns +a power failure into tables that exist on object storage and are unreachable. +Where a PRAGMA is accepted without being honored, record it as a divergence +under ADR-0004. + +Retryable errors belong here too: spec 007's `VfsError::Locked` MUST surface as +a distinct, documented busy variant, and a busy timeout MUST be settable per +connection. + +**Implementation:** `src/api.rs::Transaction`, `::Connection::pragma`, `::ApiError` (planned) + +**Tests:** `tests/unit/api_transaction_test.rs`, `tests/unit/api_durability_test.rs` (planned) + +#### Scenario: Dropped transaction rolls back + +- GIVEN an open transaction with one `INSERT` applied +- WHEN the handle drops without `commit()` +- THEN the row is absent and the pinned oracle agrees + +**Tests:** `tests/unit/api_transaction_test.rs::drop_rolls_back` (planned) + +#### Scenario: A committed transaction survives a hard kill + +- GIVEN a transaction committed under `synchronous = FULL` +- WHEN the process is killed without unwinding and the database reopened +- THEN the rows are present and `integrity_check` passes under the oracle + +**Tests:** `tests/unit/api_durability_test.rs::commit_survives_hard_kill` (planned) + +#### Scenario: Busy is retryable and distinguishable + +- GIVEN a second connection holding the WAL write lock +- WHEN a write is attempted +- THEN the error is the busy variant, `is_retryable()` is true, and a retry + after release succeeds + +**Tests:** `tests/unit/api_durability_test.rs::busy_is_retryable` (planned) + +### Requirement 6: Published Surface, Stability Policy, and Acceptance [MUST] + +The crate MUST state which modules are the supported surface and which are +implementation detail, and the facade MUST cover everything those internals +offer a consumer, Requirement 1 included, so nobody is forced back down a layer. +Today `src/lib.rs` exports the engine (`btree`, `codegen`, `dump`, `pager`, +`parser`, `planner`, `vdbe`, `vfs`, ...) while `CHANGELOG.md` says "Pre-1.0: +minor bumps may break the public API", so a consumer wiring `dump::open` to +`execute_transaction_step` builds on items carrying no promise and reasonably +hidden once `src/api.rs` exists. *SQE* pins an exact version and confines every +`sqlite_rs::` reference to one module for this reason; that is a workaround for +a missing policy, not a substitute. + +Acceptance is spec 004's harness, not a new one: a consumer statement set +becomes a fixture family, diffed against pinned `sqlite3` 3.53.4. The first +family is *SQE*'s catalog, and the whole list is `CREATE TABLE IF NOT EXISTS` +with a three-column composite `PRIMARY KEY`, `INSERT` with four bound +parameters, `SELECT ... UNION` over two namespace sources, `LIMIT 1` existence +probes, a conditional `UPDATE`, and `DELETE`. Every statement in it lands in V2 +through V4. The gap was never SQL coverage. + +**Implementation:** `src/lib.rs` module docs, `CHANGELOG.md` policy, +`tests/corpus/fixtures/consumers/sqe/` (planned) + +**Tests:** `tests/unit/api_surface_test.rs`, `tests/corpus/consumer_sqe_test.rs` (planned) + +#### Scenario: The facade needs no escape hatch + +- GIVEN a consumer using only the items this spec defines +- WHEN it creates a database, prepares and binds a statement, reads rows, runs a + transaction and reads the rows-affected count +- THEN it compiles without naming `pager`, `vdbe`, `codegen`, `dump` or `btree` + +**Tests:** `tests/unit/api_surface_test.rs::facade_is_sufficient_alone` (planned) + +#### Scenario: The consumer corpus matches the oracle + +- GIVEN the catalog statement set above, run through this API and through the + oracle +- THEN both produce identical rows in order, identical rows-affected counts, and + identical files modulo documented header fields + +**Tests:** `tests/corpus/consumer_sqe_test.rs::catalog_statements_match_oracle` (planned) + +## Not in this spec + +- **A `sqlx` driver.** Out of tree (`sqlx-sqlite-rs`), so this crate's empty + `[dependencies]` stays empty. Rationale and rejected alternatives: ADR-0033. +- **A C ABI, a rusqlite-shaped API, an `Arc` pager.** ADR-0033. +- **Async connections.** Blocking; `sqlx` drivers run blocking work on their own + executor, and an async pager is a storage decision. +- **The PRAGMA catalogue.** plan.md V7 owns the list and its tiers; Requirement + 5 covers only what a pool sets and what durability requires. +- **Foreign-key enforcement** (V8), **`ATTACH`** (V10), and the two + prerequisites above. +- **Non-POSIX platforms.** `UnixVfs` and the `src/sys/` carve-out are POSIX; a + Windows VFS is spec 003's question. From 16be7ae6f88b199861780dcb2f8eb8e2d7d3afbb Mon Sep 17 00:00:00 2001 From: Jacob Verhoeks Date: Fri, 28 Aug 2026 15:53:25 +0200 Subject: [PATCH 2/5] spec: autoindex write-side rule (010/Req-8) + incremental row access (013/Req-7) 010 gains Requirement 8: a write MUST NOT succeed while leaving an index it could not read unmaintained. Measured at 0.18.5 against stock sqlite3 3.51.0 -- inserting into a stock-created composite-PK table leaves rows out of sqlite_autoindex_*, after which the oracle undercounts and integrity_check reports rows missing, while the write returns rc=0. Names both acceptable fixes (recover the autoindex column list from the declared constraint, or refuse the write) and scopes autoindex *creation* out to V3/V7. The embedding-api spec gains Requirement 7: statements must yield rows incrementally. It also upgrades the composite-PK prerequisite from inferred to measured, flagging it as the highest-priority item in or around that spec -- it is silent corruption of a valid SQLite file, not an ergonomic gap, and its Req 6 byte-identity scenario cannot pass while it stands. That spec is numbered 012 as of this commit and is renumbered to 013 by the next one, after main landed its own spec 012. Refs: 010/Req-8, 013/Req-7 --- .../specs/010-vdbe-write-opcodes/spec.md | 97 +++++++++++++++++++ .openspec/specs/012-embedding-api/spec.md | 77 ++++++++++++++- 2 files changed, 169 insertions(+), 5 deletions(-) diff --git a/.openspec/specs/010-vdbe-write-opcodes/spec.md b/.openspec/specs/010-vdbe-write-opcodes/spec.md index 1f649357..49182de8 100644 --- a/.openspec/specs/010-vdbe-write-opcodes/spec.md +++ b/.openspec/specs/010-vdbe-write-opcodes/spec.md @@ -323,6 +323,103 @@ before the row's own `Insert`, dispatching `ON CONFLICT` **Tests:** `tests/corpus/unique_constraint_test.rs::insert_or_replace_displaces_the_conflicting_row` +### Requirement 8: A Write MUST NOT Ignore an Index It Cannot Read [MUST] + +A write to a table MUST maintain every index present for that table in +`sqlite_master`, or MUST refuse the write. It MUST NOT succeed while leaving an +index on disk unmaintained. + +Requirement 7's `emit_unique_check` iterates `schema.indexes.iter().filter(|i| +i.unique)` and `IdxInsert` (Requirement 5) maintains what codegen emits for, so +both are bounded by what the schema reader returned. `src/schema/ddl_reader.rs` +deliberately returns less than the file contains: its own test, +`auto_index_with_null_sql_is_omitted`, records that an index whose +`sqlite_master.sql` is NULL is "gracefully skipped rather than erroring", +because the naive reader cannot recover a column list from a NULL. Every +`sqlite_autoindex_*` stock SQLite creates for a `PRIMARY KEY` or `UNIQUE` table +constraint has exactly that shape. + +Graceful degradation is correct for a read. For a write it is data loss, and it +is silent: the row lands in the table b-tree, the index keeps its old contents, +and the write returns success. + +Measured 2026-08-28, this crate's CLI at 0.18.5 against stock `sqlite3` 3.51.0: + +```sql +-- stock sqlite3 creates the table; sqlite_autoindex_t_1 exists +CREATE TABLE t (a TEXT NOT NULL, b TEXT NOT NULL, c TEXT NOT NULL, v TEXT, + PRIMARY KEY (a, b, c)); +INSERT INTO t VALUES ('c','ns','t1','v1'); +``` + +```console +$ sqlite-rs exec t.db "INSERT INTO t VALUES ('c','ns','t2','v2')" # rc=0 +$ sqlite-rs exec t.db "INSERT INTO t VALUES ('c','ns','t1','dup')" # rc=0, duplicate accepted +$ sqlite3 t.db "SELECT count(*) FROM t; PRAGMA integrity_check;" +1 +wrong # of entries in index sqlite_autoindex_t_1 +row 2 missing from index sqlite_autoindex_t_1 +row 3 missing from index sqlite_autoindex_t_1 +``` + +`count(*)` answers from the stale index and undercounts. A control table with no +primary key writes and verifies clean, which places the fault at index +maintenance rather than the file format, page 1 or the schema table. + +The precedent for the shape of the fix is spec 007 Requirement 1: a hot journal +is detected and refused rather than silently ignored, because serving +pre-rollback pages is worse than failing to open. Same argument, write side. + +Two ways to satisfy this, and either is acceptable: + +- **Read the autoindex.** Its column list is recoverable without SQL text, from + the table's declared constraint in its own `CREATE TABLE`. Then the existing + `NoConflict` and `IdxInsert` paths cover it with no new opcode. +- **Refuse the write.** A table carrying an index the reader could not parse is + read-only until it can. Narrower, and it converts silent corruption into an + error a caller can act on. + +Creating `sqlite_autoindex_*` for a declared constraint is a separate `CREATE +TABLE`-side gap (`src/codegen/stmt/insert.rs` records it, V3/V7 owns it), and it +has its own file-level consequence: a table this crate creates with a declared +composite `PRIMARY KEY` has no autoindex, and stock `sqlite3` then answers any +write or `integrity_check` on it with "database disk image is malformed (11)". +Closing this requirement without closing that one leaves creation broken; closing +that one without this leaves adoption of a foreign file broken. + +**Implementation:** `src/schema/ddl_reader.rs::index_schema` (planned), consumed +by `src/codegen/stmt/insert.rs::emit_unique_check` + +**Tests:** `tests/corpus/autoindex_maintenance_test.rs` (planned) + +#### Scenario: A write to a stock-created composite-PK table keeps the index consistent + +- GIVEN a table created by the pinned oracle with a composite `PRIMARY KEY`, so + `sqlite_autoindex_*` exists, holding one row +- WHEN this crate inserts a second row +- THEN the oracle's `PRAGMA integrity_check` reports ok and `count(*)` returns 2, + or the insert was refused and the file is byte-identical to before + +**Tests:** `tests/corpus/autoindex_maintenance_test.rs::stock_composite_pk_stays_consistent` (planned) + +#### Scenario: A duplicate against an autoindex-backed constraint is refused + +- GIVEN the same table holding `('c','ns','t1')` +- WHEN this crate inserts `('c','ns','t1')` again +- THEN it fails with a uniqueness error, or the write is refused for the reason + above, and in neither case is a duplicate row persisted + +**Tests:** `tests/corpus/autoindex_maintenance_test.rs::autoindex_duplicate_is_refused` (planned) + +#### Scenario: A named index is unaffected + +- GIVEN a table with no declared primary key and a named `CREATE UNIQUE INDEX` +- WHEN this crate inserts a new key and then a duplicate +- THEN the new key is written, the duplicate is refused, and the oracle's + `integrity_check` reports ok + +**Tests:** `tests/corpus/autoindex_maintenance_test.rs::named_index_round_trips` (planned) + ## Related regimes - Tier suite: `tests/tiers/tier2.rs::t2_crud_round_trips_on_rowid_tables` diff --git a/.openspec/specs/012-embedding-api/spec.md b/.openspec/specs/012-embedding-api/spec.md index 1c111c8e..cc892814 100644 --- a/.openspec/specs/012-embedding-api/spec.md +++ b/.openspec/specs/012-embedding-api/spec.md @@ -61,6 +61,12 @@ compare-and-swap or a non-durable commit makes a table unreachable rather than slow. Requirements 1 and 5, plus the composite-key prerequisite, are the correctness core; the rest is safety and ergonomics. +That consumer is now adding a second, different use of the same crate: attaching +an arbitrary SQLite database and exposing its tables as queryable relations, so +a user can join one against an Iceberg table. Arbitrary schemas, arbitrary +affinities, arbitrary row counts. Requirement 7 exists because of it, and it is +the only requirement here driven by a read path rather than a pointer store. + ## What is missing today Each line is checkable against the tree at 0.18.5. @@ -79,6 +85,10 @@ Each line is checkable against the tree at 0.18.5. ADR-0015 left in place, which a public facade would make reachable. 6. **A durability contract.** `Pager` syncs (`src/pager.rs:589,597,697,782`) but nothing states what is guaranteed, and `synchronous` has no handler. +7. **Incremental row access.** `execute_with_db` and + `execute_with_db_and_params` return `Vec>` (`src/vdbe/exec.rs:1073, + 1093`), so a result set is fully materialized before the caller sees a row. + Nothing in `src/vdbe/` offers a step or iterator API. ## Prerequisites owned elsewhere @@ -87,11 +97,27 @@ They are listed so nobody plans around the wrong gap. - **`CREATE TABLE IF NOT EXISTS` is ignored after parsing.** `if_not_exists` appears only in `src/parser/grammar.rs` and `src/parser/printer.rs`. -- **A composite `PRIMARY KEY`/`UNIQUE` table constraint is not enforced and no - `sqlite_autoindex_*` is created** (`src/codegen/stmt/insert.rs` documents - this). Two consequences: duplicates are accepted where stock SQLite raises a - constraint error, and the schema diverges from what the oracle writes for the - same DDL, which Requirement 6's acceptance check will surface. +- **A composite `PRIMARY KEY`/`UNIQUE` table constraint is not enforced, no + `sqlite_autoindex_*` is created, and an existing one is not maintained.** Now + measured rather than inferred (0.18.5 against stock `sqlite3` 3.51.0), and it + is worse than a missing feature: writing into a stock-created table with a + declared composite PK leaves rows out of the autoindex, after which the oracle + undercounts and `integrity_check` reports rows missing, while the write + returns success. Creating the same DDL here yields a file the oracle calls + "malformed (11)" on any write. A table with no declared PK and a named + `CREATE UNIQUE INDEX` round-trips cleanly in both directions with uniqueness + enforced by both. The mechanism is `src/schema/ddl_reader.rs`'s deliberate + skip of an index whose `sqlite_master.sql` is NULL, which is right for a read + and data loss for a write. Spec 010 Requirement 8 states the write-side rule; + creating the autoindex stays V3/V7's. **This is the highest-priority item in + or around this spec**, because it is a silent-corruption bug against a valid + SQLite file rather than an ergonomic gap, and because Requirement 6's + byte-identity scenario cannot pass while it stands. + + *SQE*'s response, for reference: its catalog schema drops the declared + composite primary key in favour of a named unique index, and its adapter + refuses writes to any catalog carrying an `sqlite_autoindex_*`. Both are + workarounds for this item and come out when it closes. ## Requirements @@ -332,6 +358,47 @@ through V4. The gap was never SQL coverage. **Tests:** `tests/corpus/consumer_sqe_test.rs::catalog_statements_match_oracle` (planned) +### Requirement 7: Incremental Row Access [MUST] + +A statement MUST yield rows incrementally, as `sqlite3_step()` does. Today +`execute_with_db` and `execute_with_db_and_params` return `Vec>`, so +the engine allocates an entire result set before the caller sees the first row, +and there is no step or iterator API to fall back to. + +For a pointer store that is invisible: a dozen rows, once per commit. For any +consumer reading a database as a data source it is the difference between a +usable API and an unusable one. *SQE* is adding exactly that use, attaching +arbitrary SQLite files so a user can query and join their tables; against a +million-row table the current shape materializes the whole table before the +first batch exists. Its interim answer is a configurable row ceiling with an +error beyond it, which is honest and narrow, and it comes off when this lands. + +Memory MUST be bounded by the rows the caller has actually pulled, not by the +result set, and abandoning a partially-read statement MUST release its resources +and its cursors without waiting for the rest. + +**Implementation:** `src/api.rs::Statement::next_row` (planned), or an `Iterator` impl + +**Tests:** `tests/unit/api_streaming_test.rs` (planned) + +#### Scenario: A large result is read without materializing it + +- GIVEN a table with a row count well above any sensible buffer +- WHEN the first ten rows are read and the statement is dropped +- THEN peak allocation is proportional to the ten rows rather than the table, + and the ten values match the pinned oracle's first ten + +**Tests:** `tests/unit/api_streaming_test.rs::partial_read_is_bounded` (planned) + +#### Scenario: Abandoning a statement releases it + +- GIVEN a statement read halfway +- WHEN it is dropped +- THEN its cursors are released and a subsequent write on the same connection + proceeds + +**Tests:** `tests/unit/api_streaming_test.rs::abandoned_statement_releases_cursors` (planned) + ## Not in this spec - **A `sqlx` driver.** Out of tree (`sqlx-sqlite-rs`), so this crate's empty From 877d6d813e6f1bf233b416e7e67e0cf464d951c0 Mon Sep 17 00:00:00 2001 From: Jacob Verhoeks Date: Fri, 28 Aug 2026 15:54:01 +0200 Subject: [PATCH 3/5] docs: renumber embedding-api spec to 013 and its ADR to 0034 Main landed its own spec 012 (`012-query-constraints`) and its own ADR-0033 (constant propagation / OR-to-IN) while this branch was open, so both numbers collided on rebase. The newcomer moves: - `.openspec/specs/012-embedding-api/` -> `013-embedding-api/`, heading updated - `adr/0033-embedding-api-owns-the-connection-driver-out-of-tree.md` -> `0034-`, heading updated, and its five `spec 012` prose references retargeted to 013 - `adr/index.md` gains the 0034 row, which 0ee936b omitted entirely Renumbering ADR-0034 does not violate the immutability convention: it has never been on main and is still `Status: Proposed`. Also fixes four `**Implementation:**` lines in spec 013 that were already marked `(planned)` but written in a form `tools/assurance.py:474` does not match -- its regex only accepts `(planned)` immediately after a *single* backticked path, so `` `a`, `b` (planned) `` scored as active and its not-yet-written test links counted as dead. With that fixed, the dashboard is byte-identical to main: 86 active requirements, Completeness 85/86 (99%), Coverage 270/270 (99%), zero dead links, planned 2 -> 10. Refs: 013/Req-1, 013/Req-7 --- ...owns-the-connection-driver-out-of-tree.md} | 12 ++++++------ .openspec/adr/index.md | 1 + .../spec.md | 19 ++++++++++--------- 3 files changed, 17 insertions(+), 15 deletions(-) rename .openspec/adr/{0033-embedding-api-owns-the-connection-driver-out-of-tree.md => 0034-embedding-api-owns-the-connection-driver-out-of-tree.md} (93%) rename .openspec/specs/{012-embedding-api => 013-embedding-api}/spec.md (97%) diff --git a/.openspec/adr/0033-embedding-api-owns-the-connection-driver-out-of-tree.md b/.openspec/adr/0034-embedding-api-owns-the-connection-driver-out-of-tree.md similarity index 93% rename from .openspec/adr/0033-embedding-api-owns-the-connection-driver-out-of-tree.md rename to .openspec/adr/0034-embedding-api-owns-the-connection-driver-out-of-tree.md index 258aeef2..f42e1aaa 100644 --- a/.openspec/adr/0033-embedding-api-owns-the-connection-driver-out-of-tree.md +++ b/.openspec/adr/0034-embedding-api-owns-the-connection-driver-out-of-tree.md @@ -1,4 +1,4 @@ -# 0033 — The embedding API owns the connection; the `sqlx` driver stays out of tree +# 0034 — The embedding API owns the connection; the `sqlx` driver stays out of tree **Status:** Proposed · **Date:** 2026-08-28 @@ -23,7 +23,7 @@ driving consumer (SQE, which stores Iceberg catalog pointers in SQLite) is working around it with SELECT-then-UPDATE in a transaction, sound only while a single writer is guaranteed. -Spec 012 defines the surface and makes the counter its Requirement 1. This ADR +Spec 013 defines the surface and makes the counter its Requirement 1. This ADR records what that closes. ## Decision @@ -56,7 +56,7 @@ so `libsqlite3-sys` links here. Every existing consumer in every language would work unchanged. Rejected: it reintroduces the `unsafe` boundary the crate exists to remove, needs a carve-out larger than `src/sys/`, returns the error surface to null-pointer semantics, and forces the C threading contract on a design that -gets to choose one. Defensible later, on top of spec 012; a bad substitute for +gets to choose one. Defensible later, on top of spec 013; a bad substitute for it. **Cloning rusqlite's API.** Rejected: `.openspec/README.md` already commits to @@ -78,11 +78,11 @@ crate can track `sqlx` 0.9, 0.10 and 1.0 while this API stays still. **Async connections.** Rejected for now, not on principle: `sqlx` drivers run blocking work on their own executor, so async buys the driving consumer nothing, -and an async pager is a storage decision spec 012 must not pre-empt. +and an async pager is a storage decision spec 013 must not pre-empt. ## Consequences -- Spec 012 needs its own value block. It sits outside the V1--V12 ladder (those +- Spec 013 needs its own value block. It sits outside the V1--V12 ladder (those deliver SQL surface, it delivers consumability) and is a prerequisite for V7's stated demo, so it belongs before V8. One minor per phase (ADR-0006) implies its own minor. @@ -99,5 +99,5 @@ and an async pager is a storage decision spec 012 must not pre-empt. auto-created `sqlite_autoindex_*`. The second also affects this crate's own byte-compatibility claim, which makes it the highest-value item in the set. - Consumer statement sets become fixture families under spec 004's harness - (spec 012/Req-6), so "an application can use this" is measured rather than + (spec 013/Req-6), so "an application can use this" is measured rather than asserted. diff --git a/.openspec/adr/index.md b/.openspec/adr/index.md index 2b4d7e6c..c899d0a8 100644 --- a/.openspec/adr/index.md +++ b/.openspec/adr/index.md @@ -42,3 +42,4 @@ Specs record what the system must do; ADRs record **why it is shaped this way** | [0036](0036-pragma-synchronous-fsync-policy.md) | `PRAGMA synchronous` fsync-skip policy, and why `SynchronousMode` lives in `header.rs` | 2026-08-29 | | [0037](0037-macos-plain-fsync-not-fullfsync.md) | On macOS, `Vfs::sync` calls plain `fsync(2)`, not `std`'s `F_FULLFSYNC` | 2026-08-30 | | [0038](0038-cargo-registry-opt-in-not-committed.md) | Artifactory Cargo access is opt-in local config, never a committed source replacement | 2026-09-03 | +| [0034](0034-embedding-api-owns-the-connection-driver-out-of-tree.md) | The embedding API owns the connection; the `sqlx` driver stays out of tree | 2026-08-28 | diff --git a/.openspec/specs/012-embedding-api/spec.md b/.openspec/specs/013-embedding-api/spec.md similarity index 97% rename from .openspec/specs/012-embedding-api/spec.md rename to .openspec/specs/013-embedding-api/spec.md index cc892814..dd4ba3b0 100644 --- a/.openspec/specs/012-embedding-api/spec.md +++ b/.openspec/specs/013-embedding-api/spec.md @@ -5,7 +5,7 @@ status: draft date: 2026-08-28 --- -# 012 — Embedding API +# 013 — Embedding API The public surface an application links against. Everything below is additive: a rows-affected count, a connection and statement facade, a `Send + Sync` @@ -20,7 +20,7 @@ query engine that stores catalog pointers in SQLite; cited as *SQE* where its measured need pins a decision). Requirement 1 is the one item on this list a consumer cannot work around. -Decisions and rejected alternatives: ADR-0033. +Decisions and rejected alternatives: ADR-0034. ## Scope and inheritance @@ -168,7 +168,7 @@ handle MUST release them on drop, which `Pager` already does. *SQE* opens its catalog as `sqlite://?mode=rwc` and expects the file to appear on first use; a first-run laptop has no `empty.db` to copy. -**Implementation:** `src/api.rs::Connection::open`, `::open_with` (planned) +**Implementation:** `src/api.rs::Connection::open` (planned), plus `::open_with` **Tests:** `tests/unit/api_connection_test.rs` (planned) @@ -202,7 +202,7 @@ frequency, so compiling once saves nothing measurable; a handle owning its slots is what stops a transposed argument list writing a valid row that points at the wrong table. -**Implementation:** `src/api.rs::Statement`, `::Row` (planned) +**Implementation:** `src/api.rs::Statement` (planned), plus `::Row` **Tests:** `tests/unit/api_statement_test.rs` (planned) @@ -285,7 +285,8 @@ Retryable errors belong here too: spec 007's `VfsError::Locked` MUST surface as a distinct, documented busy variant, and a busy timeout MUST be settable per connection. -**Implementation:** `src/api.rs::Transaction`, `::Connection::pragma`, `::ApiError` (planned) +**Implementation:** `src/api.rs::Transaction` (planned), plus `::Connection::pragma` +and `::ApiError` **Tests:** `tests/unit/api_transaction_test.rs`, `tests/unit/api_durability_test.rs` (planned) @@ -335,8 +336,8 @@ parameters, `SELECT ... UNION` over two namespace sources, `LIMIT 1` existence probes, a conditional `UPDATE`, and `DELETE`. Every statement in it lands in V2 through V4. The gap was never SQL coverage. -**Implementation:** `src/lib.rs` module docs, `CHANGELOG.md` policy, -`tests/corpus/fixtures/consumers/sqe/` (planned) +**Implementation:** `src/lib.rs` (planned) — module docs, plus `CHANGELOG.md` +policy and `tests/corpus/fixtures/consumers/sqe/` **Tests:** `tests/unit/api_surface_test.rs`, `tests/corpus/consumer_sqe_test.rs` (planned) @@ -402,8 +403,8 @@ and its cursors without waiting for the rest. ## Not in this spec - **A `sqlx` driver.** Out of tree (`sqlx-sqlite-rs`), so this crate's empty - `[dependencies]` stays empty. Rationale and rejected alternatives: ADR-0033. -- **A C ABI, a rusqlite-shaped API, an `Arc` pager.** ADR-0033. + `[dependencies]` stays empty. Rationale and rejected alternatives: ADR-0034. +- **A C ABI, a rusqlite-shaped API, an `Arc` pager.** ADR-0034. - **Async connections.** Blocking; `sqlx` drivers run blocking work on their own executor, and an async pager is a storage decision. - **The PRAGMA catalogue.** plan.md V7 owns the list and its tiers; Requirement From 67adedb951bef763cfd0b938cfd367ec97b5fb24 Mon Sep 17 00:00:00 2001 From: Diederik Siderius Date: Fri, 4 Sep 2026 11:22:26 +0200 Subject: [PATCH 4/5] docs: renumber the embedding-API ADR to 0041 and reconcile spec 013 with the tree (#678) Takes over #678's spec so it can land off current `main`. Jacob's three commits are cherry-picked verbatim above this one and keep their authorship; everything here is the reconciliation they could not carry, because the tree moved after they were written. ADR renumber, 0034 -> 0041. 0034 was already taken on `main` (`0034-index-range-seeks.md`) when #678 was opened, and 0038/0039/0040 have since gone to the Cargo registry decision (#684), `Value`'s `Arc` payloads (#688) and the streaming primitive (#683). Three citations in spec 013 and the `index.md` row move with it. The index gains a gap at 0039/0040 until those two branches land, which is the correct state for this branch rather than a placeholder. Four claims in spec 013 were true when written and are not now: - **Item 7 said "nothing in `src/vdbe/` offers a step or iterator API".** `vdbe::Execution` is that API as of #683, with `run()` reimplemented as a wrapper over it. Requirement 7 is restated as a facade gap and its `Implementation:` line now points at `Execution::next_row` as the thing to build on -- #682 found the ordering matters, because a facade retrofitted onto `execute_with_db` cannot be made incremental afterwards. - **Item 3 attributed the `!Send` problem to the pager alone.** So did ADR-0041. `Value::Text`/`Blob` held `Rc` payloads, which made a result row -- precisely the thing that has to leave a worker thread -- unsendable too. Closed by #688/ADR-0039, which also narrows Requirement 4: the worker thread is still required for the pager, but it now hands rows across instead of copying them. - **The composite-PK prerequisite was one item; it is two.** #685 fixed maintenance of an existing `sqlite_autoindex_*` and the read-only safety valve (spec 010/Req 8). Creating one on `CREATE TABLE` is still open as #687. SQE's two workarounds split the same way: the write-refusal one comes out now, the dropped-composite-key one waits on #687. - **Requirement 7's acceptance scenario could not pass.** It asked that "peak allocation is proportional to the ten rows". Peak heap for a streaming read is a floor set by the page cache, not a slope in rows pulled, so no correct implementation satisfies that wording. Restated as independence from result size -- 8.68 MB flat against 137.7 MB materialized on 1,000,000 rows (#682) -- with `DEFAULT_PAGE_CACHE_CAPACITY` named as the knob that moves the floor (2000 pages -> 8.68 MB, 64 -> 291 KB, ~4.5% streaming cost). That is both the property a consumer needs and, unlike proportionality, testable. Also corrected in both specs: measurements were cited against "stock `sqlite3` 3.51.0", but the pinned oracle is 3.53.4 (`tests/corpus/oracle.rs:22`, `Cargo.toml [package.metadata.oracle]`). On this machine a bare `sqlite3` is Apple's 3.51.0 codec build, which `tools/gen_fixtures.sh` refuses by design. I re-derived the autoindex rule against the pinned 3.53.4 across eleven DDL shapes, comparing `pragma_index_info` key lists rather than index counts: every case agrees, including the counter-intuitive ones (declaration order beats primary-key-first; a rowid alias consumes no number). The citation was wrong, not the rule -- but "measured against the pinned oracle" is this repo's whole assurance basis, so it has to be accurate. Spec 010/Req 8's three scenario `Tests:` links already name the exact test functions #685 created. They stay `(planned)` here: the flip belongs to the PR that discharges the requirement, and it cannot happen on this branch because the tests do not exist on it. `make check-assurance` passes at 86/86 and 276/276, unchanged -- every requirement added here is `(planned)`, so it is excluded from scoring by design and the dashboard cannot move until the implementing tickets land. No dead links. Refs: 010/Req-8, 013/Req-1, 013/Req-4, 013/Req-7, #678, #682, #683, #685, #687, #688 Co-Authored-By: Claude Opus 5 (1M context) --- ...owns-the-connection-driver-out-of-tree.md} | 2 +- .openspec/adr/index.md | 2 +- .../specs/010-vdbe-write-opcodes/spec.md | 4 +- .openspec/specs/013-embedding-api/spec.md | 82 +++++++++++++++---- 4 files changed, 69 insertions(+), 21 deletions(-) rename .openspec/adr/{0034-embedding-api-owns-the-connection-driver-out-of-tree.md => 0041-embedding-api-owns-the-connection-driver-out-of-tree.md} (98%) diff --git a/.openspec/adr/0034-embedding-api-owns-the-connection-driver-out-of-tree.md b/.openspec/adr/0041-embedding-api-owns-the-connection-driver-out-of-tree.md similarity index 98% rename from .openspec/adr/0034-embedding-api-owns-the-connection-driver-out-of-tree.md rename to .openspec/adr/0041-embedding-api-owns-the-connection-driver-out-of-tree.md index f42e1aaa..f2cded82 100644 --- a/.openspec/adr/0034-embedding-api-owns-the-connection-driver-out-of-tree.md +++ b/.openspec/adr/0041-embedding-api-owns-the-connection-driver-out-of-tree.md @@ -1,4 +1,4 @@ -# 0034 — The embedding API owns the connection; the `sqlx` driver stays out of tree +# 0041 — The embedding API owns the connection; the `sqlx` driver stays out of tree **Status:** Proposed · **Date:** 2026-08-28 diff --git a/.openspec/adr/index.md b/.openspec/adr/index.md index c899d0a8..0ed04f67 100644 --- a/.openspec/adr/index.md +++ b/.openspec/adr/index.md @@ -42,4 +42,4 @@ Specs record what the system must do; ADRs record **why it is shaped this way** | [0036](0036-pragma-synchronous-fsync-policy.md) | `PRAGMA synchronous` fsync-skip policy, and why `SynchronousMode` lives in `header.rs` | 2026-08-29 | | [0037](0037-macos-plain-fsync-not-fullfsync.md) | On macOS, `Vfs::sync` calls plain `fsync(2)`, not `std`'s `F_FULLFSYNC` | 2026-08-30 | | [0038](0038-cargo-registry-opt-in-not-committed.md) | Artifactory Cargo access is opt-in local config, never a committed source replacement | 2026-09-03 | -| [0034](0034-embedding-api-owns-the-connection-driver-out-of-tree.md) | The embedding API owns the connection; the `sqlx` driver stays out of tree | 2026-08-28 | +| [0041](0041-embedding-api-owns-the-connection-driver-out-of-tree.md) | The embedding API owns the connection; the `sqlx` driver stays out of tree | 2026-08-28 | diff --git a/.openspec/specs/010-vdbe-write-opcodes/spec.md b/.openspec/specs/010-vdbe-write-opcodes/spec.md index 49182de8..866e229a 100644 --- a/.openspec/specs/010-vdbe-write-opcodes/spec.md +++ b/.openspec/specs/010-vdbe-write-opcodes/spec.md @@ -343,7 +343,9 @@ Graceful degradation is correct for a read. For a write it is data loss, and it is silent: the row lands in the table b-tree, the index keeps its old contents, and the write returns success. -Measured 2026-08-28, this crate's CLI at 0.18.5 against stock `sqlite3` 3.51.0: +Measured 2026-08-28, this crate's CLI at 0.18.5 against the pinned +`sqlite3` 3.53.4 oracle (`tests/corpus/oracle.rs`); re-checked on +2026-09-04 and unchanged: ```sql -- stock sqlite3 creates the table; sqlite_autoindex_t_1 exists diff --git a/.openspec/specs/013-embedding-api/spec.md b/.openspec/specs/013-embedding-api/spec.md index dd4ba3b0..bcacfd6c 100644 --- a/.openspec/specs/013-embedding-api/spec.md +++ b/.openspec/specs/013-embedding-api/spec.md @@ -20,7 +20,7 @@ query engine that stores catalog pointers in SQLite; cited as *SQE* where its measured need pins a decision). Requirement 1 is the one item on this list a consumer cannot work around. -Decisions and rejected alternatives: ADR-0034. +Decisions and rejected alternatives: ADR-0041. ## Scope and inheritance @@ -69,7 +69,7 @@ the only requirement here driven by a read path rather than a pointer store. ## What is missing today -Each line is checkable against the tree at 0.18.5. +Each line is checkable against the tree at 0.18.10. 1. **A rows-affected count.** Nothing in `src/vdbe/` reports how many rows an `INSERT`/`UPDATE`/`DELETE` changed. The only capability gap here. @@ -77,7 +77,13 @@ Each line is checkable against the tree at 0.18.5. assembles pager, header, `Program` and a positional `Vec` by hand. 3. **A `Send + Sync` handle.** `Rc` and `Rc>` are `!Send`, and ownership does not change that, so an async trait cannot - hold the engine at all. + hold the engine at all. A second `!Send` was missed on first writing and is + now closed: `Value::Text`/`Blob` held `Rc` payloads, so a result row -- + the one thing that has to *leave* a worker thread -- could not cross a + thread boundary either. Both this spec and ADR-0041 originally attributed + the problem to the pager alone. `Value` is `Arc`-backed and `Send + Sync` + as of ADR-0039, which leaves only the pager half, and the pager half is + what Requirement 4's worker thread is for. 4. **A creation API.** `DatabaseHeader::new_empty_page1` is public (`src/header.rs:295`) but no API offers it, so `examples/README.md` records that the examples copy `fixtures/empty.db` instead. @@ -85,10 +91,15 @@ Each line is checkable against the tree at 0.18.5. ADR-0015 left in place, which a public facade would make reachable. 6. **A durability contract.** `Pager` syncs (`src/pager.rs:589,597,697,782`) but nothing states what is guaranteed, and `synchronous` has no handler. -7. **Incremental row access.** `execute_with_db` and - `execute_with_db_and_params` return `Vec>` (`src/vdbe/exec.rs:1073, - 1093`), so a result set is fully materialized before the caller sees a row. - Nothing in `src/vdbe/` offers a step or iterator API. +7. **Incremental row access, at the facade.** `execute_with_db` and + `execute_with_db_and_params` return `Vec>` + (`src/vdbe/exec.rs:1073, 1093`), so those entry points still materialize a + result set before the caller sees a row. The engine half is no longer + missing: `vdbe::Execution` (ADR-0040) is a public streaming primitive, and + `run()` is now a wrapper that collects it, so batch and streaming are the + same loop. What is still absent is a consumer-facing step API -- which is + Requirement 7, restated below against that primitive rather than against + `execute_with_db`. ## Prerequisites owned elsewhere @@ -97,9 +108,18 @@ They are listed so nobody plans around the wrong gap. - **`CREATE TABLE IF NOT EXISTS` is ignored after parsing.** `if_not_exists` appears only in `src/parser/grammar.rs` and `src/parser/printer.rs`. -- **A composite `PRIMARY KEY`/`UNIQUE` table constraint is not enforced, no - `sqlite_autoindex_*` is created, and an existing one is not maintained.** Now - measured rather than inferred (0.18.5 against stock `sqlite3` 3.51.0), and it +- **A composite `PRIMARY KEY`/`UNIQUE` table constraint: the maintenance half + is fixed, the creation half is not.** As of #685 an existing + `sqlite_autoindex_*` is recovered from the owning table's DDL and maintained, + uniqueness is enforced against it, and an autoindex this reader cannot + interpret makes the table read-only rather than writable-and-corrupting + (spec 010 Requirement 8). What remains is emitting one on `CREATE TABLE`, + tracked as #687, so a table created *here* with a declared composite key + still lacks its index and the oracle still calls the file "malformed (11)" on + any write to it. The description below is the original finding, kept because + it is what the requirement was written against. Now + measured rather than inferred (0.18.5 against the pinned `sqlite3` 3.53.4 + oracle), and it is worse than a missing feature: writing into a stock-created table with a declared composite PK leaves rows out of the autoindex, after which the oracle undercounts and `integrity_check` reports rows missing, while the write @@ -116,8 +136,10 @@ They are listed so nobody plans around the wrong gap. *SQE*'s response, for reference: its catalog schema drops the declared composite primary key in favour of a named unique index, and its adapter - refuses writes to any catalog carrying an `sqlite_autoindex_*`. Both are - workarounds for this item and come out when it closes. + refuses writes to any catalog carrying an `sqlite_autoindex_*`. The second + workaround comes out now -- #685 makes writing to such a catalog safe, which + is the adoption direction SQE actually needs. The first waits on #687, + because a catalog this crate creates still needs the named index. ## Requirements @@ -232,7 +254,14 @@ Both cannot hold by making the connection type `Send`: `Rc` is not `Send` and ownership does not change that, so the compiler rejects the shape. Of the two achievable designs -- an `Arc`/lock refactor of `Pager` and `PageSource`, which ADR-0013 and ADR-0017 rejected on read-path cost, or a worker thread owned by -the connection -- this requirement specifies the second. `sqlx`'s own SQLite +the connection -- this requirement specifies the second. + +ADR-0039 narrows the scope of that choice without reopening it. `Value`'s +payloads are now `Arc`, measured at no read-path cost, so rows themselves are +`Send` and need no copy at the boundary; ADR-0013 and ADR-0017 were only ever +about the pager, and their subject matter is untouched. The worker thread is +still required, and still for exactly the reason above -- but it now hands +rows across rather than serializing them. `sqlx`'s own SQLite driver does the same for a C `sqlite3*` (`sqlx-sqlite-0.9.0/src/connection/worker.rs`: a spawned thread behind a `flume` channel, one per connection), so implementing it here gives every consumer once @@ -378,7 +407,11 @@ Memory MUST be bounded by the rows the caller has actually pulled, not by the result set, and abandoning a partially-read statement MUST release its resources and its cursors without waiting for the rest. -**Implementation:** `src/api.rs::Statement::next_row` (planned), or an `Iterator` impl +**Implementation:** `src/api.rs::Statement::next_row` (planned), or an `Iterator` +impl, built on `src/vdbe/exec.rs::Execution::next_row` rather than on +`execute_with_db` -- #682 found the ordering matters, because a facade +retrofitted onto the materializing entry point cannot be made incremental +afterwards **Tests:** `tests/unit/api_streaming_test.rs` (planned) @@ -386,8 +419,21 @@ and its cursors without waiting for the rest. - GIVEN a table with a row count well above any sensible buffer - WHEN the first ten rows are read and the statement is dropped -- THEN peak allocation is proportional to the ten rows rather than the table, - and the ten values match the pinned oracle's first ten +- THEN peak allocation is **independent of the table's row count** -- flat as + the table grows, rather than proportional to it -- and the ten values match + the pinned oracle's first ten + + Stated that way deliberately. "Proportional to the ten rows" is not + satisfiable by a correct implementation and was the original wording: peak + heap for a streaming read is dominated by the page cache, not the rows + pulled, so it is a floor rather than a slope. Measured on 1,000,000 rows + (spike 014, #682): 137.7 MB materialized against 8.68 MB streamed, and the + 8.68 MB does not move with the result size. The whole floor is one constant, + `DEFAULT_PAGE_CACHE_CAPACITY` (`src/pager.rs:63`) -- 2000 pages gives + 8.68 MB, 256 gives 1.10 MB, 64 gives 291 KB -- so a caller who needs the + floor lower has a knob, at ~4.5% streaming throughput for the smallest. + Independence from result size is the property a consumer actually needs, and + unlike proportionality it is testable. **Tests:** `tests/unit/api_streaming_test.rs::partial_read_is_bounded` (planned) @@ -403,8 +449,8 @@ and its cursors without waiting for the rest. ## Not in this spec - **A `sqlx` driver.** Out of tree (`sqlx-sqlite-rs`), so this crate's empty - `[dependencies]` stays empty. Rationale and rejected alternatives: ADR-0034. -- **A C ABI, a rusqlite-shaped API, an `Arc` pager.** ADR-0034. + `[dependencies]` stays empty. Rationale and rejected alternatives: ADR-0041. +- **A C ABI, a rusqlite-shaped API, an `Arc` pager.** ADR-0041. - **Async connections.** Blocking; `sqlx` drivers run blocking work on their own executor, and an async pager is a storage decision. - **The PRAGMA catalogue.** plan.md V7 owns the list and its tiers; Requirement From a13689971b7055b3ea2cd8ba1e79f0770d16b8c9 Mon Sep 17 00:00:00 2001 From: Diederik Siderius Date: Fri, 4 Sep 2026 13:11:24 +0200 Subject: [PATCH 5/5] =?UTF-8?q?docs:=20PRAGMA=20synchronous=20already=20ha?= =?UTF-8?q?s=20a=20handler=20=E2=80=94=20a=20fifth=20stale=20claim=20in=20?= =?UTF-8?q?spec=20013=20(#678)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit My reconciliation commit said four of spec 013's claims had stopped being true. There were five. Item 6 read "`Pager` syncs but nothing states what is guaranteed, and `synchronous` has no handler". The second clause was already false when #678 was written: #645 implemented `PRAGMA synchronous` in full — the bare query form reporting `0`/`1`/`2` like stock SQLite, all three levels, and a decided per-level fsync-skip policy recorded in ADR-0036 (`src/vdbe/pragma.rs:79`, dispatched at `src/vdbe/exec.rs:760`, with unit tests). Nothing about it is a stub. That also means Requirement 5 is further along than it claims. It asks the API to "honor `PRAGMA synchronous` at least to distinguish FULL from OFF", which is a weaker ask than what already exists, and says "what is missing is a documented guarantee and any way to trade it" — the trade mechanism is exactly what #645 added. Requirement 5's remaining work is the written guarantee, the transaction surface, and the busy/retryable error handling, not the PRAGMA. I found this while answering "will this work with SQE yet", by checking each of the spec's seven gaps against the tree instead of trusting the list. Worth noting for anyone reviewing #693: the list was written against 0.18.5 and the tree is 0.18.10, so treat every "is missing" line as a claim to re-verify rather than a fact. The four I corrected first were the ones my own branches falsified; this one had been stale for longer and nothing I built touched it. `make check-assurance` unchanged at 86/86 and 276/276, no dead links — Requirement 5 stays `(planned)`, since the requirement as a whole is not discharged even though this part of it is. Refs: 013/Req-5, #645, #678 Co-Authored-By: Claude Opus 5 (1M context) --- .openspec/specs/013-embedding-api/spec.md | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/.openspec/specs/013-embedding-api/spec.md b/.openspec/specs/013-embedding-api/spec.md index bcacfd6c..6698349e 100644 --- a/.openspec/specs/013-embedding-api/spec.md +++ b/.openspec/specs/013-embedding-api/spec.md @@ -89,8 +89,14 @@ Each line is checkable against the tree at 0.18.10. that the examples copy `fixtures/empty.db` instead. 5. **Named parameters.** `:name`, `@name`, `$name` reach the always-NULL stub ADR-0015 left in place, which a public facade would make reachable. -6. **A durability contract.** `Pager` syncs (`src/pager.rs:589,597,697,782`) - but nothing states what is guaranteed, and `synchronous` has no handler. +6. **A durability contract in writing** -- not the mechanism, which + exists. `Pager` syncs (`src/pager.rs:589,597,697,782`) and + `PRAGMA synchronous` is fully implemented: the query form and all three + levels, with a decided per-level fsync-skip policy (#645, ADR-0036, + `src/vdbe/pragma.rs:79`). This spec originally said it "has no handler", + which was already false when written. What is genuinely absent is a + *stated* guarantee -- what a consumer is promised at commit, per level -- + so a reader has to derive it from `Pager`'s source. 7. **Incremental row access, at the facade.** `execute_with_db` and `execute_with_db_and_params` return `Vec>` (`src/vdbe/exec.rs:1073, 1093`), so those entry points still materialize a @@ -301,9 +307,13 @@ returns so a multi-statement transaction is one unit, and MUST roll back a transaction handle dropped without commit. It MUST also state what is durable at commit and honor `PRAGMA synchronous` at -least to distinguish FULL from OFF. Sync points exist -(`src/pager.rs:589,597,697,782,811,845`); what is missing is a documented -guarantee and any way to trade it. A consumer storing pointers to data it cannot +least to distinguish FULL from OFF. **The `synchronous` half of this is +already done** -- #645/ADR-0036 implement all three levels with a decided +fsync-skip policy per level, which is more than "at least FULL from OFF" +asks for. Sync points exist (`src/pager.rs:589,597,697,782,811,845`) and +there is now a way to trade them. What remains for this requirement is the +written guarantee, the transaction surface, and the retryable-error handling +below. A consumer storing pointers to data it cannot otherwise find needs that in writing: *SQE*'s file holds the metadata pointer for every table in a warehouse, so a commit returning before it is durable turns a power failure into tables that exist on object storage and are unreachable.