Order migration 0021 so it never drops a table with children - #192
Merged
Conversation
Applying 0021 to the remote database failed with "FOREIGN KEY constraint failed" while local succeeded. The migration opened with PRAGMA foreign_keys=OFF, which is a documented no-op inside a transaction - wrangler wraps each migration file in one, so it silently did nothing against D1. The tests ran statements outside a transaction, where the pragma works, which is precisely why they could not see the difference. PRAGMA defer_foreign_keys is not a substitute, which I verified before relying on it: every statement succeeds and PRAGMA foreign_key_check comes back clean, but COMMIT still fails, because DROP TABLE on a parent increments SQLite's deferred-violation counter once per child row and renaming the replacement into place never decrements it. SQLite's own 12-step ALTER procedure sidesteps this by demanding foreign_keys=OFF, the one thing unavailable here. The fix is ordering, not a pragma, so the pragma is removed rather than left in as decoration. extension_submissions is copied into a holding table and dropped before extensions is rebuilt, leaving extensions childless at the moment it is replaced; extension_revisions is created afterwards. 0021 now applies with foreign keys fully enforced. The developers rebuild is dropped for the same reason: developer_claims, developer_transfers and extensions all reference it, so it can never be dropped this way, and rebuilding all three children to replace a default nothing reads is not a trade worth making. created_at/updated_at keep the 1970 placeholder, with a note on why it stays. migrations.test.ts gains applyAllAsD1(), which runs the chain with foreign keys enforced and 0021 inside a transaction. Against 0021 as merged it reproduces the production failure exactly; against this one it passes. No data was changed by the failed run: it rolled back whole, 0021 is not recorded in d1_migrations, and the remote schema is still at 0020.
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
api | f772bc4 | Commit Preview URL Branch Preview URL |
Aug 08 2026, 03:12 PM |
Follow-up to the ordering fix. The reference check ran at the end of the file, where it could never fire: with foreign keys enforced the rebuilt table's own constraint rejects a dangling row during the copy, so the failure arrived as a bare "FOREIGN KEY constraint failed" and the named check was dead code. It only ever fired in tests, which ran with foreign keys off. Moved to the front with the other pre-flight checks, so it fires before anything is copied and names what is wrong. Its second half is dropped: extension_revisions.extension_id comes from a join against extensions and cannot dangle by construction. The test now runs this case through applyAllAsD1() rather than with foreign keys off, so it exercises the path that actually runs. Also trimmed the comments this change added - the pragma explanation, the holding-table note, the developers-default note and applyAllAsD1's header were all saying the same thing more than once.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
All reported issues were addressed
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Contributor
There was a problem hiding this comment.
0 issues found across 3 files (changes from recent commits).
Requires human review: Auto-approval blocked by 1 unresolved issue from previous reviews.
Re-trigger cubic
The test named for the drop-parent-with-children ordering did not actually exercise it. seedSubmissionFixture creates no submissions, so extension_submissions was empty and extensions had no children - and SQLite only raises on DROP TABLE when a child row actually references a deleted one. Reintroducing the old ordering left this test green. The evidence was already in front of me: when I mutation-checked the ordering earlier, three tests failed and this one was not among them. They fail because the SQL references a dropped table, not because of foreign key enforcement, so they would not have caught the production failure either. It now seeds a submission with extension_id set. Null is not enough - extension_id is what makes extensions a parent, and every submission in production happened to be null, which is why the remote apply failed on the developers drop rather than this one. Restoring the old ordering now fails this test.
Contributor
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Auto-approved: Fixes a broken migration that failed on remote by reordering table drops and adding a pre-check, with a test that replicates D1's foreign-key enforcement to prevent regression. No new behavior, rollout, or operational tradeoff.
Re-trigger cubic
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
1 participant
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 the failed remote apply of
0021from #190. No data was changed by that run; it rolled back whole,0021is not recorded ind1_migrations, and the remote schema is still at0020. I confirmed that against the production database before touching anything.