Skip to content

Commit 99bd27c

Browse files
dmealingclaude
andcommitted
fix(migrate): D1/SQLite verify no longer reports false schema drift
meta verify --dialect d1 reported permanent, unfixable schema drift for a hand-migrated D1/SQLite database that genuinely matches its metadata, because the shared sqlite/d1 diff path compared distinctions SQLite cannot physically store. Four fixes, scoped to sqlite/d1 (Postgres unchanged): - Type canon: on sqlite/d1, collapse json->text and drop text maxLength before comparing (a hand-written bare TEXT column IS the maxLength'd / jsonb one — SQLite stores neither). Also stops meta migrate emitting a no-op change for a metadata-only maxLength change on sqlite. - Anonymous CHECK reconciliation: introspection parses unnamed inline CHECK(...) (masking comments + string literals so a CHECK( inside them is never mis-parsed), and the diff matches a modeled named check to an actual by normalized expression on sqlite/d1. Comma spacing in an IN(...) list is normalized outside literals only. - Bare-NULL default: the D1/wrangler runner stringifies SQL NULL to "null", read as a literal default on every no-default column (permanent, destructive-recreate false drift). A bare null/NULL is now no default; a quoted 'null' stays a literal. Gated by a new sqlite-hand-migrated-verify integration test + parseSqliteChecks / normalizeCheckExpr unit gates (598 pass). An adversarial review caught two regressions (a literal-unaware comma-collapse hiding regex/predicate drift; the relaxed CHECK finder matching inside comments/literals) — both fixed + pinned. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014xy8powhHYJ6gfFt9Ut8dL
1 parent 0792bfd commit 99bd27c

7 files changed

Lines changed: 436 additions & 27 deletions

File tree

CHANGELOG.md

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,35 @@ this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
77

88
## [Unreleased]
99

10+
### Fixed — D1/SQLite `verify` false schema drift (npm-only; `migrate-ts` + `cli`)
11+
12+
`meta verify --dialect d1` reported permanent, unfixable schema drift for a
13+
hand-migrated D1/SQLite database whose schema genuinely matches its metadata,
14+
because the shared sqlite/d1 diff path compared distinctions SQLite cannot
15+
physically represent. Four fixes, all scoped to sqlite/d1 (Postgres behavior
16+
unchanged) and gated by a new `sqlite-hand-migrated-verify` integration test plus
17+
`parseSqliteChecks` / `normalizeCheckExpr` unit gates:
18+
19+
- **`json` and `VARCHAR(N)` length no longer read as drift.** SQLite/D1 has one text
20+
storage class — a `field.object @storage:jsonb` column is stored as `TEXT`, and a
21+
`VARCHAR(N)` length is cosmetic (not enforced). A hand-written bare `TEXT` column is
22+
the same physical column as the maxLength'd / jsonb one, so the diff now canonicalizes
23+
`json→text` and drops text `maxLength` on sqlite/d1 before comparing. This also
24+
corrects `meta migrate`: a metadata-only `maxLength` change on sqlite/d1 now emits no
25+
migration — nothing physical changes in SQLite.
26+
- **Anonymous inline `CHECK (…)` constraints now reconcile.** Hand-written migrations
27+
write unnamed inline checks; introspection parses them (previously skipped, and now
28+
with a mask so a `CHECK (` inside a comment or string literal is never mis-parsed) and
29+
the diff matches a modeled named check against an actual check by normalized expression
30+
on sqlite/d1, so an already-enforced constraint is no longer re-proposed as add-check.
31+
Comma spacing in an `IN (…)` list is normalized *outside string literals only*
32+
(`'a', 'b'` == `'a','b'`, but a comma inside a literal/regex stays significant).
33+
- **A bare `NULL` default is now no default.** The D1/wrangler runner stringifies a SQL
34+
`NULL` `dflt_value` to the string `"null"`, which was read as a literal default on
35+
every no-default column — permanent (and, on SQLite/D1, destructive recreate-and-copy)
36+
false drift on every `verify`/`migrate`. `DEFAULT NULL` is equivalent to no default; a
37+
quoted `'null'` stays a genuine string literal.
38+
1039
## [0.20.1] — 2026-07-25
1140

1241
**npm `0.20.1`** (patch; NuGet 0.19.3 / PyPI 0.19.5 / Maven 7.11.3 unchanged). `meta init` quickstart polish: the post-init next-steps no longer lists the working `meta gen` / `meta docs` under "ship in later sub-projects" (only `ingest`/`serve`/`install-hooks` are unshipped), and `meta init` now scaffolds a minimal root `.gitignore` (node_modules/, *.sqlite, dist/) when the project has none, so a `git add -A` right after init does not stage node_modules or a local dev DB. No API or codegen-output change.

server/typescript/packages/migrate-ts/src/check-expr-compare.ts

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,49 @@ export function normalizeCheckExpr(expr: string): string {
3636
.trim();
3737
// PG stores `col IN (…)` as `col = ANY (ARRAY[…])`; after the bracket strip above
3838
// that reads `col = any array …`. Fold it back to the `col in …` form we emit.
39-
return stripped.replace(/=\s*any\s+array/g, "in").replace(/\s+/g, " ").trim();
39+
const folded = stripped.replace(/=\s*any\s+array/g, "in").replace(/\s+/g, " ").trim();
40+
// Normalize spacing AROUND COMMAS so a generated IN-list (`'a', 'b'`) compares equal
41+
// to a hand-written one (`'a','b'`) — the separator commas are never semantically
42+
// meaningful, and a hand SQLite migration omits the space. This is done OUTSIDE
43+
// single-quoted literals only: a comma inside a string/regex literal (`'a, b'`, or a
44+
// quantifier `'{1, 3}'`) IS meaningful, so two checks that differ only there must
45+
// stay distinct (else a real regex/predicate change reads as clean drift).
46+
return collapseCommaSpacingOutsideQuotes(folded);
47+
}
48+
49+
/** Collapse `\s*,\s*` → `,` OUTSIDE single-quoted literals (see normalizeCheckExpr). */
50+
function collapseCommaSpacingOutsideQuotes(s: string): string {
51+
let out = "";
52+
let i = 0;
53+
const n = s.length;
54+
while (i < n) {
55+
const ch = s[i]!;
56+
if (ch === "'") {
57+
// Copy the single-quoted literal verbatim ('' escapes honored) — never touched.
58+
const start = i;
59+
i++;
60+
while (i < n) {
61+
if (s[i] === "'") {
62+
if (s[i + 1] === "'") { i += 2; continue; }
63+
i++;
64+
break;
65+
}
66+
i++;
67+
}
68+
out += s.slice(start, i);
69+
continue;
70+
}
71+
if (ch === ",") {
72+
out = out.replace(/\s+$/, ""); // drop whitespace already emitted before it
73+
out += ",";
74+
i++;
75+
while (i < n && /\s/.test(s[i]!)) i++; // skip whitespace after it
76+
continue;
77+
}
78+
out += ch;
79+
i++;
80+
}
81+
return out;
4082
}
4183

4284
/** True when two CHECK expressions are equivalent after normalization. */

server/typescript/packages/migrate-ts/src/diff/index.ts

Lines changed: 68 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import type {
33
ViewDescriptor,
44
DependentRelation,
55
Change, ChangeStatus, DiffResult, AllowOptions, AmbiguousCallback, Dialect,
6+
CheckDescriptor,
67
} from "../types.js";
78
import type { SqlType } from "../sql-type.js";
89
import { sqlTypeEquals } from "../sql-type.js";
@@ -212,7 +213,7 @@ export async function diff(
212213
for (const [id, expectedTable] of expectedTables) {
213214
const actualTable = actualTables.get(id);
214215
if (!actualTable) continue;
215-
diffTableColumns(expectedTable, actualTable, changes);
216+
diffTableColumns(expectedTable, actualTable, changes, args.dialect);
216217
diffTableIndexes(expectedTable, actualTable, changes);
217218
diffTableForeignKeys(expectedTable, actualTable, changes, args.dialect);
218219
// CHECK constraints on existing tables are evolved whenever a dialect is
@@ -223,7 +224,7 @@ export async function diff(
223224
// no dialect (legacy positional callers) checks stay un-diffed — those
224225
// callers may hold hand-built snapshots with empty check lists, and
225226
// diffing them would re-propose every modeled check forever.
226-
if (args.dialect !== undefined) diffTableChecks(expectedTable, actualTable, changes);
227+
if (args.dialect !== undefined) diffTableChecks(expectedTable, actualTable, changes, args.dialect);
227228
}
228229

229230
// Pass 2b: views. Identity is (schema, name). How "changed" is decided is
@@ -272,10 +273,33 @@ function isDiffArgs(x: DiffArgs | SchemaSnapshot): x is DiffArgs {
272273
return "expected" in x && "actual" in x;
273274
}
274275

276+
/**
277+
* Collapse a SqlType to what SQLite/D1 can PHYSICALLY store, so the diff never
278+
* reports drift the database could never represent (and thus could never fix).
279+
*
280+
* SQLite/D1 has a single text storage class: `json` is stored as TEXT (no native
281+
* json type), and a `VARCHAR(N)` length is cosmetic — not enforced — so a bounded
282+
* and an unbounded text column are the same physical column. Both collapse to bare
283+
* `text`. The tool's own emit round-trips are unaffected (VARCHAR(8) and TEXT both
284+
* canon to `text`), while a hand-written `TEXT` column now matches maxLength'd /
285+
* jsonb metadata. Postgres represents both faithfully and is left exact.
286+
*/
287+
function canonTypeForDialect(t: SqlType, dialect: Dialect | undefined): SqlType {
288+
if (dialect !== "sqlite" && dialect !== "d1") return t;
289+
if (t.kind === "json") return { kind: "text" };
290+
if (t.kind === "text" && t.maxLength !== undefined) return { kind: "text" };
291+
return t;
292+
}
293+
294+
function sqlTypeEqualsForDialect(a: SqlType, b: SqlType, dialect: Dialect | undefined): boolean {
295+
return sqlTypeEquals(canonTypeForDialect(a, dialect), canonTypeForDialect(b, dialect));
296+
}
297+
275298
function diffTableColumns(
276299
expected: TableDescriptor,
277300
actual: TableDescriptor,
278301
changes: Change[],
302+
dialect?: Dialect,
279303
): void {
280304
const table = expected.name;
281305
const sx = schemaSpread(expected.schema);
@@ -288,8 +312,11 @@ function diffTableColumns(
288312
changes.push({ kind: "add-column", table, ...sx, column: ec, status: ALLOWED });
289313
continue;
290314
}
291-
// Compare type, nullable, default — emit per-aspect change.
292-
if (!sqlTypeEquals(ec.sqlType, ac.sqlType)) {
315+
// Compare type, nullable, default — emit per-aspect change. Type equality is
316+
// dialect-aware: SQLite/D1 cannot physically represent a VARCHAR length or a
317+
// native json type, so those distinctions must not read as drift (see
318+
// canonTypeForDialect). Postgres stays exact.
319+
if (!sqlTypeEqualsForDialect(ec.sqlType, ac.sqlType, dialect)) {
293320
changes.push({
294321
kind: "change-column-type", table, ...sx, column: name,
295322
from: ac.sqlType, to: ec.sqlType, status: ALLOWED,
@@ -441,23 +468,46 @@ function diffTableForeignKeys(
441468
}
442469
}
443470

444-
function diffTableChecks(expected: TableDescriptor, actual: TableDescriptor, changes: Change[]): void {
471+
function diffTableChecks(
472+
expected: TableDescriptor,
473+
actual: TableDescriptor,
474+
changes: Change[],
475+
dialect?: Dialect,
476+
): void {
445477
const sx = schemaSpread(expected.schema);
446-
const expectedChk = new Map(expected.checks.map((c) => [c.name, c]));
447-
const actualChk = new Map(actual.checks.map((c) => [c.name, c]));
448-
for (const [name, ec] of expectedChk) {
449-
const ac = actualChk.get(name);
450-
if (!ac) {
451-
changes.push({ kind: "add-check", table: expected.name, ...sx, check: ec, status: ALLOWED });
452-
} else if (!checkExprEquals(ec.expression, ac.expression)) {
453-
changes.push({ kind: "drop-check", table: expected.name, ...sx, check: name, restore: ac, status: ALLOWED });
454-
changes.push({ kind: "add-check", table: expected.name, ...sx, check: ec, status: ALLOWED });
478+
// SQLite/D1 stores no constraint identity for an inline `CHECK (…)`, so hand-written
479+
// DDL yields anonymous (empty-name) checks. When an expected named check has no
480+
// same-name actual, fall back to matching by NORMALIZED EXPRESSION so an
481+
// already-enforced constraint is not re-proposed as add-check (and its actual is not
482+
// re-proposed as drop-check). The fallback scans ALL still-unconsumed actual checks,
483+
// so it also reconciles a name-MISMATCHED actual (a check the DB named differently
484+
// than the model's generated `<table>_<col>_chk`), not only truly-anonymous ones —
485+
// beneficial and safe: the name-keyed pass runs first, so a genuine expression change
486+
// on a name-matched check is still caught as drop+add. Postgres constraints are always
487+
// named, so it keeps the exact name-keyed behavior (exprFallback off).
488+
const exprFallback = dialect === "sqlite" || dialect === "d1";
489+
const actualByName = new Map(actual.checks.filter((c) => c.name !== "").map((c) => [c.name, c]));
490+
const consumed = new Set<CheckDescriptor>();
491+
492+
for (const ec of expected.checks) {
493+
const ac = actualByName.get(ec.name);
494+
if (ac) {
495+
consumed.add(ac);
496+
if (!checkExprEquals(ec.expression, ac.expression)) {
497+
changes.push({ kind: "drop-check", table: expected.name, ...sx, check: ec.name, restore: ac, status: ALLOWED });
498+
changes.push({ kind: "add-check", table: expected.name, ...sx, check: ec, status: ALLOWED });
499+
}
500+
continue;
455501
}
456-
}
457-
for (const [name, ac] of actualChk) {
458-
if (!expectedChk.has(name)) {
459-
changes.push({ kind: "drop-check", table: expected.name, ...sx, check: name, restore: ac, status: ALLOWED });
502+
if (exprFallback) {
503+
const match = actual.checks.find((a) => !consumed.has(a) && checkExprEquals(ec.expression, a.expression));
504+
if (match) { consumed.add(match); continue; }
460505
}
506+
changes.push({ kind: "add-check", table: expected.name, ...sx, check: ec, status: ALLOWED });
507+
}
508+
for (const ac of actual.checks) {
509+
if (consumed.has(ac)) continue;
510+
changes.push({ kind: "drop-check", table: expected.name, ...sx, check: ac.name, restore: ac, status: ALLOWED });
461511
}
462512
}
463513

server/typescript/packages/migrate-ts/src/introspect/sqlite-shared.ts

Lines changed: 64 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,16 @@ export const SQLITE_EXPR_DEFAULT_PATTERNS = [
1818
export function parseSqliteDefault(raw: string | null): ColumnDefault | undefined {
1919
if (raw === null || raw === undefined || raw === "") return undefined;
2020

21+
// A bare (unquoted) NULL keyword is SQL for "no default": `DEFAULT NULL` is
22+
// functionally identical to declaring no default at all. It reaches us as the
23+
// JSON string "null" from the D1/wrangler runner (which stringifies a SQL NULL
24+
// dflt_value) or as `NULL` from parsed DDL. A GENUINE string literal default of
25+
// "null" is QUOTED (`'null'`) and is matched by the quoted branch below, so this
26+
// bare test is unambiguous. Without it every no-default column on D1 reads back a
27+
// literal "null" default that no metadata field ever declares — permanent, and on
28+
// SQLite/D1 destructive (recreate-and-copy) false drift on every verify/migrate.
29+
if (raw.trim().toLowerCase() === "null") return undefined;
30+
2131
// A QUOTE-WRAPPED value is a string literal BY CONSTRUCTION — SQLite always quotes a
2232
// literal string default. This test MUST come before the expr patterns: one of those
2333
// patterns is a bare /\(.*\)/, so a perfectly ordinary literal containing parentheses
@@ -95,9 +105,13 @@ export function sqliteTypeToSqlType(declaredType: string): SqlType {
95105
* Without it the actual side always reported `checks: []` and every expected
96106
* check re-surfaced as add-check on every single run.
97107
*
98-
* Unnamed checks (`CHECK (…)` with no CONSTRAINT clause — hand-written DDL) are
99-
* NOT parsed: they have no identity to match on. A modeled check over such a
100-
* table converges after one recreate (which rewrites it in named form).
108+
* Unnamed inline checks (`CHECK (…)` with no CONSTRAINT clause — the idiomatic
109+
* hand-written-migration form) ARE parsed, with an empty name. They have no
110+
* identity to match by name, so the diff reconciles them by NORMALIZED EXPRESSION
111+
* on sqlite/d1: an anonymous DB check that already enforces a modeled constraint
112+
* is matched to the expected named check rather than re-proposed as add-check.
113+
* (Before, they were skipped entirely, so every modeled check over such a table
114+
* re-surfaced as false drift on `verify --dialect d1` on every run.)
101115
*
102116
* The expression is scanned with balanced parens and string-literal awareness
103117
* (an enum member may contain `(`/`)`), and returned verbatim — the diff's
@@ -106,10 +120,18 @@ export function sqliteTypeToSqlType(declaredType: string): SqlType {
106120
export function parseSqliteChecks(createSql: string | null | undefined): CheckDescriptor[] {
107121
if (createSql === null || createSql === undefined || createSql === "") return [];
108122
const out: CheckDescriptor[] = [];
109-
const re = /\bCONSTRAINT\s+(?:"((?:[^"]|"")+)"|([A-Za-z_][A-Za-z0-9_$]*))\s+CHECK\s*\(/gi;
123+
// Find `CHECK (` on a MASKED copy (comments + single-quoted literals blanked to
124+
// spaces of equal length) so a `CHECK (` appearing inside a `--`/`/* */` comment
125+
// or a string literal (`DEFAULT 'see CHECK (x)'`) can NEVER be mis-parsed as a
126+
// constraint. Positions are 1:1, so the balanced expression is sliced from the
127+
// ORIGINAL (with its real quotes intact).
128+
const masked = maskCommentsAndStrings(createSql);
129+
// An optional `CONSTRAINT <name>` prefix then `CHECK (`. Named → group 1/2;
130+
// bare inline `CHECK (` → name defaults to "" (anonymous, expression-matched).
131+
const re = /(?:\bCONSTRAINT\s+(?:"((?:[^"]|"")+)"|([A-Za-z_][A-Za-z0-9_$]*))\s+)?\bCHECK\s*\(/gi;
110132
let m: RegExpExecArray | null;
111-
while ((m = re.exec(createSql)) !== null) {
112-
const name = m[1] !== undefined ? m[1].replace(/""/g, '"') : m[2]!;
133+
while ((m = re.exec(masked)) !== null) {
134+
const name = m[1] !== undefined ? m[1].replace(/""/g, '"') : (m[2] ?? "");
113135
const open = re.lastIndex - 1; // position of the "(" the regex just consumed
114136
let depth = 0;
115137
let inString = false;
@@ -137,6 +159,42 @@ export function parseSqliteChecks(createSql: string | null | undefined): CheckDe
137159
return out;
138160
}
139161

162+
/**
163+
* Same-length copy of a CREATE TABLE DDL with SQL comments (`--` line, `/* *\/`
164+
* block) and single-quoted string literals blanked to spaces, so a token scan
165+
* (e.g. the CHECK finder) cannot match inside them. Double-quoted identifiers are
166+
* PRESERVED — a constraint or column name may be double-quoted, and none legitimately
167+
* contains a `CHECK (` token. Positions are unchanged for downstream slicing.
168+
*/
169+
function maskCommentsAndStrings(sql: string): string {
170+
const chars = sql.split("");
171+
let i = 0;
172+
const n = sql.length;
173+
while (i < n) {
174+
const ch = sql[i];
175+
if (ch === "'") {
176+
const end = skipSingleQuoted(sql, i);
177+
for (let j = i; j < end; j++) chars[j] = " ";
178+
i = end;
179+
} else if (ch === '"') {
180+
i = skipDoubleQuoted(sql, i); // preserve the identifier verbatim
181+
} else if (ch === "-" && sql[i + 1] === "-") {
182+
let j = i;
183+
while (j < n && sql[j] !== "\n") { chars[j] = " "; j++; }
184+
i = j;
185+
} else if (ch === "/" && sql[i + 1] === "*") {
186+
let j = i + 2;
187+
while (j < n && !(sql[j] === "*" && sql[j + 1] === "/")) j++;
188+
const end = Math.min(n, j + 2);
189+
for (let k = i; k < end; k++) chars[k] = " ";
190+
i = end;
191+
} else {
192+
i++;
193+
}
194+
}
195+
return chars.join("");
196+
}
197+
140198
/** Skip a single-quoted SQL string starting at `i` (position of the opening
141199
* quote); returns the position just past the closing quote ('' escapes honored). */
142200
function skipSingleQuoted(s: string, i: number): number {

server/typescript/packages/migrate-ts/test/check-evolution/normalize.test.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ describe("normalizeCheckExpr", () => {
2020
"status = ANY (ARRAY['OPEN'::text, 'CLOSED'::text])",
2121
)).toBe(true);
2222
expect(normalizeCheckExpr("status = ANY (ARRAY['OPEN'::text, 'CLOSED'::text])"))
23-
.toBe("status in 'open', 'closed'");
23+
.toBe("status in 'open','closed'");
2424
// different member sets remain distinct after the fold
2525
expect(checkExprEquals(
2626
"status IN ('OPEN', 'CLOSED')",
@@ -49,6 +49,16 @@ describe("normalizeCheckExpr", () => {
4949
test("genuinely different expressions are not equal", () => {
5050
expect(checkExprEquals("col >= 0", "col >= 5")).toBe(false);
5151
});
52+
53+
test("separator commas normalize, but a comma INSIDE a literal stays meaningful", () => {
54+
// Separator commas (outside quotes) collapse, so a generated IN-list matches a
55+
// hand-written one regardless of spacing...
56+
expect(checkExprEquals("status IN ('a','b')", "status IN ('a', 'b')")).toBe(true);
57+
// ...but a comma inside a string/regex literal is real: two checks differing only
58+
// there must NOT compare equal, or a genuine regex/predicate change reads as clean.
59+
expect(checkExprEquals("label <> 'a, b'", "label <> 'a,b'")).toBe(false);
60+
expect(checkExprEquals("code ~ '^x{1, 3}$'", "code ~ '^x{1,3}$'")).toBe(false);
61+
});
5262
test("undefined is never equal", () => {
5363
expect(checkExprEquals(undefined, "x")).toBe(false);
5464
});

0 commit comments

Comments
 (0)