From 06d27a57db17aa8ca78763cc6fa3df008d3236ff Mon Sep 17 00:00:00 2001 From: Thomas Hart Date: Fri, 4 Sep 2026 20:39:09 +0000 Subject: [PATCH 1/5] feat: Add declarative schema DSL and reversible migrations Schema is a value you can diff. Up is desired minus current, down is the reverse diff so drops still recreate tables. The runner journals zero-padded ids and refuses holes in the applied prefix. --- README.md | 20 ++++ src/ddl.ts | 197 ++++++++++++++++++++++++++++++++++++++++ src/index.ts | 2 + src/migrate.ts | 158 ++++++++++++++++++++++++++++++++ test/schema-dsl.test.ts | 120 ++++++++++++++++++++++++ 5 files changed, 497 insertions(+) create mode 100644 src/ddl.ts create mode 100644 src/migrate.ts create mode 100644 test/schema-dsl.test.ts diff --git a/README.md b/README.md index 32f92c2..dabf1f3 100644 --- a/README.md +++ b/README.md @@ -16,10 +16,17 @@ Most ORM tutorials stop at `findMany`. This repo goes the other way. It builds t - Phantom types so column refs carry their TypeScript value type - Immutable query values (each clause returns a new builder) - Empty `IN ()` as a false predicate, because that list is not valid SQL +- Declarative schema as a value: tables, columns, indexes, foreign keys +- Structural schema diff (desired vs current) and reverse-diff down migrations +- Topological sort of DDL from foreign-key edges, including self-references +- Migration journal with a zero-padded id prefix invariant +- Dialect type mapping (`BOOLEAN`/`TIMESTAMPTZ` vs SQLite `INTEGER`/`TEXT`) +- Expand/contract: in-place column ALTER is rejected; drop and add instead ## What's implemented - Type-safe SELECT/WHERE/JOIN query builder with prepared-statement binding +- Declarative schema DSL + migration runner with up/down and a diff generator ## Usage @@ -48,6 +55,19 @@ const compiled = from(orders) `compiled.sql` is parameterized. `compiled.params` is `[true, 500, 20]`. +Schema changes are a value you can diff. `diffSchema(from, to)` is up; `diffSchema(to, from)` is down, so a drop still knows how to recreate the table. + +```ts +import { col, defineSchema, diffSchema, Migrator, table } from 'orm-workbench' + +const from = defineSchema({}) +const to = defineSchema({ + users: table({ id: col.integer().primaryKey(), email: col.text().unique() }).index('users_email_idx', ['email']), + orders: table({ id: col.integer().primaryKey(), userId: col.integer().references('users', 'id', { onDelete: 'cascade' }) }), +}) +new Migrator(session, 'sqlite').migrateUp([{ id: '001_init', up: diffSchema(from, to), down: diffSchema(to, from) }]) +``` + ```bash pnpm install pnpm run typecheck diff --git a/src/ddl.ts b/src/ddl.ts new file mode 100644 index 0000000..0446893 --- /dev/null +++ b/src/ddl.ts @@ -0,0 +1,197 @@ +import type { Dialect } from './query' +import { quoteIdent, type ColumnKind } from './schema' + +export type OnDelete = 'cascade' | 'restrict' | 'set null' + +export type FieldConfig = { + readonly kind: ColumnKind + readonly nullable: boolean + readonly primaryKey: boolean + readonly unique: boolean + readonly defaultSql?: string + readonly references?: { readonly table: string; readonly column: string; readonly onDelete: OnDelete } +} + +export type FieldDef = FieldConfig & { readonly name: string } +export type IndexDef = { readonly name: string; readonly columns: readonly string[]; readonly unique: boolean } +export type TableSchema = { readonly name: string; readonly columns: readonly FieldDef[]; readonly indexes: readonly IndexDef[] } +export type DatabaseSchema = { readonly tables: readonly TableSchema[] } + +export class Field { + constructor(readonly spec: FieldConfig) {} + nullable(): Field { return new Field({ ...this.spec, nullable: true }) } + primaryKey(): Field { return new Field({ ...this.spec, primaryKey: true, nullable: false }) } + unique(): Field { return new Field({ ...this.spec, unique: true }) } + defaultSql(sql: string): Field { + if (sql.trim() === '' || sql.includes(';')) throw new Error(`Unsafe DEFAULT expression: ${JSON.stringify(sql)}`) + return new Field({ ...this.spec, defaultSql: sql }) + } + references(table: string, column: string, opts?: { onDelete?: OnDelete }): Field { + return new Field({ ...this.spec, references: { table, column, onDelete: opts?.onDelete ?? 'restrict' } }) + } +} + +function of(kind: ColumnKind): Field { + return new Field({ kind, nullable: false, primaryKey: false, unique: false }) +} +export const col = { + integer: () => of('integer'), + text: () => of('text'), + boolean: () => of('boolean'), + numeric: () => of('numeric'), + timestamptz: () => of('timestamptz'), +} +export class TableDraft { + constructor(readonly columns: Readonly>, readonly indexes: readonly IndexDef[] = []) {} + index(name: string, columns: readonly string[], opts?: { unique?: boolean }): TableDraft { + if (columns.length === 0) throw new Error(`Index ${JSON.stringify(name)} needs at least one column`) + return new TableDraft(this.columns, [...this.indexes, { name, columns: [...columns], unique: opts?.unique === true }]) + } +} +export function table(columns: Record): TableDraft { + if (Object.keys(columns).length === 0) throw new Error('table() needs at least one column') + return new TableDraft(columns) +} +export function defineSchema(drafts: Record): DatabaseSchema { + const tables: TableSchema[] = [] + const indexNames = new Set() + for (const [name, draft] of Object.entries(drafts)) { + quoteIdent(name) + const columns: FieldDef[] = [] + let pks = 0 + const colNames = new Set() + for (const [colName, field] of Object.entries(draft.columns)) { + quoteIdent(colName) + colNames.add(colName) + if (field.spec.primaryKey) pks += 1 + if (field.spec.primaryKey && field.spec.nullable) throw new Error(`Primary key ${name}.${colName} cannot be nullable`) + if (field.spec.references?.onDelete === 'set null' && !field.spec.nullable) { + throw new Error(`${name}.${colName} SET NULL requires a nullable column`) + } + if (field.spec.references) { + quoteIdent(field.spec.references.table) + quoteIdent(field.spec.references.column) + } + columns.push({ name: colName, ...field.spec }) + } + if (pks > 1) throw new Error(`Table ${name} has ${pks} primary keys; use one`) + const indexes: IndexDef[] = [] + for (const idx of draft.indexes) { + quoteIdent(idx.name) + if (indexNames.has(idx.name)) throw new Error(`Duplicate index name ${idx.name}`) + indexNames.add(idx.name) + for (const c of idx.columns) { + if (!colNames.has(c)) throw new Error(`Index ${idx.name} references unknown column ${name}.${c}`) + quoteIdent(c) + } + indexes.push(idx) + } + tables.push({ name, columns, indexes }) + } + const byName = new Map(tables.map((t) => [t.name, t])) + for (const t of tables) { + for (const c of t.columns) { + const ref = c.references + if (!ref) continue + const parent = byName.get(ref.table) + if (!parent) throw new Error(`${t.name}.${c.name} references unknown table ${ref.table}`) + const target = parent.columns.find((x) => x.name === ref.column) + if (!target) throw new Error(`${t.name}.${c.name} references unknown column ${ref.table}.${ref.column}`) + if (target.kind !== c.kind) { + throw new Error(`${t.name}.${c.name} type ${c.kind} does not match ${ref.table}.${ref.column} (${target.kind})`) + } + } + } + topoTables(tables) + return { tables } +} +export function topoTables(tables: readonly TableSchema[]): TableSchema[] { + const byName = new Map(tables.map((t) => [t.name, t])) + const names = tables.map((t) => t.name) + const indeg = new Map(names.map((n) => [n, 0])) + const children = new Map(names.map((n) => [n, []])) + for (const t of tables) { + const deps = new Set() + for (const c of t.columns) { + const ref = c.references + if (!ref || ref.table === t.name || !byName.has(ref.table)) continue + deps.add(ref.table) + } + for (const parent of deps) { + indeg.set(t.name, (indeg.get(t.name) ?? 0) + 1) + children.get(parent)?.push(t.name) + } + } + const ready = names.filter((n) => indeg.get(n) === 0) + const ordered: string[] = [] + while (ready.length > 0) { + const n = ready.shift() + if (n === undefined) break + ordered.push(n) + for (const child of children.get(n) ?? []) { + const next = (indeg.get(child) ?? 1) - 1 + indeg.set(child, next) + if (next === 0) ready.push(child) + } + } + if (ordered.length !== names.length) throw new Error('Foreign keys form a cycle') + return ordered.map((n) => byName.get(n)!) +} +export type MigrationOp = + | { kind: 'createTable'; table: TableSchema } + | { kind: 'dropTable'; name: string } + | { kind: 'addColumn'; table: string; column: FieldDef } + | { kind: 'dropColumn'; table: string; column: string } + | { kind: 'createIndex'; table: string; index: IndexDef } + | { kind: 'dropIndex'; name: string } + +function onDeleteSql(action: OnDelete): string { + return action === 'cascade' ? 'CASCADE' : action === 'restrict' ? 'RESTRICT' : 'SET NULL' +} +function sqlType(kind: ColumnKind, dialect: Dialect): string { + if (kind === 'timestamptz') return dialect === 'postgres' ? 'TIMESTAMPTZ' : 'TEXT' + if (kind === 'boolean') return dialect === 'postgres' ? 'BOOLEAN' : 'INTEGER' + return kind === 'numeric' ? 'NUMERIC' : kind === 'integer' ? 'INTEGER' : 'TEXT' +} +function columnSql(col: FieldDef, dialect: Dialect, inlineRefs: boolean): string { + const parts = [quoteIdent(col.name), sqlType(col.kind, dialect)] + if (!col.nullable) parts.push('NOT NULL') + if (col.primaryKey) parts.push('PRIMARY KEY') + if (col.unique) parts.push('UNIQUE') + if (col.defaultSql !== undefined) parts.push(`DEFAULT ${col.defaultSql}`) + if (inlineRefs && col.references) { + const ref = col.references + parts.push(`REFERENCES ${quoteIdent(ref.table)} (${quoteIdent(ref.column)}) ON DELETE ${onDeleteSql(ref.onDelete)}`) + } + return parts.join(' ') +} +export function compileOps(ops: readonly MigrationOp[], dialect: Dialect): string[] { + return ops.map((op) => { + switch (op.kind) { + case 'createTable': { + const body = op.table.columns.map((c) => columnSql(c, dialect, false)) + for (const c of op.table.columns) { + if (!c.references) continue + body.push( + `FOREIGN KEY (${quoteIdent(c.name)}) REFERENCES ${quoteIdent(c.references.table)} (${quoteIdent(c.references.column)}) ON DELETE ${onDeleteSql(c.references.onDelete)}`, + ) + } + return `CREATE TABLE ${quoteIdent(op.table.name)} (\n ${body.join(',\n ')}\n)` + } + case 'dropTable': + return `DROP TABLE ${quoteIdent(op.name)}` + case 'addColumn': + if (op.column.primaryKey) throw new Error('Cannot ADD COLUMN PRIMARY KEY; rebuild the table') + if (op.column.unique && dialect === 'sqlite') throw new Error('SQLite cannot ADD COLUMN UNIQUE; add a unique index') + return `ALTER TABLE ${quoteIdent(op.table)} ADD COLUMN ${columnSql(op.column, dialect, true)}` + case 'dropColumn': + return `ALTER TABLE ${quoteIdent(op.table)} DROP COLUMN ${quoteIdent(op.column)}` + case 'createIndex': { + const uniq = op.index.unique ? 'UNIQUE ' : '' + return `CREATE ${uniq}INDEX ${quoteIdent(op.index.name)} ON ${quoteIdent(op.table)} (${op.index.columns.map(quoteIdent).join(', ')})` + } + case 'dropIndex': + return `DROP INDEX ${quoteIdent(op.name)}` + } + }) +} diff --git a/src/index.ts b/src/index.ts index 25a2b24..6bc8d8a 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,2 +1,4 @@ export * from './schema' export * from './query' +export * from './ddl' +export * from './migrate' diff --git a/src/migrate.ts b/src/migrate.ts new file mode 100644 index 0000000..2b9d23f --- /dev/null +++ b/src/migrate.ts @@ -0,0 +1,158 @@ +import { compileOps, topoTables, type DatabaseSchema, type FieldDef, type IndexDef, type MigrationOp, type TableSchema } from './ddl' +import type { Dialect } from './query' +import { quoteIdent } from './schema' + +export type SqlSession = { + exec: (sql: string, params?: readonly unknown[]) => void + all: (sql: string, params?: readonly unknown[]) => Record[] +} + +export type Migration = { readonly id: string; readonly up: readonly MigrationOp[]; readonly down: readonly MigrationOp[] } + +const JOURNAL = '_schema_migrations' +const ID_RE = /^\d{3}_[A-Za-z0-9_]+$/ + +function tableMap(schema: DatabaseSchema): Map { + return new Map(schema.tables.map((t) => [t.name, t])) +} +function fieldEq(a: FieldDef, b: FieldDef): boolean { + return JSON.stringify([a.kind, a.nullable, a.primaryKey, a.unique, a.defaultSql, a.references]) === JSON.stringify([b.kind, b.nullable, b.primaryKey, b.unique, b.defaultSql, b.references]) +} +function indexEq(a: IndexDef, b: IndexDef): boolean { + return a.unique === b.unique && a.columns.length === b.columns.length && a.columns.every((c, i) => c === b.columns[i]) +} +export function diffSchema(from: DatabaseSchema, to: DatabaseSchema): MigrationOp[] { + const ops: MigrationOp[] = [] + const prev = tableMap(from) + const created = to.tables.filter((t) => !prev.has(t.name)) + const dropped = from.tables.filter((t) => !to.tables.some((x) => x.name === t.name)) + for (const table of topoTables(created)) { + ops.push({ kind: 'createTable', table: { name: table.name, columns: table.columns, indexes: [] } }) + } + for (const table of to.tables) { + const before = prev.get(table.name) + if (!before) continue + const beforeCols = new Map(before.columns.map((c) => [c.name, c])) + const afterCols = new Map(table.columns.map((c) => [c.name, c])) + for (const col of table.columns) { + const old = beforeCols.get(col.name) + if (!old) ops.push({ kind: 'addColumn', table: table.name, column: col }) + else if (!fieldEq(old, col)) throw new Error(`Cannot alter column ${table.name}.${col.name} in place; drop and add it`) + } + for (const col of before.columns) { + if (!afterCols.has(col.name)) ops.push({ kind: 'dropColumn', table: table.name, column: col.name }) + } + } + for (const table of to.tables) { + const before = prev.get(table.name) + if (!before) { + for (const idx of table.indexes) ops.push({ kind: 'createIndex', table: table.name, index: idx }) + continue + } + const oldIdx = new Map(before.indexes.map((i) => [i.name, i])) + const newIdx = new Map(table.indexes.map((i) => [i.name, i])) + for (const idx of table.indexes) { + const old = oldIdx.get(idx.name) + if (!old) ops.push({ kind: 'createIndex', table: table.name, index: idx }) + else if (!indexEq(old, idx)) { + ops.push({ kind: 'dropIndex', name: idx.name }) + ops.push({ kind: 'createIndex', table: table.name, index: idx }) + } + } + for (const idx of before.indexes) { + if (!newIdx.has(idx.name)) ops.push({ kind: 'dropIndex', name: idx.name }) + } + } + for (const table of [...topoTables(dropped)].reverse()) { + for (const idx of table.indexes) ops.push({ kind: 'dropIndex', name: idx.name }) + ops.push({ kind: 'dropTable', name: table.name }) + } + return ops +} +function assertIds(migrations: readonly Migration[]): void { + const seen = new Set() + let prev: string | undefined + for (const m of migrations) { + if (!ID_RE.test(m.id)) throw new Error(`Invalid migration id ${JSON.stringify(m.id)}`) + if (seen.has(m.id)) throw new Error(`Duplicate migration id ${m.id}`) + seen.add(m.id) + if (prev !== undefined && m.id <= prev) throw new Error(`Migrations must be sorted by id (${prev} then ${m.id})`) + prev = m.id + } +} +export class Migrator { + constructor(private readonly db: SqlSession, private readonly dialect: Dialect) {} + + applied(): string[] { + this.ensureJournal() + return this.db.all(`SELECT ${quoteIdent('id')} FROM ${quoteIdent(JOURNAL)} ORDER BY ${quoteIdent('id')}`).map((row) => { + if (typeof row.id !== 'string') throw new Error('Corrupt migration journal') + return row.id + }) + } + migrateUp(migrations: readonly Migration[]): string[] { + assertIds(migrations) + const done = this.applied() + this.assertPrefix(migrations, done) + const ran: string[] = [] + for (const m of migrations) { + if (done.includes(m.id)) continue + this.apply(m, 'up') + ran.push(m.id) + } + return ran + } + migrateDown(migrations: readonly Migration[], steps = 1): string[] { + if (!Number.isInteger(steps) || steps < 0) throw new Error(`migrateDown() expects a non-negative integer, got ${String(steps)}`) + assertIds(migrations) + const done = this.applied() + this.assertPrefix(migrations, done) + const byId = new Map(migrations.map((m) => [m.id, m])) + const ran: string[] = [] + for (let i = 0; i < steps; i += 1) { + const id = done[done.length - 1 - i] + if (id === undefined) break + const m = byId.get(id) + if (!m) throw new Error(`Applied migration ${id} is not in the provided set`) + this.apply(m, 'down') + ran.push(id) + } + return ran + } + private ensureJournal(): void { + this.db.exec( + `CREATE TABLE IF NOT EXISTS ${quoteIdent(JOURNAL)} (${quoteIdent('id')} TEXT NOT NULL PRIMARY KEY, ${quoteIdent('appliedAt')} TEXT NOT NULL)`, + ) + } + private assertPrefix(migrations: readonly Migration[], applied: string[]): void { + const ids = migrations.map((m) => m.id) + const known = new Set(ids) + for (const id of applied) { + if (!known.has(id)) throw new Error(`Unknown applied migration ${JSON.stringify(id)}`) + } + for (let i = 0; i < applied.length; i += 1) { + if (applied[i] !== ids[i]) throw new Error('Migration history is not a prefix of the migration list') + } + } + private ph(n: number): string { + return this.dialect === 'sqlite' ? '?' : `$${n}` + } + private apply(m: Migration, direction: 'up' | 'down'): void { + this.db.exec('BEGIN') + try { + for (const sql of compileOps(direction === 'up' ? m.up : m.down, this.dialect)) this.db.exec(sql) + if (direction === 'up') { + this.db.exec(`INSERT INTO ${quoteIdent(JOURNAL)} (${quoteIdent('id')}, ${quoteIdent('appliedAt')}) VALUES (${this.ph(1)}, ${this.ph(2)})`, [ + m.id, + new Date().toISOString(), + ]) + } else { + this.db.exec(`DELETE FROM ${quoteIdent(JOURNAL)} WHERE ${quoteIdent('id')} = ${this.ph(1)}`, [m.id]) + } + this.db.exec('COMMIT') + } catch (err) { + this.db.exec('ROLLBACK') + throw err + } + } +} diff --git a/test/schema-dsl.test.ts b/test/schema-dsl.test.ts new file mode 100644 index 0000000..290003d --- /dev/null +++ b/test/schema-dsl.test.ts @@ -0,0 +1,120 @@ +import { expect, test } from 'vitest' +import { + col, compileOps, defineSchema, diffSchema, Migrator, table, type Migration, type SqlSession, +} from '../src/index' + +const empty = defineSchema({}) +const users = defineSchema({ + users: table({ id: col.integer().primaryKey(), email: col.text().unique(), active: col.boolean().defaultSql('1') }).index('users_email_idx', ['email']), +}) +const orders = defineSchema({ + users: table({ id: col.integer().primaryKey(), email: col.text().unique() }), + orders: table({ + id: col.integer().primaryKey(), + userId: col.integer().references('users', 'id', { onDelete: 'cascade' }), + note: col.text().nullable(), + paidAt: col.timestamptz().nullable(), + }).index('orders_user_idx', ['userId']), +}) + +function migration(id: string, from: typeof empty, to: typeof empty): Migration { + return { id, up: diffSchema(from, to), down: diffSchema(to, from) } +} + +type SqliteDb = { exec: (sql: string) => void; prepare: (sql: string) => { all: (...p: unknown[]) => unknown; run: (...p: unknown[]) => unknown }; close: () => void } + +function sqlite(): { session: SqlSession; names: () => string[]; close: () => void } { + const loaded = (globalThis as { process?: { getBuiltinModule?: (id: string) => { DatabaseSync: new (path: string) => SqliteDb } } }).process?.getBuiltinModule?.('node:sqlite') + if (!loaded) throw new Error('node:sqlite is not available in this runtime') + const db = new loaded.DatabaseSync(':memory:') + db.exec('PRAGMA foreign_keys = ON') + const session: SqlSession = { + exec: (sql, params) => (!params || params.length === 0 ? db.exec(sql) : db.prepare(sql).run(...params)), + all: (sql, params) => db.prepare(sql).all(...(params ?? [])) as Record[], + } + const names = () => (db.prepare(`SELECT name FROM sqlite_master WHERE type = 'table' AND name NOT LIKE 'sqlite_%' ORDER BY name`).all() as { name: string }[]).map((row) => row.name) + return { session, names, close: () => db.close() } +} + +test('compiles dialect types, FKs, defaults; create order follows FK edges', () => { + const up = diffSchema(empty, orders) + expect(up.map((op) => op.kind)).toEqual(['createTable', 'createTable', 'createIndex']) + expect(up[0] && up[0].kind === 'createTable' ? up[0].table.name : '').toBe('users') + expect(up[1] && up[1].kind === 'createTable' ? up[1].table.name : '').toBe('orders') + const pg = compileOps(up, 'postgres') + expect(pg[0]).toContain('"email" TEXT NOT NULL UNIQUE') + expect(pg[1]).toContain('"paidAt" TIMESTAMPTZ') + expect(pg[1]).toContain('FOREIGN KEY ("userId") REFERENCES "users" ("id") ON DELETE CASCADE') + expect(compileOps(up, 'sqlite')[1]).toContain('"paidAt" TEXT') + expect(compileOps(up, 'sqlite')[1]).toContain('"userId" INTEGER NOT NULL') + expect(compileOps(diffSchema(empty, users), 'sqlite')[0]).toContain('DEFAULT 1') + expect(compileOps([{ kind: 'addColumn', table: 'users', column: { name: 'bio', kind: 'text', nullable: true, primaryKey: false, unique: false } }], 'postgres')[0]).toBe( + 'ALTER TABLE "users" ADD COLUMN "bio" TEXT', + ) + expect(diffSchema(orders, empty).map((op) => (op.kind === 'dropTable' ? op.name : op.kind))).toEqual(['dropIndex', 'orders', 'users']) +}) + +test('schema DSL rejects cycles, bad defaults, and in-place column rewrites', () => { + expect(() => table({})).toThrow(/at least one column/) + expect(() => defineSchema({ t: table({ a: col.integer().primaryKey(), b: col.integer().primaryKey() }) })).toThrow(/primary keys/) + expect(() => defineSchema({ t: table({ a: col.integer().references('t', 'a', { onDelete: 'set null' }) }) })).toThrow(/nullable/) + expect(() => defineSchema({ t: table({ a: col.integer().references('missing', 'id') }) })).toThrow(/unknown table/) + expect(() => + defineSchema({ + a: table({ id: col.integer().primaryKey(), bId: col.integer().references('b', 'id') }), + b: table({ id: col.integer().primaryKey(), aId: col.integer().references('a', 'id') }), + }), + ).toThrow(/cycle/) + expect(() => col.text().defaultSql('x; drop')).toThrow(/Unsafe DEFAULT/) + expect(() => table({ id: col.integer() }).index('empty', [])).toThrow(/at least one column/) + expect(() => defineSchema({ t: table({ id: col.integer() }).index('t_x', ['nope']) })).toThrow(/unknown column/) + const orgs = defineSchema({ orgs: table({ id: col.integer().primaryKey(), parentId: col.integer().nullable().references('orgs', 'id') }) }) + expect(compileOps(diffSchema(empty, orgs), 'sqlite')[0]).toContain('FOREIGN KEY ("parentId") REFERENCES "orgs" ("id")') + expect(() => diffSchema(users, defineSchema({ users: table({ id: col.integer().primaryKey(), email: col.integer().unique() }) }))).toThrow(/in place/) + const added = defineSchema({ + users: table({ id: col.integer().primaryKey(), email: col.text().unique(), active: col.boolean().defaultSql('1'), bio: col.text().nullable() }).index( + 'users_email_idx', + ['email'], + ), + }) + expect(diffSchema(users, added).map((op) => op.kind)).toEqual(['addColumn']) + expect(diffSchema(added, users).map((op) => op.kind)).toEqual(['dropColumn']) + const pkCol = { name: 'id', kind: 'integer' as const, nullable: false, primaryKey: true, unique: false } + const uniqCol = { name: 'e', kind: 'text' as const, nullable: false, primaryKey: false, unique: true } + expect(() => compileOps([{ kind: 'addColumn', table: 't', column: pkCol }], 'sqlite')).toThrow(/PRIMARY KEY/) + expect(() => compileOps([{ kind: 'addColumn', table: 't', column: uniqCol }], 'sqlite')).toThrow(/UNIQUE/) +}) + +test('migrator applies, skips, rolls back, and walks down against sqlite', () => { + const { session, names, close } = sqlite() + try { + const m = new Migrator(session, 'sqlite') + const chain = [migration('001_users', empty, users), migration('002_orders', users, orders)] + expect(m.migrateUp(chain)).toEqual(['001_users', '002_orders']) + expect(m.migrateUp(chain)).toEqual([]) + expect(m.applied()).toEqual(['001_users', '002_orders']) + expect(names()).toEqual(['_schema_migrations', 'orders', 'users']) + expect(m.migrateDown(chain, 1)).toEqual(['002_orders']) + expect(names()).toEqual(['_schema_migrations', 'users']) + expect(m.migrateDown(chain, 8)).toEqual(['001_users']) + expect(names()).toEqual(['_schema_migrations']) + expect(m.migrateDown(chain, 0)).toEqual([]) + m.migrateUp(chain) + const boom: Migration = { id: '003_dup', up: diffSchema(empty, users), down: [] } + expect(() => m.migrateUp([...chain, boom])).toThrow() + expect(m.applied()).toEqual(['001_users', '002_orders']) + session.exec(`INSERT INTO "_schema_migrations" ("id", "appliedAt") VALUES (?, ?)`, ['004_ghost', '2026-01-01T00:00:00.000Z']) + expect(() => + m.migrateUp([...chain, { id: '003_mid', up: [], down: [] }, { id: '004_ghost', up: [], down: [] }]), + ).toThrow(/prefix/) + expect(() => m.migrateUp([{ id: '1_bad', up: [], down: [] }])).toThrow(/Invalid migration id/) + expect(() => m.migrateDown(chain, -1)).toThrow(/non-negative/) + } finally { + close() + } + const stmts: string[] = [] + const pg: SqlSession = { exec: (sql) => { stmts.push(sql) }, all: () => [] } + expect(new Migrator(pg, 'postgres').migrateUp([{ id: '001_init', up: [], down: [] }])).toEqual(['001_init']) + expect(stmts.some((sql) => sql.includes('VALUES ($1, $2)'))).toBe(true) + expect(stmts.every((sql) => !sql.includes('?'))).toBe(true) +}) From fa123a8057ab5084e3ad8c814c19293dcfd3d46b Mon Sep 17 00:00:00 2001 From: Thomas Hart Date: Fri, 4 Sep 2026 20:51:36 +0000 Subject: [PATCH 2/5] fix: drop indexes before columns on schema diff SQLite rejects DROP COLUMN while an index still references the column. Postgres would drop the index with the column and then fail the extra DROP INDEX. Emit those index drops first, and only once. SQLite also cannot DROP COLUMN for PRIMARY KEY, UNIQUE, or foreign keys, or ADD COLUMN NOT NULL without DEFAULT. Fail at compile time instead of emitting data-dependent DDL. --- src/ddl.ts | 12 ++++++-- src/migrate.ts | 27 +++++++++++++---- test/schema-dsl.test.ts | 65 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 97 insertions(+), 7 deletions(-) diff --git a/src/ddl.ts b/src/ddl.ts index 0446893..a860a13 100644 --- a/src/ddl.ts +++ b/src/ddl.ts @@ -141,7 +141,7 @@ export type MigrationOp = | { kind: 'createTable'; table: TableSchema } | { kind: 'dropTable'; name: string } | { kind: 'addColumn'; table: string; column: FieldDef } - | { kind: 'dropColumn'; table: string; column: string } + | { kind: 'dropColumn'; table: string; column: FieldDef } | { kind: 'createIndex'; table: string; index: IndexDef } | { kind: 'dropIndex'; name: string } @@ -183,9 +183,17 @@ export function compileOps(ops: readonly MigrationOp[], dialect: Dialect): strin case 'addColumn': if (op.column.primaryKey) throw new Error('Cannot ADD COLUMN PRIMARY KEY; rebuild the table') if (op.column.unique && dialect === 'sqlite') throw new Error('SQLite cannot ADD COLUMN UNIQUE; add a unique index') + if (dialect === 'sqlite' && !op.column.nullable && op.column.defaultSql === undefined) { + throw new Error('SQLite cannot ADD COLUMN NOT NULL without DEFAULT') + } return `ALTER TABLE ${quoteIdent(op.table)} ADD COLUMN ${columnSql(op.column, dialect, true)}` case 'dropColumn': - return `ALTER TABLE ${quoteIdent(op.table)} DROP COLUMN ${quoteIdent(op.column)}` + if (dialect === 'sqlite') { + if (op.column.primaryKey) throw new Error('SQLite cannot DROP COLUMN PRIMARY KEY; rebuild the table') + if (op.column.unique) throw new Error('SQLite cannot DROP COLUMN UNIQUE; rebuild the table') + if (op.column.references) throw new Error('SQLite cannot DROP COLUMN with a foreign key; rebuild the table') + } + return `ALTER TABLE ${quoteIdent(op.table)} DROP COLUMN ${quoteIdent(op.column.name)}` case 'createIndex': { const uniq = op.index.unique ? 'UNIQUE ' : '' return `CREATE ${uniq}INDEX ${quoteIdent(op.index.name)} ON ${quoteIdent(op.table)} (${op.index.columns.map(quoteIdent).join(', ')})` diff --git a/src/migrate.ts b/src/migrate.ts index 2b9d23f..b574c2c 100644 --- a/src/migrate.ts +++ b/src/migrate.ts @@ -26,6 +26,7 @@ export function diffSchema(from: DatabaseSchema, to: DatabaseSchema): MigrationO const prev = tableMap(from) const created = to.tables.filter((t) => !prev.has(t.name)) const dropped = from.tables.filter((t) => !to.tables.some((x) => x.name === t.name)) + const droppedIndexes = new Set() for (const table of topoTables(created)) { ops.push({ kind: 'createTable', table: { name: table.name, columns: table.columns, indexes: [] } }) } @@ -39,8 +40,15 @@ export function diffSchema(from: DatabaseSchema, to: DatabaseSchema): MigrationO if (!old) ops.push({ kind: 'addColumn', table: table.name, column: col }) else if (!fieldEq(old, col)) throw new Error(`Cannot alter column ${table.name}.${col.name} in place; drop and add it`) } - for (const col of before.columns) { - if (!afterCols.has(col.name)) ops.push({ kind: 'dropColumn', table: table.name, column: col.name }) + const removed = before.columns.filter((c) => !afterCols.has(c.name)) + const removedNames = new Set(removed.map((c) => c.name)) + for (const idx of before.indexes) { + if (!idx.columns.some((name) => removedNames.has(name)) || droppedIndexes.has(idx.name)) continue + ops.push({ kind: 'dropIndex', name: idx.name }) + droppedIndexes.add(idx.name) + } + for (const col of removed) { + ops.push({ kind: 'dropColumn', table: table.name, column: col }) } } for (const table of to.tables) { @@ -55,16 +63,25 @@ export function diffSchema(from: DatabaseSchema, to: DatabaseSchema): MigrationO const old = oldIdx.get(idx.name) if (!old) ops.push({ kind: 'createIndex', table: table.name, index: idx }) else if (!indexEq(old, idx)) { - ops.push({ kind: 'dropIndex', name: idx.name }) + if (!droppedIndexes.has(idx.name)) { + ops.push({ kind: 'dropIndex', name: idx.name }) + droppedIndexes.add(idx.name) + } ops.push({ kind: 'createIndex', table: table.name, index: idx }) } } for (const idx of before.indexes) { - if (!newIdx.has(idx.name)) ops.push({ kind: 'dropIndex', name: idx.name }) + if (newIdx.has(idx.name) || droppedIndexes.has(idx.name)) continue + ops.push({ kind: 'dropIndex', name: idx.name }) + droppedIndexes.add(idx.name) } } for (const table of [...topoTables(dropped)].reverse()) { - for (const idx of table.indexes) ops.push({ kind: 'dropIndex', name: idx.name }) + for (const idx of table.indexes) { + if (droppedIndexes.has(idx.name)) continue + ops.push({ kind: 'dropIndex', name: idx.name }) + droppedIndexes.add(idx.name) + } ops.push({ kind: 'dropTable', name: table.name }) } return ops diff --git a/test/schema-dsl.test.ts b/test/schema-dsl.test.ts index 290003d..e2d101e 100644 --- a/test/schema-dsl.test.ts +++ b/test/schema-dsl.test.ts @@ -85,6 +85,71 @@ test('schema DSL rejects cycles, bad defaults, and in-place column rewrites', () expect(() => compileOps([{ kind: 'addColumn', table: 't', column: uniqCol }], 'sqlite')).toThrow(/UNIQUE/) }) +test('sqlite drops indexes before columns and rejects unsafe ALTER TABLE', () => { + const indexed = defineSchema({ + items: table({ id: col.integer().primaryKey(), label: col.text() }).index('items_label_idx', ['label']), + }) + const stripped = defineSchema({ + items: table({ id: col.integer().primaryKey() }), + }) + const dropIndexed = diffSchema(indexed, stripped) + expect(dropIndexed.map((op) => op.kind)).toEqual(['dropIndex', 'dropColumn']) + expect(dropIndexed.filter((op) => op.kind === 'dropIndex')).toHaveLength(1) + expect(dropIndexed[1] && dropIndexed[1].kind === 'dropColumn' ? dropIndexed[1].column.name : '').toBe('label') + + const uniqueCol = defineSchema({ t: table({ id: col.integer().primaryKey(), email: col.text().unique() }) }) + const noUnique = defineSchema({ t: table({ id: col.integer().primaryKey() }) }) + expect(() => compileOps(diffSchema(uniqueCol, noUnique), 'sqlite')).toThrow(/UNIQUE/) + + const pkTable = defineSchema({ t: table({ id: col.integer().primaryKey(), name: col.text() }) }) + const noPk = defineSchema({ t: table({ name: col.text() }) }) + expect(() => compileOps(diffSchema(pkTable, noPk), 'sqlite')).toThrow(/PRIMARY KEY/) + + const withFk = defineSchema({ + users: table({ id: col.integer().primaryKey() }), + orders: table({ id: col.integer().primaryKey(), userId: col.integer().references('users', 'id') }), + }) + const noFk = defineSchema({ + users: table({ id: col.integer().primaryKey() }), + orders: table({ id: col.integer().primaryKey() }), + }) + expect(() => compileOps(diffSchema(withFk, noFk), 'sqlite')).toThrow(/foreign key/) + + const notNullAdd = { + kind: 'addColumn' as const, + table: 't', + column: { name: 'n', kind: 'text' as const, nullable: false, primaryKey: false, unique: false }, + } + expect(() => compileOps([notNullAdd], 'sqlite')).toThrow(/NOT NULL/) + expect(compileOps([notNullAdd], 'postgres')[0]).toContain('NOT NULL') + + const withNote = defineSchema({ + items: table({ id: col.integer().primaryKey(), note: col.text().nullable() }), + }) + const { session, close } = sqlite() + try { + const m = new Migrator(session, 'sqlite') + const dropLabel = [migration('001_items', empty, indexed), migration('002_drop_label', indexed, stripped)] + expect(m.migrateUp([dropLabel[0]!])).toEqual(['001_items']) + session.exec(`INSERT INTO "items" ("id", "label") VALUES (1, 'a')`) + expect(m.migrateUp(dropLabel)).toEqual(['002_drop_label']) + expect(session.all(`SELECT "id" FROM "items"`)).toEqual([{ id: 1 }]) + expect(() => + m.migrateUp([...dropLabel, { id: '003_unique', up: diffSchema(uniqueCol, noUnique), down: [] }]), + ).toThrow(/UNIQUE/) + expect(() => m.migrateUp([...dropLabel, { id: '003_pk', up: diffSchema(pkTable, noPk), down: [] }])).toThrow(/PRIMARY KEY/) + expect(() => m.migrateUp([...dropLabel, { id: '003_fk', up: diffSchema(withFk, noFk), down: [] }])).toThrow(/foreign key/) + expect(() => m.migrateUp([...dropLabel, { id: '003_nn', up: [notNullAdd], down: [] }])).toThrow(/NOT NULL/) + const noteChain = [...dropLabel, migration('003_note', stripped, withNote)] + expect(m.migrateUp(noteChain)).toEqual(['003_note']) + expect(session.all(`SELECT "note" FROM "items"`)).toEqual([{ note: null }]) + expect(m.migrateDown(noteChain, 1)).toEqual(['003_note']) + expect(session.all(`SELECT "id" FROM "items"`)).toEqual([{ id: 1 }]) + } finally { + close() + } +}) + test('migrator applies, skips, rolls back, and walks down against sqlite', () => { const { session, names, close } = sqlite() try { From 9e3a71547ec5eab9ce5a8f1039838f1c2a80c0d9 Mon Sep 17 00:00:00 2001 From: Thomas Hart Date: Fri, 4 Sep 2026 21:03:42 +0000 Subject: [PATCH 3/5] fix: reject foreign keys to non-unique parent columns SQLite and Postgres both require the referenced column to be a primary key, UNIQUE, or covered by a single-column unique index. defineSchema now throws at schema time instead of letting CREATE TABLE succeed and INSERT fail with a foreign key mismatch. --- src/ddl.ts | 7 +++++++ test/schema-dsl.test.ts | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/src/ddl.ts b/src/ddl.ts index a860a13..7e4d674 100644 --- a/src/ddl.ts +++ b/src/ddl.ts @@ -100,11 +100,18 @@ export function defineSchema(drafts: Record): DatabaseSchema if (target.kind !== c.kind) { throw new Error(`${t.name}.${c.name} type ${c.kind} does not match ${ref.table}.${ref.column} (${target.kind})`) } + if (!isUniqueFkTarget(parent, target)) { + throw new Error(`${t.name}.${c.name} references non-unique column ${ref.table}.${ref.column}`) + } } } topoTables(tables) return { tables } } +function isUniqueFkTarget(parent: TableSchema, target: FieldDef): boolean { + if (target.primaryKey || target.unique) return true + return parent.indexes.some((idx) => idx.unique && idx.columns.length === 1 && idx.columns[0] === target.name) +} export function topoTables(tables: readonly TableSchema[]): TableSchema[] { const byName = new Map(tables.map((t) => [t.name, t])) const names = tables.map((t) => t.name) diff --git a/test/schema-dsl.test.ts b/test/schema-dsl.test.ts index e2d101e..e495a88 100644 --- a/test/schema-dsl.test.ts +++ b/test/schema-dsl.test.ts @@ -85,6 +85,38 @@ test('schema DSL rejects cycles, bad defaults, and in-place column rewrites', () expect(() => compileOps([{ kind: 'addColumn', table: 't', column: uniqCol }], 'sqlite')).toThrow(/UNIQUE/) }) +test('defineSchema requires unique or primary-key FK parents', () => { + expect(() => + defineSchema({ + users: table({ id: col.integer().primaryKey(), email: col.text() }), + orders: table({ id: col.integer().primaryKey(), email: col.text().references('users', 'email') }), + }), + ).toThrow(/orders\.email.*users\.email/) + + const byPk = defineSchema({ + users: table({ id: col.integer().primaryKey() }), + orders: table({ id: col.integer().primaryKey(), userId: col.integer().references('users', 'id') }), + }) + expect(byPk.tables.map((t) => t.name).sort()).toEqual(['orders', 'users']) + + const byUniqueIndex = defineSchema({ + users: table({ id: col.integer().primaryKey(), email: col.text() }).index('users_email_uq', ['email'], { unique: true }), + orders: table({ id: col.integer().primaryKey(), email: col.text().references('users', 'email') }), + }) + expect(byUniqueIndex.tables).toHaveLength(2) + + const { session, close } = sqlite() + try { + const m = new Migrator(session, 'sqlite') + expect(m.migrateUp([migration('001_fk_idx', empty, byUniqueIndex)])).toEqual(['001_fk_idx']) + session.exec(`INSERT INTO "users" ("id", "email") VALUES (1, 'a@b.com')`) + session.exec(`INSERT INTO "orders" ("id", "email") VALUES (1, 'a@b.com')`) + expect(session.all(`SELECT "email" FROM "orders"`)).toEqual([{ email: 'a@b.com' }]) + } finally { + close() + } +}) + test('sqlite drops indexes before columns and rejects unsafe ALTER TABLE', () => { const indexed = defineSchema({ items: table({ id: col.integer().primaryKey(), label: col.text() }).index('items_label_idx', ['label']), From 9f3123914e957065a5f4c23f81ba2e0936910d48 Mon Sep 17 00:00:00 2001 From: Thomas Hart Date: Fri, 4 Sep 2026 21:16:05 +0000 Subject: [PATCH 4/5] fix: reject sqlite ADD COLUMN forms the engine cannot apply SQLite refuses ADD COLUMN with a REFERENCES clause and a non-NULL default, and it refuses CURRENT_TIME/DATE/TIMESTAMP or parenthesized defaults. Fail those in compileOps before apply. CREATE TABLE with the same column, and constant defaults on ADD COLUMN, still compile. --- src/ddl.ts | 24 +++++- test/schema-dsl.test.ts | 157 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 179 insertions(+), 2 deletions(-) diff --git a/src/ddl.ts b/src/ddl.ts index 7e4d674..20a1a80 100644 --- a/src/ddl.ts +++ b/src/ddl.ts @@ -172,6 +172,14 @@ function columnSql(col: FieldDef, dialect: Dialect, inlineRefs: boolean): string } return parts.join(' ') } +function isNullSqlDefault(sql: string): boolean { + return sql.trim().toUpperCase() === 'NULL' +} +function isNonConstantSqlDefault(sql: string): boolean { + const trimmed = sql.trim() + if (/^CURRENT_(?:TIME|DATE|TIMESTAMP)$/i.test(trimmed)) return true + return trimmed.startsWith('(') +} export function compileOps(ops: readonly MigrationOp[], dialect: Dialect): string[] { return ops.map((op) => { switch (op.kind) { @@ -190,8 +198,20 @@ export function compileOps(ops: readonly MigrationOp[], dialect: Dialect): strin case 'addColumn': if (op.column.primaryKey) throw new Error('Cannot ADD COLUMN PRIMARY KEY; rebuild the table') if (op.column.unique && dialect === 'sqlite') throw new Error('SQLite cannot ADD COLUMN UNIQUE; add a unique index') - if (dialect === 'sqlite' && !op.column.nullable && op.column.defaultSql === undefined) { - throw new Error('SQLite cannot ADD COLUMN NOT NULL without DEFAULT') + if (dialect === 'sqlite') { + if (!op.column.nullable && op.column.defaultSql === undefined) { + throw new Error('SQLite cannot ADD COLUMN NOT NULL without DEFAULT') + } + if ( + op.column.references && + op.column.defaultSql !== undefined && + !isNullSqlDefault(op.column.defaultSql) + ) { + throw new Error('SQLite cannot ADD COLUMN REFERENCES with a non-NULL default') + } + if (op.column.defaultSql !== undefined && isNonConstantSqlDefault(op.column.defaultSql)) { + throw new Error('SQLite cannot ADD COLUMN with a non-constant default') + } } return `ALTER TABLE ${quoteIdent(op.table)} ADD COLUMN ${columnSql(op.column, dialect, true)}` case 'dropColumn': diff --git a/test/schema-dsl.test.ts b/test/schema-dsl.test.ts index e495a88..1d3ff57 100644 --- a/test/schema-dsl.test.ts +++ b/test/schema-dsl.test.ts @@ -215,3 +215,160 @@ test('migrator applies, skips, rolls back, and walks down against sqlite', () => expect(stmts.some((sql) => sql.includes('VALUES ($1, $2)'))).toBe(true) expect(stmts.every((sql) => !sql.includes('?'))).toBe(true) }) + +test('sqlite addColumn rejects FK non-NULL defaults and non-constant defaults', () => { + const fkTarget = { table: 'users', column: 'id', onDelete: 'restrict' as const } + const fkNotNullDefault = { + kind: 'addColumn' as const, + table: 'posts', + column: { + name: 'userId', + kind: 'integer' as const, + nullable: false, + primaryKey: false, + unique: false, + defaultSql: '1', + references: fkTarget, + }, + } + const fkNullableDefault = { + kind: 'addColumn' as const, + table: 'posts', + column: { + name: 'userId', + kind: 'integer' as const, + nullable: true, + primaryKey: false, + unique: false, + defaultSql: '1', + references: fkTarget, + }, + } + const currentTs = { + kind: 'addColumn' as const, + table: 'posts', + column: { + name: 'paidAt', + kind: 'timestamptz' as const, + nullable: true, + primaryKey: false, + unique: false, + defaultSql: 'CURRENT_TIMESTAMP', + }, + } + const exprDefault = { + kind: 'addColumn' as const, + table: 'posts', + column: { + name: 'n', + kind: 'integer' as const, + nullable: true, + primaryKey: false, + unique: false, + defaultSql: '(1+1)', + }, + } + const constOne = { + kind: 'addColumn' as const, + table: 'posts', + column: { name: 'n', kind: 'integer' as const, nullable: true, primaryKey: false, unique: false, defaultSql: '1' }, + } + const constX = { + kind: 'addColumn' as const, + table: 'posts', + column: { name: 'tag', kind: 'text' as const, nullable: true, primaryKey: false, unique: false, defaultSql: "'x'" }, + } + const fkNullDefault = { + kind: 'addColumn' as const, + table: 'posts', + column: { + name: 'userId', + kind: 'integer' as const, + nullable: true, + primaryKey: false, + unique: false, + defaultSql: 'NULL', + references: fkTarget, + }, + } + + expect(() => compileOps([fkNotNullDefault], 'sqlite')).toThrow(/ADD COLUMN REFERENCES.*non-NULL default/) + expect(() => compileOps([fkNullableDefault], 'sqlite')).toThrow(/ADD COLUMN REFERENCES.*non-NULL default/) + expect(() => compileOps([currentTs], 'sqlite')).toThrow(/non-constant default/) + expect(() => + compileOps([{ ...currentTs, column: { ...currentTs.column, defaultSql: 'current_timestamp' } }], 'sqlite'), + ).toThrow(/non-constant default/) + expect(() => + compileOps([{ ...currentTs, column: { ...currentTs.column, defaultSql: 'CURRENT_DATE' } }], 'sqlite'), + ).toThrow(/non-constant default/) + expect(() => compileOps([exprDefault], 'sqlite')).toThrow(/non-constant default/) + expect(compileOps([constOne], 'sqlite')[0]).toContain('DEFAULT 1') + expect(compileOps([constX], 'sqlite')[0]).toContain("DEFAULT 'x'") + expect(compileOps([fkNullDefault], 'sqlite')[0]).toContain('REFERENCES') + expect(compileOps([fkNotNullDefault], 'postgres')[0]).toContain('DEFAULT 1') + expect(compileOps([currentTs], 'postgres')[0]).toContain('CURRENT_TIMESTAMP') + + const createdWithFkDefault = defineSchema({ + users: table({ id: col.integer().primaryKey() }), + orders: table({ + id: col.integer().primaryKey(), + userId: col.integer().defaultSql('1').references('users', 'id'), + }), + }) + const createSql = compileOps(diffSchema(empty, createdWithFkDefault), 'sqlite') + expect(createSql[1]).toContain('DEFAULT 1') + expect(createSql[1]).toContain('FOREIGN KEY ("userId") REFERENCES "users" ("id")') + + const base = defineSchema({ + users: table({ id: col.integer().primaryKey() }), + posts: table({ id: col.integer().primaryKey() }), + }) + const withNullableFk = defineSchema({ + users: table({ id: col.integer().primaryKey() }), + posts: table({ + id: col.integer().primaryKey(), + userId: col.integer().nullable().references('users', 'id'), + }), + }) + + const { session, close } = sqlite() + try { + const m = new Migrator(session, 'sqlite') + const start = [migration('001_base', empty, base)] + expect(m.migrateUp(start)).toEqual(['001_base']) + session.exec(`INSERT INTO "users" ("id") VALUES (1)`) + session.exec(`INSERT INTO "posts" ("id") VALUES (1)`) + expect(() => m.migrateUp([...start, { id: '002_fk_nn', up: [fkNotNullDefault], down: [] }])).toThrow( + /ADD COLUMN REFERENCES.*non-NULL default/, + ) + expect(() => m.migrateUp([...start, { id: '002_fk_d', up: [fkNullableDefault], down: [] }])).toThrow( + /ADD COLUMN REFERENCES.*non-NULL default/, + ) + expect(() => m.migrateUp([...start, { id: '002_ts', up: [currentTs], down: [] }])).toThrow(/non-constant default/) + expect(() => m.migrateUp([...start, { id: '002_expr', up: [exprDefault], down: [] }])).toThrow(/non-constant default/) + expect(m.applied()).toEqual(['001_base']) + + const addFk = [...start, migration('002_fk_null', base, withNullableFk)] + expect(m.migrateUp(addFk)).toEqual(['002_fk_null']) + expect(session.all(`SELECT "userId" FROM "posts"`)).toEqual([{ userId: null }]) + session.exec(`UPDATE "posts" SET "userId" = 1 WHERE "id" = 1`) + expect(session.all(`SELECT "userId" FROM "posts"`)).toEqual([{ userId: 1 }]) + + const addConsts = [...addFk, { id: '003_consts', up: [constOne, constX], down: [] }] + expect(m.migrateUp(addConsts)).toEqual(['003_consts']) + expect(session.all(`SELECT "n", "tag" FROM "posts"`)).toEqual([{ n: 1, tag: 'x' }]) + } finally { + close() + } + + const created = sqlite() + try { + const m = new Migrator(created.session, 'sqlite') + expect(m.migrateUp([migration('001_fk_default', empty, createdWithFkDefault)])).toEqual(['001_fk_default']) + created.session.exec(`INSERT INTO "users" ("id") VALUES (1)`) + created.session.exec(`INSERT INTO "orders" ("id") VALUES (1)`) + expect(created.session.all(`SELECT "userId" FROM "orders"`)).toEqual([{ userId: 1 }]) + } finally { + created.close() + } +}) From 3d51a75c0e5622f96718ec1f5728a9962e541eb0 Mon Sep 17 00:00:00 2001 From: Thomas Hart Date: Fri, 4 Sep 2026 21:28:32 +0000 Subject: [PATCH 5/5] fix: reject sqlite ADD COLUMN NOT NULL DEFAULT NULL SQLite treats DEFAULT NULL the same as a missing default on ADD COLUMN NOT NULL. Fail those in compileOps. CREATE TABLE with the same column still compiles and applies. --- src/ddl.ts | 7 +++++-- test/schema-dsl.test.ts | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/src/ddl.ts b/src/ddl.ts index 20a1a80..701b340 100644 --- a/src/ddl.ts +++ b/src/ddl.ts @@ -199,8 +199,11 @@ export function compileOps(ops: readonly MigrationOp[], dialect: Dialect): strin if (op.column.primaryKey) throw new Error('Cannot ADD COLUMN PRIMARY KEY; rebuild the table') if (op.column.unique && dialect === 'sqlite') throw new Error('SQLite cannot ADD COLUMN UNIQUE; add a unique index') if (dialect === 'sqlite') { - if (!op.column.nullable && op.column.defaultSql === undefined) { - throw new Error('SQLite cannot ADD COLUMN NOT NULL without DEFAULT') + if ( + !op.column.nullable && + (op.column.defaultSql === undefined || isNullSqlDefault(op.column.defaultSql)) + ) { + throw new Error('SQLite cannot ADD COLUMN NOT NULL without a non-NULL DEFAULT') } if ( op.column.references && diff --git a/test/schema-dsl.test.ts b/test/schema-dsl.test.ts index 1d3ff57..f16a396 100644 --- a/test/schema-dsl.test.ts +++ b/test/schema-dsl.test.ts @@ -152,8 +152,32 @@ test('sqlite drops indexes before columns and rejects unsafe ALTER TABLE', () => table: 't', column: { name: 'n', kind: 'text' as const, nullable: false, primaryKey: false, unique: false }, } + const notNullNullDefault = { + kind: 'addColumn' as const, + table: 't', + column: { + name: 'n', + kind: 'text' as const, + nullable: false, + primaryKey: false, + unique: false, + defaultSql: 'NULL', + }, + } expect(() => compileOps([notNullAdd], 'sqlite')).toThrow(/NOT NULL/) + expect(() => compileOps([notNullNullDefault], 'sqlite')).toThrow(/NOT NULL/) + expect(() => + compileOps([{ ...notNullNullDefault, column: { ...notNullNullDefault.column, defaultSql: 'null' } }], 'sqlite'), + ).toThrow(/non-NULL DEFAULT/) expect(compileOps([notNullAdd], 'postgres')[0]).toContain('NOT NULL') + expect(compileOps([notNullNullDefault], 'postgres')[0]).toContain('DEFAULT NULL') + + const createdNnNull = defineSchema({ + t: table({ id: col.integer().primaryKey(), n: col.text().defaultSql('NULL') }), + }) + const createdNnNullSql = compileOps(diffSchema(empty, createdNnNull), 'sqlite')[0] + expect(createdNnNullSql).toContain('"n" TEXT NOT NULL') + expect(createdNnNullSql).toContain('DEFAULT NULL') const withNote = defineSchema({ items: table({ id: col.integer().primaryKey(), note: col.text().nullable() }), @@ -172,6 +196,9 @@ test('sqlite drops indexes before columns and rejects unsafe ALTER TABLE', () => expect(() => m.migrateUp([...dropLabel, { id: '003_pk', up: diffSchema(pkTable, noPk), down: [] }])).toThrow(/PRIMARY KEY/) expect(() => m.migrateUp([...dropLabel, { id: '003_fk', up: diffSchema(withFk, noFk), down: [] }])).toThrow(/foreign key/) expect(() => m.migrateUp([...dropLabel, { id: '003_nn', up: [notNullAdd], down: [] }])).toThrow(/NOT NULL/) + expect(() => m.migrateUp([...dropLabel, { id: '003_nn_null', up: [notNullNullDefault], down: [] }])).toThrow( + /NOT NULL/, + ) const noteChain = [...dropLabel, migration('003_note', stripped, withNote)] expect(m.migrateUp(noteChain)).toEqual(['003_note']) expect(session.all(`SELECT "note" FROM "items"`)).toEqual([{ note: null }]) @@ -180,6 +207,16 @@ test('sqlite drops indexes before columns and rejects unsafe ALTER TABLE', () => } finally { close() } + + const created = sqlite() + try { + const m = new Migrator(created.session, 'sqlite') + expect(m.migrateUp([migration('001_nn_null', empty, createdNnNull)])).toEqual(['001_nn_null']) + created.session.exec(`INSERT INTO "t" ("id", "n") VALUES (1, 'ok')`) + expect(created.session.all(`SELECT "id", "n" FROM "t"`)).toEqual([{ id: 1, n: 'ok' }]) + } finally { + created.close() + } }) test('migrator applies, skips, rolls back, and walks down against sqlite', () => {