Repository navigation
Declared tables: ddl becomes a wide-open multi-statement SQL batch - #994
Merged
Merged
Conversation
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #956
A
[[table]]'sddlbecomes 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)validatesnameas an identifier, then runs the whole batch throughexecute_batchinside one transaction.is_virtual_table_ddlandis_without_rowid_ddl(the two whitespace-normalizing string sniffs) are deleted, and with themDbError::DdlParse, which nothing could construct any more. Newpub(crate)catalog helpers:user_table_names(conn)andtable_catalog_entry(conn, name).lib.rs— validation via the catalog, not parsing. A before/afterpragma_table_listdiff brackets the batch, then a pureclassify_declared_table(name, entry, created)settles it: no row →TableNotCreatednaming what the batch did create;type='virtual'→ the newTableIsVirtual;wr=1→ the existing WITHOUT ROWID warning. A batch SQLite rejects becomes the newTableDdl { name, source }, whoseDisplayprefixes SQLite's own message withtable '<name>':.persist.rs— sweep drop order.drop_user_tablesnow takes its order frompragma_table_list's type (virtuals first, shadows left to their owner) instead ofsqlite_master's creation order. Today's order happens to be safe; dropping avec0shadow before its vtab poisons the vtab's drop even withIF EXISTS(probed against sqlite-vec 0.1.9), which addlbatch can now reach.ddlstring, 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::executeprepares only the first statement of the string it is given and silently discards the tail. So addlwithCREATE 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:
2b1117apushed the 11 failing tests alone, and CI went red onIntegration testswith--test ddl_batch, 7 failures on the new assertions (the 4 e2e cases failed in the same run). Implementation followed in9191ced.packages/rust/tests/ddl_batch.rs(7): multi-statement batch runs every statement; an FTS5 index declared inddltracks 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; addledit 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,MATCHover the FTS5 index works, and both error shapes exit non-zero naming the entry.create_tablebatch/rollback/name-validation,user_table_namesordering and exclusions,table_catalog_entrykind +wr, all fourclassify_declared_tablearms, the two new errorDisplays, and the sweep over a vtab plus its shadows.packages/rust/tests/sdk.rs::ddl_sqlite_rejects_errorsnow assertsTableDdland thetable 't':prefix instead of a bareCore(_).Manual run
.dirsql.tomldeclaring a row table +CREATE INDEX+ an external-content FTS5 vtab + insert/delete triggers, against a real tree:All three error shapes, exit 1:
Parity
No drift.
ddlis 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.mdgains a "parity by construction" paragraph and four test-matrix rows.E2E Verification
cargo test --workspace --features cli, which includes the*_e2e.rsCLI suites)packages/python/e2e-attestation.jsonrefreshed ifpackages/pythonchanged (just e2e-attest-python)packages/ts/e2e-attestation.jsonrefreshed ifpackages/tschanged (just e2e-attest-ts)cargo test --workspace --features clicargo clippy --workspace -- -D warnings,cargo fmt --all -- --checkuvx testing-conventions unit colocated-test / unit lint / integration lint --language rustuvx testing-conventions unit mutation --language rust --base origin/mainuvx testing-conventions unit coverage --language rust packages/rust/srccd 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--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_rowchmod 000s a file and expects the read to fail. This sandbox runs as uid 0, which reads it anyway. Verified pre-existing by checking outorigin/main's core and re-running. The same applies to a first-run-only flake inpackages/python/tests/integration/binding/persist_test.py::it_leaves_the_cache_file_untouched(WAL checkpoint timing right after a freshmaturin develop) — also reproduced onorigin/main, passes on every subsequent run.Changelog / Migrations
<root>/<pkg>/changelog.d/for each changed package (packages/rust/changelog.d/2026-08-20-ddl-batch.md)<root>/<pkg>/migrations.d/(packages/rust/migrations.d/2026-08-20-ddl-batch.md—TableNotCreatedgained a field,TableDdl/TableIsVirtualare new,DbError::DdlParseis removed, and several runtime behaviors changed without an API change)Only
packages/rusthas source changes; the two binding packages are touched solely by their refreshed e2e attestations.Docs
docs/reference/config.mdgains a### Batch ddlsection (the FTS5 recipe, the three catalog outcomes, the one-transaction rule, and the two consequences ofddlrunning once — batch-inserted rows are not file-tracked, and anyddledit 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 ownnamethat 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.mdgains "Add indexes and full-text search".