Skip to content

Commit a235d2b

Browse files
dmealingclaude
dmealing
and
claude
committed
docs(#226): design — detect-and-refuse D1 referenced-table rebuilds
Design spec for the D1 migrate FK bug (#226): rebuilding a referenced table on remote D1 fails silently against production because PRAGMA foreign_keys=OFF is a no-op inside D1's implicit transaction. Chosen approach is detect-and-refuse (loud generation-time failure) over the prototyped-but-disproportionate cascade auto-fix. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 3905ca9 commit a235d2b

1 file changed

Lines changed: 198 additions & 0 deletions

File tree

‎docs/superpowers/specs/2026-07-27-d1-referenced-table-rebuild-refuse-design.md‎

Lines changed: 198 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,198 @@
1+
# D1 referenced-table rebuild — detect-and-refuse (#226)
2+
3+
- **Date:** 2026-07-27
4+
- **Status:** Design approved; ready for implementation plan
5+
- **Issue:** [#226](https://github.com/metaobjectsdev/metaobjects/issues/226) — `migrate --dialect d1`: the SQLite table-rebuild's `PRAGMA foreign_keys OFF` is a no-op on remote D1, so a parent-table rebuild fails with `FOREIGN KEY constraint failed`
6+
- **Surface:** `@metaobjectsdev/migrate-ts` (D1 emitter) + `@metaobjectsdev/cli`. TS-only — D1 is a TS-only dialect; no cross-port impact.
7+
8+
## Problem
9+
10+
On Cloudflare **remote** D1, a generated migration that rebuilds a table which is
11+
referenced by another table's foreign key fails at apply time with
12+
`FOREIGN KEY constraint failed`, aborting the migration. The failure is **silent
13+
until a table holds rows** (an empty DB orphans nothing), so it typically first
14+
appears against a populated production database, after the migration has been
15+
generated, reviewed, and committed.
16+
17+
### Why it happens
18+
19+
The D1 emitter delegates the whole rebuild to the SQLite emitter
20+
(`emit/d1.ts` → `renderSqlite`), inheriting SQLite's 12-step table rebuild
21+
(`emit/sqlite.ts`, `renderRecreate`):
22+
23+
```sql
24+
PRAGMA foreign_keys = OFF;
25+
BEGIN TRANSACTION;
26+
CREATE TABLE "__new_x" (...);
27+
INSERT INTO "__new_x" (...) SELECT ... FROM "x";
28+
DROP TABLE "x";
29+
ALTER TABLE "__new_x" RENAME TO "x";
30+
COMMIT;
31+
PRAGMA foreign_keys = ON;
32+
PRAGMA foreign_key_check;
33+
```
34+
35+
The D1 safety pass (`emit/d1-safety-pass.ts`) strips `BEGIN`/`COMMIT` but passes the
36+
`PRAGMA foreign_keys` statements through verbatim. On remote D1 the whole batch runs
37+
inside D1's **implicit transaction**, and SQLite **ignores `PRAGMA foreign_keys`
38+
while a transaction is open** — so the `OFF` never takes effect. `DROP TABLE "x"`
39+
then runs with FK enforcement live; when `x` is a **referenced parent** with existing
40+
child rows, the drop is rejected.
41+
42+
A rebuild is triggered by any recreate-triggering change on the referenced table:
43+
adding/removing a `CHECK`, changing a column's type / nullability / default, adding or
44+
removing a foreign key, or a `field.enum @values` change (which lowers to a `CHECK`
45+
change). See the recreate-triggering kinds in `emit/sqlite.ts` (`add-check`,
46+
`drop-check`, `change-column-type`, `change-column-nullable`, `change-column-default`,
47+
`add-fk`, `drop-fk`).
48+
49+
### Why it is D1-specific
50+
51+
Local SQLite is correct: its emitted `PRAGMA foreign_keys = OFF` precedes `BEGIN`, so
52+
it takes effect before the transaction opens. Postgres does not use the rebuild
53+
recipe. Only D1's stripped-`BEGIN` / single-implicit-transaction apply model exposes
54+
the defect. Remote is applied via `wrangler d1 migrations apply <binding> --remote`,
55+
which hands the whole migration file to D1 as one implicit transaction.
56+
57+
## Scope decision: detect-and-refuse, not auto-fix
58+
59+
The auto-fix ("cascade" — rebuild the referencing children without their FK, rebuild
60+
the parent, restore the children's FK) was prototyped and **works**, but it is
61+
substantial, engine-quirk-dependent machinery (reverse FK-graph, two strict ordering
62+
rules, self-referential special-casing, and a genuine dead-end for multi-table
63+
cycles) for a narrow case on a single niche dialect. See
64+
[Appendix A](#appendix-a--prototype-findings) for the prototype evidence.
65+
66+
The bug's actually-dangerous property is that it fails **silently, against
67+
production**. Detect-and-refuse removes that property completely at a fraction of the
68+
cost: turn a silent runtime failure into a **loud generation-time failure** with a
69+
clear workaround. This matches the issue's own guidance ("a loud generation-time
70+
failure is far better than a runtime failure against production") and the D1 safety
71+
pass's existing philosophy of refusing statement sequences it knows cannot apply
72+
(`ATTACH`/`DETACH`/`VACUUM`).
73+
74+
The cascade auto-fix is recorded as a possible future enhancement (a follow-up issue),
75+
not built here.
76+
77+
## Design
78+
79+
### Detection
80+
81+
In the D1 emit path, before returning the emitted SQL, compute:
82+
83+
1. **The rebuilt-table set** — the tables that will go through `renderRecreate`,
84+
derived from `changes` by selecting the recreate-triggering kinds and grouping by
85+
table (the same partition `renderSqlite` already performs internally; the D1 path
86+
computes it from the structured `changes`, not by parsing emitted SQL).
87+
2. **Referenced-ness** — a rebuilt table `T` is *refused* when any foreign key in the
88+
schema targets `T` (`FkDescriptor.refTable === T`), whether from another table or
89+
from `T` itself (a self-referential FK hits the same wall — the temp table
90+
references the original name, so the `DROP` of the referenced original still
91+
fails).
92+
93+
If any rebuilt table is referenced, **throw** a dedicated, structured error and emit
94+
nothing. If none is, the D1 output is produced exactly as today (`renderSqlite` +
95+
`applyD1SafetyPass`) — **byte-identical** for every migration that does not rebuild a
96+
referenced table.
97+
98+
The check is a direct scan of the schema's foreign keys — no reverse graph, no
99+
topological sort, no dependency-closure.
100+
101+
**Detection data source (over-refuse bias).** A *missed* refusal reintroduces the
102+
exact silent-prod-failure this change exists to kill, so detection errs toward
103+
**over-refusing**: a rebuilt table is refused if it is referenced in the **target
104+
(expected) schema** *or* by a currently-existing child in the actual schema. The
105+
implementation plan pins the exact source against the diff inputs available to the D1
106+
emitter (`expectedSchema`, and the actual-schema FK information available at migrate
107+
time); when the available data is ambiguous, refuse. Over-refusing costs a D1 user a
108+
hand-written migration in a rare case; under-refusing costs a silent production
109+
failure.
110+
111+
### The error
112+
113+
A dedicated error type (sibling to `D1UnsupportedStatementError`) carrying the
114+
refused table and its referencing table(s), with a message that names the cause and
115+
the workaround. Example rendered message:
116+
117+
```
118+
Cannot rebuild table "parent" on Cloudflare D1 — it is referenced by a foreign key
119+
from "child". D1 applies migrations inside an implicit transaction where
120+
`PRAGMA foreign_keys = OFF` is a no-op, so dropping the referenced table during the
121+
rebuild fails with "FOREIGN KEY constraint failed".
122+
123+
The rebuild was triggered by a change to "parent" (a CHECK, column type/nullability/
124+
default, foreign key, or enum-values change). To apply it on D1, hand-write this
125+
migration: rebuild "child" to temporarily drop its foreign key, rebuild "parent",
126+
then rebuild "child" to restore the foreign key. Or make the change on an
127+
unreferenced table.
128+
```
129+
130+
### Where it lives
131+
132+
- Detection + the throw live in the D1 emitter (`emit/d1.ts`), operating on the
133+
structured `changes` + schema inputs it already receives — not on the emitted SQL
134+
string, and not in the safety pass (which is string-level and lacks FK-graph
135+
awareness).
136+
- A small helper enumerates the rebuilt-table set from `changes` and tests
137+
referenced-ness against the schema's `FkDescriptor`s.
138+
- No change to `emit/sqlite.ts` behavior, `emit/postgres.ts`, or the local-SQLite /
139+
Postgres paths. If a shared "which kinds trigger a recreate" predicate is convenient,
140+
it is extracted read-only from the existing SQLite logic without changing that
141+
logic.
142+
143+
### CLI behavior
144+
145+
`meta migrate --dialect d1` surfaces the thrown error as a clear generation-time
146+
failure and writes no migration file (the existing emit-error path). `meta verify
147+
--dialect d1` is unaffected — it reports drift (that the change is needed) but does not
148+
emit, so it continues to work; the user learns the change exists from `verify` and
149+
learns it must be hand-written from `migrate`.
150+
151+
## Testing
152+
153+
- **Unit (`emit/d1`):**
154+
- throws on a rebuild of a table referenced by another table's FK;
155+
- throws on a rebuild of a self-referentially-referenced table;
156+
- does **not** throw on a rebuild of an unreferenced (leaf) table;
157+
- does **not** throw on non-rebuild changes (`add-column`, `create-table`,
158+
`add-index`, …);
159+
- the thrown error names the refused table and its referencer.
160+
- **Byte-identical guard:** for a migration with no referenced-table rebuild, the D1
161+
output equals the pre-change output (no-churn snapshot).
162+
- **Integration (libSQL, real engine — mirrors `test/integration/sqlite-fk-convergence.test.ts`):**
163+
reproduce remote D1 by wrapping the emitted batch in **one explicit transaction**
164+
with `foreign_keys = ON` at the connection level (SQLite ignores `PRAGMA
165+
foreign_keys` inside a transaction — identical to remote D1's implicit transaction).
166+
- documents the defect: the *old* naive D1 output for a referenced-parent rebuild
167+
fails with `FOREIGN KEY constraint failed` under that model;
168+
- proves no over-refusal: a *leaf*-table rebuild's D1 output applies cleanly under
169+
the same model, data is intact, and a re-diff is **EMPTY** (the migrate-engine
170+
doctrine: emit → apply to a real engine → re-diff empty).
171+
172+
## Non-goals
173+
174+
- Auto-migrating a referenced-table rebuild on D1 (the cascade) — deferred to a
175+
follow-up issue.
176+
- Any change to local SQLite or Postgres emitters.
177+
- Multi-table foreign-key cycles — subsumed by the refuse (they are referenced tables).
178+
179+
## Appendix A — prototype findings
180+
181+
Throwaway libSQL prototypes (single connection, the rebuild wrapped in one explicit
182+
transaction with `foreign_keys = ON`, faithfully modelling remote D1) established:
183+
184+
1. **The defect reproduces** deterministically: a referenced-parent rebuild fails at
185+
`DROP TABLE <parent>` with `FOREIGN KEY constraint failed`; `PRAGMA foreign_keys =
186+
OFF` is confirmed a no-op inside the transaction.
187+
2. **`PRAGMA defer_foreign_keys = ON` does not rescue a parent rebuild** (it defers
188+
row-level checks, not the drop-of-a-referenced-table guard) — though it does rescue
189+
a pure child rebuild. There is no cheap PRAGMA-only fix.
190+
3. **A cascade does work** (rebuild referencing children without their FK → rebuild
191+
parent → restore), validated for single-parent, transitive (`g→c→p`), multi-child,
192+
and self-referential graphs with data intact and FKs re-enforced — but only via
193+
temp-name FK rewriting plus two strict ordering rules (referrers-first `DROP`,
194+
parents-first `RENAME`). Multi-table cycles have no reliable atomic sequence.
195+
196+
These findings are the justification for choosing detect-and-refuse: the auto-fix is
197+
real but disproportionate, and the same libSQL-in-one-transaction technique becomes
198+
the integration gate above.

0 commit comments

Comments
 (0)