Repository navigation
fix(driver-turso): a Turso remote database enforces the storage.notNull a field declares - #23893
Conversation
… storage.notNull RemoteTransport's CREATE TABLE and both ALTER TABLE ADD COLUMN sites (syncSchema, syncSchemasBatch) now render a declared field's column through one helper, columnDeclarationSQL: the storage type, then NOT NULL when the field declares storage.notNull, asked as SqlDriver.createColumn asks it (truthiness, every type, multi-value included, never required). A refused ADD COLUMN now names its statement, as knex's error does on the local face. The remote drift-report caveat is reworded: the sync creates the constraint now, but databases synced earlier keep nullable columns. Claude-Session: https://claude.ai/code/session_01TQLy3Yq6vF4SDpTXdr3Uwi Co-authored-by: Claude <noreply@anthropic.com>
…, not a transport's error code Claude-Session: https://claude.ai/code/session_01TQLy3Yq6vF4SDpTXdr3Uwi Co-authored-by: Claude <noreply@anthropic.com>
…rso-remote-not-null
…drift caveat, leaving the remedy clause byte-identical Claude-Session: https://claude.ai/code/session_01TQLy3Yq6vF4SDpTXdr3Uwi Co-authored-by: Claude <noreply@anthropic.com>
…rso-remote-not-null
📓 Docs Drift CheckThis PR changes 1 package(s): 3 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 7 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin d2b7cef8305a3076d7e32c6854acb427b5145d02 && git checkout d2b7cef8305a3076d7e32c6854acb427b5145d02
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 497518c3f67d8cc022fc6663d2e08f97117298a4 fcc038d2fb05e6152f7c05431834e0ea06e3f9b8 && git checkout -B drift-repro 497518c3f67d8cc022fc6663d2e08f97117298a4 && git merge --no-ff fcc038d2fb05e6152f7c05431834e0ea06e3f9b8
node scripts/docs-audit/affected-docs.mjs --json 497518c3f67d8cc022fc6663d2e08f97117298a4
|
席位核对 · ✅ ACCEPT: PR #23893 at
|
Fixes #23847
Clause-②: no
What was wrong
RemoteTransport, driver-turso's remote face and thelibsql://transport that hosted environments use, built every declared column withoutNOT NULL.buildCreateTableSQLand bothALTER TABLE … ADD COLUMNsites (insyncSchemaand insyncSchemasBatch) wrote only the storage type. So a field declaredstorage: { notNull: true }was unenforced on every Turso remote database. The local face'sSqlDriver.createColumnhas always created that columnNOT NULL(ADR-0113). The shared differ then read each such column as destructivetighten_not_nulldrift, on a database the remote sync had only just created.What changes
RemoteTransport.columnDeclarationSQL(field)is now the one renderer for every statement that materializes a declared field's column on this face. It writes the storage type frommapFieldTypeToSQL, thenNOT NULLwhendeclaresColumnNotNull(field)holds. The predicate asks whatcreateColumn's owndeclaresColumnNotNullasks: whetherstorage.notNullis truthy. That holds on every field type, multi-value JSON columns included (the Both migration generators emit NOT NULL on amultiple: truecolumn, whichSqlDriver.createColumnreturns before ever constraining — and the pin asserts the generator side #17231 case), andrequiredalone never triggers it.buildCreateTableSQL, thesyncSchemaADD COLUMN and thesyncSchemasBatchADD COLUMN. The three remote schema doors (syncSchemasBatch,syncSchema,initObjects) all reach those statements.ADD COLUMNfromsyncSchemanow names its statement. It uses knex'sSQL - messageform, as the local face's error already does. libSQL's own message names no table and no column, and the engine's per-object report names only the object. The error object, its class and its code are unchanged.turso-driver.ts, the caveat text only. The remote face'sdriftReportCaveatsentence, and the header docblock above it, said in the present tense that the remote sync creates noNOT NULL. This change makes that false, so those NOT NULL sentences now say that tables created by an earlier release keep their nullable columns. The caveat stays non-null. Every line carrying the "a destructive entry has no remedy on this transport" clause, and its docblock twin, is byte-identical tomain; os migrate apply cannot run on a Turso remote database (it arms deferred DDL), so destructive drift on a remote primary has no CLI remedy #23848 owns that clause (relay 6213357665, accepted in 6213866355).Measurements
The committed test uses the instrument
turso-remote-drift-detection.test.tsestablished: a remote-modeTursoDriverover a real@libsql/clientfile:client (libSQL 3.45.1), with the local face as the control. A second leg ran against a real server: the officialsqld 0.24.32(tursodatabase/libsql releaselibsql-server-v0.24.32, sha25671720fc8…336879f, matching its published checksum), reached aslibsql://127.0.0.1:PORT?tls=0. That leg is a scratch suite and is not committed, because CI has no sqld.Positive control on unchanged code (
f7e836da1):titleandtags(bothstorage.notNull) withnotnull=0, anddetectManagedDriftanswerstags:tighten_not_nullandtitle:tighten_not_null. The ADD COLUMN path behaves the same. A populated table accepts the "notNull" column silently, as nullable.file:client: the remote reader and the local face reading the same file both report the same twotighten_not_nullentries. A table the local face created reads[].On the fix (sqld re-run at
5a41f2167): every door givestitle:nn=1 tags:nn=1 plain:nn=0and drift[], on CREATE TABLE and on ADD COLUMN. A populated table refuses withALTER TABLE "…" ADD COLUMN "added" TEXT NOT NULL - SQLITE_UNKNOWN: SQLite error: Cannot add a NOT NULL column with default value NULL, and the column stays absent.Ablation:
scripts/ablation-replace.mjsin WRAP mode replacedreturn declaresColumnNotNull(field) ? … NOT NULL … : colType;withreturn colType;(anchor 1 to 0, blob039d3cd8tob133667a), and the new suite ran while mutated. Result: 10 of 10 red. The restore is proven: the blob equals HEAD's039d3cd8andgit diff HEADis empty. The test reaches the subject through relative imports, so it readssrc/and no build sits between the mutation and the run.turso-remote-not-null.test.ts, 10 casessyncSchemasBatch/syncSchema/initObjects: the remotenotnullflags equal the local face's for the same declarations (title,amount,activeand multi-valuetagsare 1;required-onlynote,notNull: falseand plain are 0). The remote and the local reader both report drift[].titleNULL is refused by the database on both faces, and no row lands.Cannot add a NOT NULL column with default value NULL). The remotesyncSchemarefusal namesALTER TABLE "rows_t" ADD COLUMN "added" TEXT NOT NULL, and no nullable column is slipped in.ADD COLUMN on a table that already holds rows
Once a table has a row, SQLite refuses
ADD COLUMN … NOT NULLwithout a non-NULL DEFAULT. An empty table accepts it. Both were measured on libSQL 3.45.1 and on sqld 0.24.32. The local face sends the same statement and gets the same refusal, measured asalter table … add column … text not null - Cannot add a NOT NULL column with default value NULL. So that object's sync fails loudly on both faces, and the engine reports it per object:ObjectQLPluginreplays a failed batch per object at warn first, so only the failing object loses its DDL. TheNOT NULLis never dropped to let the statement pass.One case still differs, and it is about defaults rather than nullability. The remote face emits no declared column DEFAULT for any field, while
createColumnemits a literaldefaultValue(applyDeclaredColumnDefault). So astorage.notNullfield with a literaldefaultValue, added to a populated table, is accepted locally (existing rows take the default) and refused here. Case 5 pins that as a named asymmetry, to be inverted when this face emits declared defaults. Whether it should, and from which shared source, goes back to the dispatching seat as an open question. A third copy of the default rules (the CLI'sdeclaredColumnDefaultis already the second) is a choice for that seat, not for this PR.Not in this PR
tighten_not_null. No existing database is migrated here. That is why the caveat stays, and why theOS_ARTIFACT_URLboot gate still does not act on a remote report (os migrate apply cannot run on a Turso remote database (it arms deferred DDL), so destructive drift on a remote primary has no CLI remedy #23848 is the other precondition).Acceptance notes
turso-driver.tsNOT NULL sentences. The caveat's existing pin (turso-remote-drift-detection.test.ts, which expects the text to containNOT NULL) still holds.packages/cli/src/utils/artifact-boot-migration.tshas a code comment whose present tense is now stale in the same way: "its schema sync does not create a declared NOT NULL". It is a comment in another package, so it is not edited here. carrier: os migrate apply cannot run on a Turso remote database (it arms deferred DDL), so destructive drift on a remote primary has no CLI remedy #23848, which reworks that gate's remote branch.declaresColumnNotNullis module-private, so this transport spells the same question itself. The new test pins their parity: remotenotnullflags equal the local face's for one declaration set. Exporting the driver-sql predicate would give it one source; noted, not filed.f7e836da1: the remote CREATE TABLE givescreated_at/updated_atTEXT DEFAULT (datetime('now'))(the zone-naiveYYYY-MM-DD HH:MM:SSform), while the local face givesDEFAULT strftime('%Y-%m-%dT%H:%M:%fZ', 'now'). Whether any write path leaves those columns to the database default was not measured, so this is noted, not filed. (Added by thedomain:engineseat 2 from the dev's report.)f7e836da1) and after (fcc038d2f).Tests (head
fcc038d2f)pnpm --filter @objectstack/driver-turso test: 91 files, 2425 passed, 33 skipped (VERDICT command-exit 0).pnpm --filter @objectstack/driver-turso typecheck: exit 0.tsc --listFilesincludes all 91 test files, the new one among them..tsfiles: exit 0.Gates (head
fcc038d2f)node scripts/pm/dispatch-gates.mjs --commandsderived 67 families, and all 67 exit 0.--ranreports 67 derived, 67 run, 0 NOT-MEASURED, 0 UNRUN.check-changeset-fixed,check:authz-resolver,check:error-code-casing,check:filter-alias-parity.Generated by Claude Code