Skip to content

Declared tables: ddl becomes a wide-open multi-statement SQL batch - #994

Merged
thekevinscott merged 4 commits into
mainfrom
claude/tackle-956-xzen74
Aug 20, 2026
Merged

thekevinscott merged 4 commits into
mainfrom
claude/tackle-956-xzen74

Conversation

@thekevinbot

@thekevinbot thekevinbot commented Aug 20, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #956

A [[table]]'s ddl becomes a wide-open multi-statement SQL batch, and what it produced is settled against SQLite's catalog rather than by reading the text. B-tree indexes, FTS5 virtual tables and triggers become expressible in config at zero further dirsql cost.

Builds on #962 / #968 (required explicit name, catalog existence check).

What changed

  • db.rs — create_table(name, ddl) validates name as an identifier, then runs the whole batch through execute_batch inside one transaction. is_virtual_table_ddl and is_without_rowid_ddl (the two whitespace-normalizing string sniffs) are deleted, and with them DbError::DdlParse, which nothing could construct any more. New pub(crate) catalog helpers: user_table_names(conn) and table_catalog_entry(conn, name).
  • lib.rs — validation via the catalog, not parsing. A before/after pragma_table_list diff brackets the batch, then a pure classify_declared_table(name, entry, created) settles it: no row → TableNotCreated naming what the batch did create; type='virtual' → the new TableIsVirtual; wr=1 → the existing WITHOUT ROWID warning. A batch SQLite rejects becomes the new TableDdl { name, source }, whose Display prefixes SQLite's own message with table '<name>': .
  • persist.rs — sweep drop order. drop_user_tables now takes its order from pragma_table_list's type (virtuals first, shadows left to their owner) instead of sqlite_master's creation order. Today's order happens to be safe; dropping a vec0 shadow before its vtab poisons the vtab's drop even with IF EXISTS (probed against sqlite-vec 0.1.9), which a ddl batch can now reach.
  • Persist hash already covered the whole ddl string, so it needed no change — it is now genuinely the one invalidation lane, since the whole string finally runs. Covered by a new test rather than new code.

A bug this uncovered

Connection::execute prepares only the first statement of the string it is given and silently discards the tail. So a ddl with CREATE TABLE t (...); CREATE INDEX ... quietly created the table without the index, and a batch whose second statement SQLite could not even parse still loaded and exited 0. Both now behave as written.

Tests

Red-first: 2b1117a pushed the 11 failing tests alone, and CI went red on Integration tests with --test ddl_batch, 7 failures on the new assertions (the 4 e2e cases failed in the same run). Implementation followed in 9191ced.

  • packages/rust/tests/ddl_batch.rs (7): multi-statement batch runs every statement; an FTS5 index declared in ddl tracks inserts and deletes across a warm rebuild; a rejected batch fails the build and rolls back (asserted against the on-disk cache); a virtual declared table errors; the not-created error names what the batch created; a ddl edit rebuilds the cache; a rebuild sweeps a cache holding virtual tables.
  • packages/rust/tests/ddl_batch_e2e.rs (4): the batch indexes and queries through the real CLI, MATCH over the FTS5 index works, and both error shapes exit non-zero naming the entry.
  • Unit tests for every new surface: create_table batch/rollback/name-validation, user_table_names ordering and exclusions, table_catalog_entry kind + wr, all four classify_declared_table arms, the two new error Displays, and the sweep over a vtab plus its shadows.

packages/rust/tests/sdk.rs::ddl_sqlite_rejects_errors now asserts TableDdl and the table 't': prefix instead of a bare Core(_).

Manual run

.dirsql.toml declaring a row table + CREATE INDEX + an external-content FTS5 vtab + insert/delete triggers, against a real tree:

$ dirsql query "SELECT name FROM pragma_index_list('messages')" -c ./.dirsql.toml
[{"name":"messages_session"}]

$ dirsql query "SELECT m.idx, snippet(messages_fts,0,'[',']','~',8) AS hit,
                round(bm25(messages_fts),3) AS score
                FROM messages_fts JOIN messages m ON m.rowid = messages_fts.rowid
                WHERE messages_fts MATCH 'deploy' ORDER BY score" -c ./.dirsql.toml
[{"hit":"how do I [deploy] the worker","idx":0,"score":-0.0}]

$ dirsql query "SELECT COUNT(*) AS n FROM messages_fts WHERE messages_fts MATCH 'deploy'" -c ./.dirsql.toml --persist
[{"n":1}]
$ rm sessions/s1/messages/a.json
$ dirsql query "SELECT COUNT(*) AS n FROM messages_fts WHERE messages_fts MATCH 'deploy'" -c ./.dirsql.toml --persist
[{"n":0}]                      # the delete trigger kept the index current

All three error shapes, exit 1:

