Skip to content

Commit 8ccc7c2

Browse files
dmealingclaude
andcommitted
fix(migrate): recreate dependent views around rebuilt tables (#243); FQN-resolve FK targets; refuse duplicate SQL names
- #243: a view over a table rebuilt by a CHECK/FK/enum-values change (or pulled into the D1 FK-cascade as a referrer) was stranded across the DROP/RENAME and failed at apply. The diff now drops/recreates dependent views for the full recreate-trigger set on sqlite/d1 (postgres unchanged), and the cascade owns the view drop/recreate in-order with no double-emit. - FK targets resolve by fully-qualified name (bare @references qualified with the referrer's package, per resolutionKey()); an ambiguous bare name no longer mis-binds a cross-package same-named entity. - Two metadata objects generating the same database name now fail at build time with ERR_DUPLICATE_SQL_NAME instead of an un-appliable migration. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TJRi8FtEW24z9HKGUL1xh8
1 parent dcc6694 commit 8ccc7c2

9 files changed

Lines changed: 490 additions & 35 deletions

File tree

CHANGELOG.md

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,35 @@ here. The format follows [Keep a Changelog](https://keepachangelog.com/), and
55
this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html)
66
(pre-1.0; MINOR bumps may introduce breaking changes with notice).
77

8+
## [Unreleased]
9+
10+
**npm-only**`migrate-ts` (schema migrations are TS-owned, ADR-0015); PyPI / NuGet /
11+
Maven Central are unchanged.
12+
13+
### Fixed — a projection/view over a rebuilt table is no longer stranded mid-migration (#243)
14+
15+
On SQLite and Cloudflare D1, a table is rebuilt via recreate-and-copy (DROP + RENAME) not
16+
only for column changes but also for CHECK / foreign-key / evolved `field.enum @values`
17+
changes — and, on D1, for any table pulled into the FK-cascade as a referrer. A view
18+
reading that table was left dangling across the rebuild, so the migration failed at apply
19+
time (`error in view …: no such table …`). The diff now drops a dependent view **before**
20+
the rebuild and recreates it **after** for the CHECK/FK/enum-values class too (previously
21+
only column-altering changes triggered it), and the D1 cascade drops/recreates every view
22+
over an affected table in the correct order — no double-emit. Postgres is unaffected
23+
(FK/CHECK changes there are `ALTER … ADD/DROP CONSTRAINT`, no table rebuild).
24+
25+
### Fixed — foreign-key targets resolve by fully-qualified name; duplicate generated SQL names are refused
26+
27+
The schema builder resolved an FK's target entity by **bare** name, so when two packages
28+
declared a same-named entity the last one loaded won an internal map and a cross-package FK
29+
could silently point at the **wrong** table. FK targets now resolve by fully-qualified name
30+
(a bare `@references` is qualified with the referrer's own package, mirroring the loader's
31+
`resolutionKey()` contract); an ambiguous bare name resolves to nothing rather than
32+
guessing. Separately, two distinct metadata objects that generate the **same** database
33+
name (schema-qualified — table/table, view/view, or table/view, across packages) now fail
34+
at build time with `ERR_DUPLICATE_SQL_NAME` naming both owners, instead of emitting an
35+
un-appliable migration the database would reject.
36+
837
## [0.20.8] — 2026-07-28
938

1039
**npm-only** — the changed code is all in `migrate-ts` (D1 is a TS-only dialect);

docs/features/migrations-and-drift.md

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -84,15 +84,10 @@ populated production database and re-converges — a follow-up `meta verify`/`me
8484
migrate` sees no drift. The cascade is built over the **union of the actual (live) and
8585
expected (target) schemas' foreign-key graphs**, so it also covers the case a
8686
target-schema-only check would miss: a single migration that both rebuilds a
87-
referenced table *and* drops the referencing foreign key in the same run.
88-
89-
One known exception: a table with a **dependent projection/view** that gets rebuilt
90-
by the cascade (or by any CHECK/FK/enum-values rebuild) may need that view
91-
hand-managed — the diff layer only auto-drops/recreates a dependent view for
92-
column-altering changes, not the CHECK/FK/enum-values class the cascade exists to
93-
handle. Tracked as [#243](https://github.com/metaobjectsdev/metaobjects/issues/243).
94-
The common case — no dependent view on a rebuilt table — applies cleanly as described
95-
above.
87+
referenced table *and* drops the referencing foreign key in the same run. A
88+
projection/view that reads a rebuilt table is dropped before the rebuild and recreated
89+
after — for the CHECK/FK/enum-values rebuild class as well as column changes — so a
90+
dependent view is never stranded mid-migration.
9691

9792
The one case still hand-written: a **multi-table foreign-key cycle** (table A
9893
references B references … references A, two or more tables). A cycle has no
Lines changed: 260 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,260 @@
1+
/**
2+
* #243 probe — a dependent VIEW over a table REBUILT by a non-column-altering change
3+
* (enum @values / CHECK), on SQLite and on the D1 cascade.
4+
*
5+
* Concern (from the #241 final review): Pass 2c injects a view drop/recreate only for
6+
* COLUMN-altering changes, so a CHECK/FK/enum-triggered rebuild (or a D1 cascade referrer
7+
* rebuild) of a table a view reads might break the view. This reproduces it on a real
8+
* engine to decide: real bug (fix) or not (close with proof). Runs against the built
9+
* @metaobjectsdev/migrate-ts (this package depends on the published/built API).
10+
*/
11+
import { describe, test, expect, beforeEach, afterEach } from "bun:test";
12+
import { mkdtempSync, rmSync } from "node:fs";
13+
import { tmpdir } from "node:os";
14+
import { join } from "node:path";
15+
import { Kysely, sql } from "kysely";
16+
import { LibsqlDialect, libsql } from "@libsql/kysely-libsql";
17+
import { buildExpectedSchema, diff, emit, introspectSqlite } from "@metaobjectsdev/migrate-ts";
18+
import { buildProjectionViews } from "@metaobjectsdev/codegen-ts";
19+
import { MetaDataLoader, InMemoryStringSource } from "@metaobjectsdev/metadata";
20+
21+
let tmpDir: string;
22+
let dbPath: string;
23+
let k: Kysely<Record<string, unknown>>;
24+
25+
beforeEach(() => {
26+
tmpDir = mkdtempSync(join(tmpdir(), "d1-243-"));
27+
dbPath = join(tmpDir, "t.db");
28+
k = new Kysely({ dialect: new LibsqlDialect({ url: `file:${dbPath}` }) });
29+
});
30+
afterEach(async () => { await k.destroy(); rmSync(tmpDir, { recursive: true, force: true }); });
31+
32+
function splitSql(text: string): string[] {
33+
return text.trim().split(";").map((s) => s.trim()).filter(Boolean);
34+
}
35+
async function applyRaw(text: string): Promise<void> {
36+
for (const stmt of splitSql(text)) await sql.raw(stmt).execute(k);
37+
}
38+
/** Apply statements inside ONE libSQL transaction with foreign_keys=ON — models remote D1. */
39+
async function applyInImplicitTxn(stmts: string[]): Promise<{ ok: boolean; error?: string }> {
40+
const client = libsql.createClient({ url: `file:${dbPath}` });
41+
await client.execute("PRAGMA foreign_keys = ON");
42+
const tx = await client.transaction("write");
43+
try {
44+
for (const s of stmts) await tx.execute(s);
45+
await tx.commit();
46+
return { ok: true };
47+
} catch (e) {
48+
try { await tx.rollback(); } catch { /* ignore */ }
49+
return { ok: false, error: e instanceof Error ? e.message : String(e) };
50+
} finally { client.close(); }
51+
}
52+
53+
// v1 / v2 differ only by Program.status enum @values (DRAFT,PUBLISHED) → (+ARCHIVED).
54+
// An enum @values change lowers to a CHECK change on SQLite → recreate-and-copy of
55+
// `programs`. The view v_program_summary READS programs.status (isPublished) + id.
56+
function meta(values: string[]): string {
57+
return JSON.stringify({ "metadata.root": { package: "acme", children: [
58+
{ "object.entity": { name: "Program", children: [
59+
{ "source.rdb": { "@table": "programs" } },
60+
{ "field.long": { name: "id" } },
61+
{ "field.enum": { name: "status", "@values": values, "@required": true } },
62+
{ "identity.primary": { name: "id", "@fields": "id", "@generation": "increment" } },
63+
] } },
64+
{ "object.projection": { name: "ProgramSummary", children: [
65+
{ "source.rdb": { "@kind": "view", "@table": "v_program_summary" } },
66+
{ "identity.primary": { name: "id", extends: "Program.id", "@fields": "id" } },
67+
{ "field.long": { name: "id", extends: "Program.id", children: [
68+
{ "origin.passthrough": { "@from": "Program.id" } } ] } },
69+
{ "field.boolean": { name: "isPublished", children: [
70+
{ "origin.computed": { "@expr": { op: "eq", left: { field: "status" }, right: { value: "PUBLISHED" } } } } ] } },
71+
] } },
72+
]}});
73+
}
74+
75+
async function build(values: string[], dialect: "sqlite" | "d1") {
76+
const root = (await new MetaDataLoader().load([new InMemoryStringSource(meta(values))])).root;
77+
const columnNamingStrategy = "literal" as const;
78+
return buildExpectedSchema(root, {
79+
dialect, columnNamingStrategy,
80+
views: buildProjectionViews(root, { dialect, columnNamingStrategy }),
81+
});
82+
}
83+
84+
describe("#243 — dependent view over an enum/CHECK-rebuilt table", () => {
85+
test("SQLite: enum @values change on a VIEWED column rebuilds the table; view survives + re-diff EMPTY", async () => {
86+
const expected1 = await build(["DRAFT", "PUBLISHED"], "sqlite");
87+
const actual0 = await introspectSqlite(k);
88+
const d0 = await diff({ expected: expected1, actual: actual0, dialect: "sqlite" });
89+
await applyRaw(emit(d0.changes, { dialect: "sqlite", expectedSchema: expected1,
90+
...(actual0.meta !== undefined && { actualMeta: actual0.meta }) }).up);
91+
92+
await sql.raw(`INSERT INTO "programs" ("id","status") VALUES (1,'PUBLISHED'),(2,'DRAFT')`).execute(k);
93+
const before = await sql.raw(`SELECT "id","isPublished" FROM "v_program_summary" ORDER BY "id"`).execute(k);
94+
expect(before.rows.length).toBe(2);
95+
96+
const expected2 = await build(["DRAFT", "PUBLISHED", "ARCHIVED"], "sqlite");
97+
const actual1 = await introspectSqlite(k);
98+
const d1diff = await diff({ expected: expected2, actual: actual1, dialect: "sqlite", allow: { dropCheck: true } });
99+
expect(d1diff.changes.length).toBeGreaterThan(0); // there IS a rebuild
100+
const up = emit(d1diff.changes, { dialect: "sqlite", expectedSchema: expected2,
101+
...(actual1.meta !== undefined && { actualMeta: actual1.meta }) }).up;
102+
await applyRaw(up);
103+
104+
const after = await sql.raw(`SELECT "id","isPublished" FROM "v_program_summary" ORDER BY "id"`).execute(k);
105+
expect(after.rows.length).toBe(2);
106+
expect((after.rows as Array<{ id: number; isPublished: number }>).map((r) => r.isPublished)).toEqual([1, 0]);
107+
108+
const followup = await diff({ expected: expected2, actual: await introspectSqlite(k), dialect: "sqlite" });
109+
expect(followup.changes).toEqual([]);
110+
});
111+
112+
test("D1 cascade: enum @values change on a VIEWED table applies in one-txn; view survives + re-diff EMPTY", async () => {
113+
const expected1 = await build(["DRAFT", "PUBLISHED"], "d1");
114+
const actual0 = await introspectSqlite(k);
115+
const d0 = await diff({ expected: expected1, actual: actual0, dialect: "d1" });
116+
await applyRaw(emit(d0.changes, { dialect: "d1", expectedSchema: expected1, actualSchema: actual0,
117+
...(actual0.meta !== undefined && { actualMeta: actual0.meta }) }).up);
118+
await sql.raw(`INSERT INTO "programs" ("id","status") VALUES (1,'PUBLISHED'),(2,'DRAFT')`).execute(k);
119+
120+
const expected2 = await build(["DRAFT", "PUBLISHED", "ARCHIVED"], "d1");
121+
const actual1 = await introspectSqlite(k);
122+
const d1diff = await diff({ expected: expected2, actual: actual1, dialect: "d1", allow: { dropCheck: true } });
123+
const up = emit(d1diff.changes, { dialect: "d1", expectedSchema: expected2, actualSchema: actual1,
124+
...(actual1.meta !== undefined && { actualMeta: actual1.meta }) }).up;
125+
const res = await applyInImplicitTxn(splitSql(up));
126+
expect(res).toEqual({ ok: true });
127+
128+
const after = await sql.raw(`SELECT "id","isPublished" FROM "v_program_summary" ORDER BY "id"`).execute(k);
129+
expect(after.rows.length).toBe(2);
130+
131+
const followup = await diff({ expected: expected2, actual: await introspectSqlite(k), dialect: "d1" });
132+
expect(followup.changes).toEqual([]);
133+
});
134+
135+
// Case 2: a view over a table pulled into the D1 cascade ONLY as an FK referrer (the
136+
// referrer itself is unchanged). Rebuild the PARENT via an enum change → the cascade
137+
// rebuilds the child too → a view on the child must survive.
138+
function metaChild(values: string[]): string {
139+
return JSON.stringify({ "metadata.root": { package: "acme", children: [
140+
{ "object.entity": { name: "Program", children: [
141+
{ "source.rdb": { "@table": "programs" } },
142+
{ "field.long": { name: "id" } },
143+
{ "field.enum": { name: "status", "@values": values, "@required": true } },
144+
{ "identity.primary": { name: "id", "@fields": "id", "@generation": "increment" } },
145+
] } },
146+
{ "object.entity": { name: "Note", children: [
147+
{ "source.rdb": { "@table": "notes" } },
148+
{ "field.long": { name: "id" } },
149+
{ "field.long": { name: "programId", "@required": true } },
150+
{ "field.string": { name: "body", "@required": true } },
151+
{ "identity.primary": { name: "id", "@fields": "id", "@generation": "increment" } },
152+
{ "identity.reference": { name: "ref_program", "@fields": "programId", "@references": "Program" } },
153+
] } },
154+
{ "object.projection": { name: "NoteSummary", children: [
155+
{ "source.rdb": { "@kind": "view", "@table": "v_note" } },
156+
{ "identity.primary": { name: "id", extends: "Note.id", "@fields": "id" } },
157+
{ "field.long": { name: "id", extends: "Note.id", children: [
158+
{ "origin.passthrough": { "@from": "Note.id" } } ] } },
159+
{ "field.string": { name: "body", extends: "Note.body", children: [
160+
{ "origin.passthrough": { "@from": "Note.body" } } ] } },
161+
] } },
162+
]}});
163+
}
164+
async function buildChild(values: string[]) {
165+
const root = (await new MetaDataLoader().load([new InMemoryStringSource(metaChild(values))])).root;
166+
const columnNamingStrategy = "literal" as const;
167+
return buildExpectedSchema(root, {
168+
dialect: "d1", columnNamingStrategy,
169+
views: buildProjectionViews(root, { dialect: "d1", columnNamingStrategy }),
170+
});
171+
}
172+
173+
test("D1 cascade referrer: view on a child pulled in via FK survives the parent rebuild + re-diff EMPTY", async () => {
174+
const expected1 = await buildChild(["DRAFT", "PUBLISHED"]);
175+
const actual0 = await introspectSqlite(k);
176+
const d0 = await diff({ expected: expected1, actual: actual0, dialect: "d1" });
177+
await applyRaw(emit(d0.changes, { dialect: "d1", expectedSchema: expected1, actualSchema: actual0,
178+
...(actual0.meta !== undefined && { actualMeta: actual0.meta }) }).up);
179+
await sql.raw(`INSERT INTO "programs" ("id","status") VALUES (1,'PUBLISHED')`).execute(k);
180+
await sql.raw(`INSERT INTO "notes" ("id","programId","body") VALUES (1,1,'hello')`).execute(k);
181+
182+
const expected2 = await buildChild(["DRAFT", "PUBLISHED", "ARCHIVED"]);
183+
const actual1 = await introspectSqlite(k);
184+
const d1diff = await diff({ expected: expected2, actual: actual1, dialect: "d1", allow: { dropCheck: true } });
185+
const up = emit(d1diff.changes, { dialect: "d1", expectedSchema: expected2, actualSchema: actual1,
186+
...(actual1.meta !== undefined && { actualMeta: actual1.meta }) }).up;
187+
const res = await applyInImplicitTxn(splitSql(up));
188+
expect(res).toEqual({ ok: true });
189+
190+
const after = await sql.raw(`SELECT "id","body" FROM "v_note" ORDER BY "id"`).execute(k);
191+
expect(after.rows.length).toBe(1);
192+
193+
const followup = await diff({ expected: expected2, actual: await introspectSqlite(k), dialect: "d1" });
194+
expect(followup.changes).toEqual([]);
195+
});
196+
197+
// Case 3: cascade fires (parent referenced by a child) AND a view is on the DIRECTLY
198+
// changed parent — so the diff DOES inject a drop-view/create-view that must be
199+
// suppressed from `rest` and emitted by the cascade in the correct order instead.
200+
function metaParentView(values: string[]): string {
201+
return JSON.stringify({ "metadata.root": { package: "acme", children: [
202+
{ "object.entity": { name: "Program", children: [
203+
{ "source.rdb": { "@table": "programs" } },
204+
{ "field.long": { name: "id" } },
205+
{ "field.enum": { name: "status", "@values": values, "@required": true } },
206+
{ "identity.primary": { name: "id", "@fields": "id", "@generation": "increment" } },
207+
] } },
208+
{ "object.entity": { name: "Note", children: [
209+
{ "source.rdb": { "@table": "notes" } },
210+
{ "field.long": { name: "id" } },
211+
{ "field.long": { name: "programId", "@required": true } },
212+
{ "identity.primary": { name: "id", "@fields": "id", "@generation": "increment" } },
213+
{ "identity.reference": { name: "ref_program", "@fields": "programId", "@references": "Program" } },
214+
] } },
215+
{ "object.projection": { name: "ProgramView", children: [
216+
{ "source.rdb": { "@kind": "view", "@table": "v_program" } },
217+
{ "identity.primary": { name: "id", extends: "Program.id", "@fields": "id" } },
218+
{ "field.long": { name: "id", extends: "Program.id", children: [
219+
{ "origin.passthrough": { "@from": "Program.id" } } ] } },
220+
{ "field.boolean": { name: "isPublished", children: [
221+
{ "origin.computed": { "@expr": { op: "eq", left: { field: "status" }, right: { value: "PUBLISHED" } } } } ] } },
222+
] } },
223+
]}});
224+
}
225+
async function buildParentView(values: string[]) {
226+
const root = (await new MetaDataLoader().load([new InMemoryStringSource(metaParentView(values))])).root;
227+
const columnNamingStrategy = "literal" as const;
228+
return buildExpectedSchema(root, {
229+
dialect: "d1", columnNamingStrategy,
230+
views: buildProjectionViews(root, { dialect: "d1", columnNamingStrategy }),
231+
});
232+
}
233+
234+
test("D1 cascade + view on the DIRECTLY-changed parent: no double-emit, view survives + re-diff EMPTY", async () => {
235+
const expected1 = await buildParentView(["DRAFT", "PUBLISHED"]);
236+
const actual0 = await introspectSqlite(k);
237+
const d0 = await diff({ expected: expected1, actual: actual0, dialect: "d1" });
238+
await applyRaw(emit(d0.changes, { dialect: "d1", expectedSchema: expected1, actualSchema: actual0,
239+
...(actual0.meta !== undefined && { actualMeta: actual0.meta }) }).up);
240+
await sql.raw(`INSERT INTO "programs" ("id","status") VALUES (1,'PUBLISHED')`).execute(k);
241+
await sql.raw(`INSERT INTO "notes" ("id","programId") VALUES (1,1)`).execute(k);
242+
243+
const expected2 = await buildParentView(["DRAFT", "PUBLISHED", "ARCHIVED"]);
244+
const actual1 = await introspectSqlite(k);
245+
const d1diff = await diff({ expected: expected2, actual: actual1, dialect: "d1", allow: { dropCheck: true } });
246+
const up = emit(d1diff.changes, { dialect: "d1", expectedSchema: expected2, actualSchema: actual1,
247+
...(actual1.meta !== undefined && { actualMeta: actual1.meta }) }).up;
248+
// the view is dropped/created exactly once (cascade owns it; not double-emitted from rest)
249+
expect(up.match(/DROP VIEW IF EXISTS "v_program"/g)?.length).toBe(1);
250+
expect(up.match(/CREATE VIEW "v_program"/g)?.length).toBe(1);
251+
const res = await applyInImplicitTxn(splitSql(up));
252+
expect(res).toEqual({ ok: true });
253+
254+
const after = await sql.raw(`SELECT "id","isPublished" FROM "v_program" ORDER BY "id"`).execute(k);
255+
expect(after.rows.length).toBe(1);
256+
257+
const followup = await diff({ expected: expected2, actual: await introspectSqlite(k), dialect: "d1" });
258+
expect(followup.changes).toEqual([]);
259+
});
260+
});

0 commit comments

Comments
 (0)