Repository navigation
feat(sqlite): generate migrations (#51, fuzzed) - #121
Merged
Merged
Conversation
Found by fuzzing migrations between random schemas on seeded rows: - A rebuild that keeps no column (every old column dropped, new ones added) wrote no INSERT, so it deleted every row though only the columns' data was confirmed lost. It now copies the rowids, and each row takes the new columns' defaults. - Replacing every column of a table dropped them before adding the new ones, and SQLite cannot drop a table's last column. Column adds now run before column drops. - A column made NOT NULL that has a default failed the rebuild over existing NULLs: INSERT ... SELECT passes an explicit NULL, which does not take the default. The copy is COALESCE(column, default) now. docs/migrations.md: a rebuild under D1 or any transaction fires ON DELETE CASCADE in tables that reference the rebuilt one (PRAGMA foreign_keys=OFF does nothing there), a rebuild renumbers rowids, and ADD COLUMN appends. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…schema and keep rows Two random schemas over a shared pool of table and column names: new, dropped and changed tables, columns, types, nullability, literal and expression defaults, keys, UNIQUE columns and indexes. The migration diffSqliteSchemas writes runs inside a transaction on rows that fit both schemas. The PRAGMA shape must equal a fresh create of the new schema (column order aside) and every row must survive, holding its values or the new column's default. Pins the three counterexamples fixed in the previous commit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 59 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…ound trip - The migration property runs on D1 too: each migration as one batch on a local workerd binding, as Alchemy's D1 resource applies it, compared with a fresh create on a second D1 database. - Inputs rename a shared table and one of its kept columns through confirm_rename hints (about half of all inputs do), so renames combine with every other change and with rebuilds. - A drizzle-kit 1.0 snapshot of any random schema (written as drizzle-kit writes it, key columns `notNull: false` included) imports with nothing left to generate. - Changesets no longer say SQLite DDL or migrations are not implemented. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts: # tests/fuzz/regressions.ts
…t a hint
From an independent review:
- A rebuild that changes a kept column's type converts its values by
SQLite's type affinity ('007' to 7, '1e3' to 1000) with no prompt. It
now needs confirm_data_loss for that column, like a drop.
- A column or table renamed by case only (Url to url) read as a drop and
an add, which then collided: SQLite names ignore case. The diff now
renames it, with no hint, since nothing is lost. SQLite refuses a
case-only table rename in one step, so it goes through a temporary name.
- The migration fuzzer renames by case without hints too, and confirms
drops and type changes under old and new names.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Makisuo
added this pull request to stack #124
October 25, 2026 19:16
This was referenced Oct 25, 2026
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
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.
This is #51 by @tao-io, with review fixes and a migration fuzzer on top. Their commits are unchanged. GitHub doesn't let maintainers push to an organization-owned fork, so the fixes couldn't go onto #51 directly. It's stacked on #120 (DDL), which is stacked on #119 (query dialects). Refs #48.
What #51 adds
See #51:
effect-orm generatefordialect: "sqlite",diffSqliteSchemas(ALTER TABLE where SQLite allows it, otherwise a table rebuild), rename hints, drizzle-kit 1.0 SQLite snapshot import, and/migraterefusing SQLite folders.What this adds
A migration fuzzer (
tests/fuzz/migrations.fuzz.test.ts). It generates two random schemas over shared table and column names, covering new, dropped and changed tables, columns, types, nullability, literal and expression defaults, keys, UNIQUE columns and indexes. Then it:20,000 runs pass. It found two bugs within 100 runs:
{c2}→{c0})INSERT, so every row was deleted, though only the column data was confirmed lostrowid; rows keep, new columns take defaults{c1, c2}→{c3})Also, from review:
INSERT … SELECTpasses an explicit NULL, which doesn't take the default, so the copy is nowCOALESCE(col, default).docs/migrations.mdnow has three additions:DROP TABLEfiresON DELETE CASCADEin tables that reference the rebuilt one (PRAGMA foreign_keys=OFFdoes nothing there; reproduced in review);ADD COLUMNappends, so column order can differ from a fresh create.The three counterexamples are pinned in
tests/fuzz/regressions.ts. With the fixes reverted, the pins fail.Verification
bun run typecheck: pass.bun run testwith ClickHouse 26.8.2.7: 68 files, 1,248 tests passed, none skipped. Doc citations, export catalogs and doc examples ok.bun scripts/check-package.ts: ok.Left as-is (from review)
a→bwhile creating a newa) is reported as unsupported.Second round (after review)
confirm_data_lossfor it. The copy converts values by SQLite's type affinity ('007'→7,'1e3'→1000), which happened silently before.Url→url,links→Links). SQLite compares names without case, so before this the change read as a drop plus an add that collided. SQLite refuses a case-only table rename in one step, so it goes through a temporary name.confirm_renamehints (about half of all inputs), or by case only with no hint.FUZZ_SCALE=20.check-packageis ok.🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need.