table 'records': SQLite error: near "(": syntax error in \nCREATE INDEX oops (\n at offset 19
table 'records': its `ddl` created a virtual table called 'records'. …
table 'messages': its `ddl` ran but created no table called 'messages' (it created: mesages, mesages_fts). …

Parity

No drift. ddl is an opaque string every SDK already forwards unchanged, so no binding signature moves and Python, Rust and TypeScript gain the batch, the catalog validation and the new load errors together. PARITY.md gains a "parity by construction" paragraph and four test-matrix rows.

E2E Verification

  • Ran e2e suites locally for every affected SDK
  • Python SDK e2e: pass (32 passed)
  • TypeScript SDK e2e: pass (16 passed, 5 files)
  • Rust core e2e (if applicable): pass (cargo test --workspace --features cli, which includes the *_e2e.rs CLI suites)
  • packages/python/e2e-attestation.json refreshed if packages/python changed (just e2e-attest-python)
  • packages/ts/e2e-attestation.json refreshed if packages/ts changed (just e2e-attest-ts)
  • Command(s) run:
    • cargo test --workspace --features cli
    • cargo clippy --workspace -- -D warnings, cargo fmt --all -- --check
    • uvx testing-conventions unit colocated-test / unit lint / integration lint --language rust
    • uvx testing-conventions unit mutation --language rust --base origin/main
    • uvx testing-conventions unit coverage --language rust packages/rust/src
    • cd packages/python && uv run python -m pytest tests/e2e/ -x -q (+ binding/hermetic/unit: 239 passed)
    • cd packages/ts && pnpm test:e2e (+ pnpm test:binding: 95 passed)
    • uvx testing-conventions e2e attest … in both packages → e2e-attestations/claude-tackle-956-xzen74.json
  • Result summary: all green. Mutation: 15 mutants tested, 0 survivors. Coverage (unit-only, --lib --features cli --branch): regions 92.90 / functions 92.13 / lines 93.61 / branch 78.49, all above the [rust.coverage] floors (91/91/92/72) — no floor change needed.

One local-only failure, unrelated and not reproducible in CI: packages/rust/tests/vtab.rs::unreadable_file_yields_null_content_without_erroring_the_row chmod 000s a file and expects the read to fail. This sandbox runs as uid 0, which reads it anyway. Verified pre-existing by checking out origin/main's core and re-running. The same applies to a first-run-only flake in packages/python/tests/integration/binding/persist_test.py::it_leaves_the_cache_file_untouched (WAL checkpoint timing right after a fresh maturin develop) — also reproduced on origin/main, passes on every subsequent run.

Changelog / Migrations

  • Changelog fragment added under <root>/<pkg>/changelog.d/ for each changed package (packages/rust/changelog.d/2026-08-20-ddl-batch.md)
  • Migration fragment added under <root>/<pkg>/migrations.d/ (packages/rust/migrations.d/2026-08-20-ddl-batch.md — TableNotCreated gained a field, TableDdl/TableIsVirtual are new, DbError::DdlParse is removed, and several runtime behaviors changed without an API change)

Only packages/rust has source changes; the two binding packages are touched solely by their refreshed e2e attestations.

Docs

docs/reference/config.md gains a ### Batch ddl section (the FTS5 recipe, the three catalog outcomes, the one-transaction rule, and the two consequences of ddl running once — batch-inserted rows are not file-tracked, and any ddl edit rebuilds the cache), plus new parse-error entries. Three places that said an extension-backed virtual table cannot be declared were corrected — it is the [[table]]'s own name that may not be virtual, not the batch: docs/reference/config.md ([[dirsql.extension]]), docs/howto/load-extension.md, docs/howto/search-by-meaning.md. docs/howto/define-tables.md gains "Add indexes and full-text search".

claude added 4 commits August 20, 2026 20:11
Integration and CLI e2e coverage for the behavior #956 asks for: a
`[[table]]`'s `ddl` runs as a whole SQL batch, and what it produced is
settled against SQLite's catalog rather than by reading the text.

All eleven fail today, and two of them expose the reason this matters:
`Connection::execute` prepares only the first statement of the string and
silently discards the tail, so `CREATE INDEX` never runs and a batch whose
second statement SQLite cannot even parse still exits 0.

skip-changelog: red tests only; the behavior lands in the next commit
`ddl` was one statement, string-sniffed for `CREATE VIRTUAL TABLE` and
`WITHOUT ROWID` — and `Connection::execute` prepared only the first statement
of whatever was handed to it, so a second one was silently discarded. Indexes,
FTS5 tables and triggers were inexpressible in config, and a batch whose tail
SQLite could not parse still loaded.

It is now a wide-open batch: `execute_batch` inside one transaction, so a
rejected statement leaves nothing behind and surfaces SQLite's own message
under the entry that caused it. What the batch produced is settled against
`pragma_table_list`, never by reading DDL text — no row for `name` fails the
load naming what the batch did create, a virtual `name` fails, and `wr=1`
keeps the existing WITHOUT ROWID warning. Both string sniffs and the now
unconstructible `DbError::DdlParse` are deleted.

The cache sweep takes its drop order from the catalog's type, virtuals first,
so a `vec0` table's shadow tables can no longer be dropped out from under it.
The persist hash already covered the whole `ddl` string, so editing any part
of a batch remains the one invalidation lane.

Closes #956
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Declared tables: wide-open multi-statement ddl, catalog validation

3 participants