Repository navigation
Conversation
|
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 |
|
Thanks! #121 has your commits unchanged plus a migration fuzzer (random schema pairs, run on seeded rows, checked against a fresh create). It found two bugs. First, a rebuild that keeps no column ( |
* feat(sqlite): generate writes a first SQLite migration and snapshot
* feat(sqlite): add columns with ALTER TABLE ADD COLUMN
* feat(sqlite): rebuild tables for changes ALTER TABLE cannot make
* feat(sqlite): rename tables and columns from confirm_rename hints
* feat(sqlite): adopt a drizzle-kit 1.0 folder with generate --baseline --from-drizzle
* test(sqlite): apply generated migrations to a local D1 database
* fix(migrate): refuse SQLite folders until the runner has a SQLite ledger
* docs(kit): name SQLite in the CLI help
* docs(sqlite): document generated SQLite migrations and drizzle-kit adoption
* docs(sqlite): explain the SQLite diff in its header
* fix(sqlite): migrations keep rows, never drop a table's last column
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>
* test(fuzz): migrations between random SQLite schemas reach the fresh 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>
* test(fuzz): migrations on D1 and with renames; drizzle-kit adoption round 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>
* fix(sqlite): confirm type changes in a rebuild; rename by case without 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>
* test(fuzz): the D1 migration engine retries transport errors only
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Taras Kornichuk <dread9ko@gmail.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Depends on #50 (stacked; the diff includes #49 and #50 until they merge). Refs #48.
Changes
SnapshotandSchemaDialectgainsqlite(SqliteSnapshot).AnySchemaEntityincludes the SQLite entities, so the branch graph,checkand snapshot hashing work for SQLite folders unchanged.effect-orm generatewithdialect: "sqlite"diffsSQLite.tabledefinitions against the folder and writes<timestamp>_<name>/migration.sqlandsnapshot.json. The SQL is split by--> statement-breakpoint, each statement under a-- <label>comment. SQLite never gets amigration.json.diffSqliteSchemasandSqliteMigrationOp(/schema):CREATE TABLE,DROP TABLE(needsconfirm_data_loss).ALTER TABLE ADD COLUMNwhen SQLite allows it: nullable or a literal default, not in the key or a UNIQUE constraint.ALTER TABLE DROP COLUMN(needsconfirm_data_loss) unless the column is in the key or a UNIQUE constraint.rebuild_tableop:PRAGMA foreign_keys=OFF,CREATE TABLE "__new_<t>",INSERT ... SELECTof the shared columns,DROP TABLE,ALTER TABLE ... RENAME TO,PRAGMA foreign_keys=ON, then the table's indexes again. This is drizzle-kit's form of SQLite's documented procedure.DROP INDEX/CREATE [UNIQUE] INDEX.ALTER TABLE ... RENAME TOandRENAME COLUMNonly from aconfirm_renamehint.Hintacceptskind: "table" | "column"next to"enum_value". A hint that does not fit both schemas is reported as unsupported.generate --baseline --from-drizzlereads a drizzle-kit 1.0 SQLitesnapshot.json(version 7) with the newfromDrizzleSqliteSnapshot: tables, columns, keys, indexes, UNIQUE constraints. Anything else (foreign keys, checks, views, generated or AUTOINCREMENT columns, partial indexes) is listed and nothing is written.dialectOfnow reads the dialect a drizzle-kit snapshot names, instead of assuming Postgres./migraterun,status,verify,baselineandresolveStepfail with aMigrateSourceErrorfor a SQLite folder. Without this they would use the ClickHouse ledger.docs/migrations.md, updates todocs/sqlite.md,docs/reference.md, the README lines and the CLI help, and a minor changeset.Validation
bun install --frozen-lockfile,bun run build,bun run typecheck: ok.bun run testwith a local ClickHouse 26.8.2.7 in Docker: 65 files, 1075 tests passed, 0 skipped. Doc citations ok (26 files), export catalog ok (287 /clickhouse, 205 /postgres, 167 /sqlite), doc examples ok (26 files).bun scripts/check-package.ts: ESM and CommonJS consumers ok; the ESM consumer diffs and snapshots a SQLite schema from the tarball.src/kit/sqlite-generate.test.ts(16 tests) runsgeneratein a temporary folder. For each change it applies the migrations in order to:memory:and comparesPRAGMA table_info,index_listandindex_xinfowith a fresh create of the final schema. Seeded rows must survive. Changes covered: first create, add column, drop column, type, nullability, default, UNIQUE, expression-default column, primary key, index add/drop, table drop, a column a rebuild leaves out, table and column rename.src/kit/__fixtures__/drizzle-sqliteis a folder drizzle-kit 1.0.0-rc.5 generated forshort_linkand a second table (composite key, defaults, a UNIQUE column). Its source schema is next to it. Baseline, thengeneratefrom the equalSQLite.tabledefinitions: no change. One changed index then produces exactly its drop and create.Scope and limits
verify(U4);/migraterefuses SQLite folders on purpose.meta/_journal.json) is not read; Alchemy refuses it too and points todrizzle-kit up.PRAGMA foreign_keys=OFF/ONis a no-op inside a transaction. effect-orm models no foreign keys yet, so the pragmas only matter for keys created elsewhere.Decisions to review
NOT NULLand recordsnotNull: false, so SQLite accepts a NULL key. The importer reads a key column as not null, which is what drizzle-kit means and whatSQLite.tablewrites. A drizzle-created table then matches with no rebuild, but still accepts a NULL key until it is rebuilt for another change. A futureverifymust treat this case as known.confirm_renamefor a column takes<new table name>.<old column>, so it composes with a table rename in the same migration.migration.sqlis a comment only, as for Postgres. Alchemy sends it in one batch with its own bookkeeping statement.Need help on this PR? Tag
@codesmith-botwith what you need.