From 25c01d54bc3b2bac3dac63aac13f822e7b40c4ca Mon Sep 17 00:00:00 2001 From: Jacob Verhoeks Date: Fri, 28 Aug 2026 14:33:15 +0200 Subject: [PATCH 1/3] 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 366f343c7f2bd01c42959dcf6c1a6db2f8805f85 Mon Sep 17 00:00:00 2001 From: Jacob Verhoeks Date: Fri, 28 Aug 2026 15:53:25 +0200 Subject: [PATCH 2/3] 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 8eed977bb8f3c0ff3dd502d4d06fac7a44d842e9 Mon Sep 17 00:00:00 2001 From: Jacob Verhoeks Date: Fri, 28 Aug 2026 15:54:01 +0200 Subject: [PATCH 3/3] 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 0c2b02d5..009000d2 100644 --- a/.openspec/adr/index.md +++ b/.openspec/adr/index.md @@ -37,3 +37,4 @@ Specs record what the system must do; ADRs record **why it is shaped this way** | [0031](0031-vendor-nix-subset.md) | Vendor a `nix` subset: reintroduce a single, narrow `unsafe` boundary | 2026-08-26 | | [0032](0032-hash-group-by-second-strategy.md) | Hash `GROUP BY` is a second strategy with its own opcode family, and still emits groups in key order | 2026-08-27 | | [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-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