diff --git a/CHANGELOG.md b/CHANGELOG.md index b89f4e30a..a3e3cc62e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -99,6 +99,26 @@ existing `--allow drop-table` policy, so nothing drops without explicit opt-in. Previously-generated broken `*.queries.ts` / `*.routes.ts` files are **not** auto-pruned (`meta gen` never deletes existing files) — remove them by hand. +### Fixed — migrate emits constraint/index DROPs before DROP COLUMN so referenced-column drops apply (#255) + +**npm-only** (`migrate-ts`; PyPI / NuGet / Maven Central unchanged — schema migrations are +TS-owned, ADR-0015). The SQL emitter's stage ordering ran `DROP COLUMN` before `DROP CONSTRAINT` +(foreign key / check) and `DROP INDEX`, so dropping a column that a still-present foreign key +referenced — or that a still-present index backed — produced an **un-appliable** migration +(`cannot drop column … because other objects depend on it`). Both the Postgres and SQLite +emitters now hoist every constraint/index drop ahead of column mutation: `drop-fk`/`drop-check` +first, then `drop-index` (a foreign key depends on the unique/PK index backing its target, so the +FK must be dropped before that index), then the column ops; the matching adds +(`add-fk`/`add-check`/`add-index`) stay after column mutation (they reference columns that must +already exist). Reproduced against a real Postgres before fixing (emit → apply → introspect → +re-diff-empty), including the combined FK + backing-index + column-drop case. + +Byte-identical for any migration that contains none of `drop-fk` / `drop-check` / `drop-index`. A +migration that combines one of those drops with a column change now emits the same statements in +the corrected order — **re-review any committed-but-not-yet-applied migration file** that drops a +foreign key / check / index alongside a column, since its statement order changes (and will now +apply where it previously failed). + ## [0.20.9] — 2026-07-28 **npm-only** — `migrate-ts` + `codegen-ts` (schema migrations and projection-view codegen diff --git a/server/typescript/packages/migrate-ts/src/emit/postgres.ts b/server/typescript/packages/migrate-ts/src/emit/postgres.ts index 3f3086c7a..a4db63781 100644 --- a/server/typescript/packages/migrate-ts/src/emit/postgres.ts +++ b/server/typescript/packages/migrate-ts/src/emit/postgres.ts @@ -7,18 +7,39 @@ import { DEFAULT_DB_SCHEMA_POSTGRES } from "@metaobjectsdev/metadata"; import { renderFingerprintMarker, viewFingerprint } from "../view-fingerprint.js"; import { viewReplaceIsLegal } from "../view-column-types.js"; -// Stages run low → high. drop-view + drop-fk run BEFORE drop-table so a view -// that depends on a soon-to-be-dropped table is removed first. create-view -// runs AFTER add-fk so the view can reference the new schema in full. +// Stages run low → high. drop-view runs BEFORE drop-table so a view that +// depends on a soon-to-be-dropped table is removed first. create-view runs +// AFTER add-fk so the view can reference the new schema in full. +// +// #255: a constraint/index DROP and its ADD counterpart share the same kind +// but need OPPOSITE ordering relative to column mutation — a drop must run +// BEFORE the column change (the constraint/index must be gone before its +// column is dropped, or Postgres refuses `DROP COLUMN` with "other objects +// depend on it" — this applies just as much to an index backing a UNIQUE/FK +// target as it does to the FK/CHECK constraint itself), while an add must run +// AFTER (the column it references must already exist). One stage can't +// satisfy both, so ALL drops — drop-fk/drop-check/drop-index — are hoisted +// ahead of any column mutation; their ADD counterparts (add-fk/add-check/ +// add-index) stay at their later stages. +// +// Within that "drops" group, drop-fk/drop-check must ALSO run BEFORE +// drop-index: an FK constraint depends on the unique/PK index backing its +// target column (that's how Postgres enforces the target must be unique), so +// dropping the index first fails with the same "other objects depend on it" +// class of error, one level removed — "constraint … depends on index …". +// drop-index therefore gets its own stage (1.5) strictly between drop-fk/ +// drop-check/create-table (1) and column mutation (2). const STAGE_ORDER: Record = { "drop-view": 0, + "drop-fk": 1, "drop-check": 1, "create-table": 1, + "drop-index": 1.5, "add-column": 2, "drop-column": 2, "change-column-type": 2, "change-column-nullable": 2, "change-column-default": 2, "rename-column": 3, "rename-table": 3, - "add-index": 4, "drop-index": 4, - "add-fk": 5, "drop-fk": 5, - "add-check": 5, "drop-check": 5, + "add-index": 4, + "add-fk": 5, + "add-check": 5, "drop-table": 6, "create-view": 7, "replace-view": 7, }; diff --git a/server/typescript/packages/migrate-ts/src/emit/sqlite.ts b/server/typescript/packages/migrate-ts/src/emit/sqlite.ts index 0b2d607f9..958dba81f 100644 --- a/server/typescript/packages/migrate-ts/src/emit/sqlite.ts +++ b/server/typescript/packages/migrate-ts/src/emit/sqlite.ts @@ -8,19 +8,45 @@ export interface CarryColumns { insertCols: string[]; selectCols: string[]; } // Stage ordering similar to PG; recreate-and-copy bundles get inserted // at their first triggering change's position in Task 23. +// +// #255: a constraint/index DROP and its ADD counterpart share the same kind +// but need OPPOSITE ordering relative to column mutation — a drop must run +// BEFORE the column change (the constraint/index must be gone before its +// column is dropped, or the DDL fails on the referenced-column dependency — +// this applies just as much to an index backing a UNIQUE/FK target as it does +// to the FK/CHECK constraint itself), while an add must run AFTER (the column +// it references must already exist). One stage can't satisfy both, so ALL +// drops — drop-fk/drop-check/drop-index — are hoisted ahead of any column +// mutation; their ADD counterparts (add-fk/add-check/add-index) stay at their +// later stages. +// +// Within that "drops" group, drop-fk/drop-check must ALSO run BEFORE +// drop-index — mirrors postgres.ts: an FK depends on the unique/PK index +// backing its target column, so dropping the index first can fail on that +// dependency, one level removed. drop-index gets its own stage (1.5) strictly +// between drop-fk/drop-check/create-table (1) and column mutation (2). +// (drop-fk/drop-check are always recreate-triggering on SQLite — see +// RECREATE_TRIGGERING_KINDS below — so this mainly orders a drop-fk's +// table-recreate ahead of a native drop-column on a DIFFERENT table it once +// referenced; within the SAME table's recreate bundle, tableChanges order +// doesn't affect the emitted recipe. drop-index is NOT recreate-triggering — +// it's a plain `DROP INDEX`, so this ordering directly controls emission +// order among native statements.) const STAGE_ORDER: Record = { // drop-view runs FIRST (mirrors postgres): a view that depends on a table about // to be recreated-and-copied must be dropped before the DROP TABLE / RENAME, or // SQLite's rename re-parses the dependent view and can error mid-recreate. The // diff's Pass 2c injects exactly this drop(before)/create(after) pair. "drop-view": 0, + "drop-fk": 1, "drop-check": 1, "create-table": 1, + "drop-index": 1.5, "add-column": 2, "drop-column": 2, "change-column-type": 2, "change-column-nullable": 2, "change-column-default": 2, "rename-column": 3, "rename-table": 3, - "add-index": 4, "drop-index": 4, - "add-fk": 5, "drop-fk": 5, - "add-check": 5, "drop-check": 5, + "add-index": 4, + "add-fk": 5, + "add-check": 5, "drop-table": 6, // create-view / replace-view run LAST — after every table change the view reads. "create-view": 99, "replace-view": 99, diff --git a/server/typescript/packages/migrate-ts/test/integration/pg-drop-fk-before-drop-column.test.ts b/server/typescript/packages/migrate-ts/test/integration/pg-drop-fk-before-drop-column.test.ts new file mode 100644 index 000000000..d1a876c42 --- /dev/null +++ b/server/typescript/packages/migrate-ts/test/integration/pg-drop-fk-before-drop-column.test.ts @@ -0,0 +1,208 @@ +/** + * Real-Postgres gate for #255 — "a table whose column is referenced by + * another table's FK, where the metadata drops both the FK and the column." + * + * Root cause: the emitter's STAGE_ORDER put `drop-column` (stage 2) before + * `drop-fk`/`drop-check`/`drop-index` (originally stage 5/5/4), so the + * emitted UP SQL issued `ALTER TABLE "programs" DROP COLUMN "code"` while + * `weeks_program_code_fk` — another table's FK constraint referencing that + * column — AND `uniqueCode` — the UNIQUE index backing that same column + * (Postgres requires an FK target to be unique) — both still existed. + * Postgres refuses: "cannot drop column code of table programs because other + * objects depend on it." The fix hoists ALL drops that can have an external + * dependent — `drop-fk` / `drop-check` / `drop-index` — ahead of column + * mutation, while their ADD counterparts (`add-fk` / `add-check` / + * `add-index`) stay at their later, post-column stage. Within that group, + * `drop-fk`/`drop-check` ALSO run before `drop-index`: the FK constraint + * itself depends on the unique index backing its target column, so dropping + * the index first fails one level removed ("cannot drop index … because + * other objects depend on it" / "constraint … depends on index …"). This + * test proves the reordered SQL actually applies against a real engine, for + * the COMBINED scenario (a column that is BOTH FK-referenced AND + * index-backed, with the metadata dropping the FK, the index, and the column + * all in one change-set) — a unit assertion on statement order alone is not + * sufficient evidence; see emit-postgres.test.ts / emit-sqlite.test.ts for + * that half. + * + * Gated on MIGRATE_TS_PG_URL, like every other pg integration test in this + * package. Skips cleanly when unset. + */ + +import { test, expect, beforeAll, afterAll, describe } from "bun:test"; +import { Pool } from "pg"; +import { Kysely, PostgresDialect, sql } from "kysely"; +import { MetaDataLoader, InMemoryStringSource } from "@metaobjectsdev/metadata"; +import { buildExpectedSchema } from "../../src/expected-schema.js"; +import { introspectPostgres } from "../../src/introspect/postgres.js"; +import { diff } from "../../src/diff/index.js"; +import { emit } from "../../src/emit/index.js"; + +const PG_URL = process.env["MIGRATE_TS_PG_URL"]; +const realDescribe = PG_URL ? describe : describe.skip; + +// V1: Program (id PK, code UNIQUE) + Week (id PK, programCode FK -> Program.code). +const V1 = JSON.stringify({ + "metadata.root": { + package: "acme", + children: [ + { + "object.entity": { + name: "Program", + children: [ + { "source.rdb": {} }, + { "field.long": { name: "id" } }, + { "field.string": { name: "code", "@required": true } }, + { "identity.primary": { name: "pk", "@fields": ["id"], "@generation": "increment" } }, + { "identity.secondary": { name: "uniqueCode", "@fields": ["code"] } }, + ], + }, + }, + { + "object.entity": { + name: "Week", + children: [ + { "source.rdb": {} }, + { "field.long": { name: "id" } }, + { "field.string": { name: "programCode", "@required": true } }, + { "identity.primary": { name: "pk", "@fields": ["id"], "@generation": "increment" } }, + { "identity.reference": { name: "ref_program", "@fields": ["programCode"], "@references": "Program.code" } }, + ], + }, + }, + ], + }, +}); + +// V2: Program drops `code` (+ its identity.secondary index); Week drops the +// FK (identity.reference) but keeps its own `programCode` column (now a +// plain, un-constrained column) — the exact shape that produces a drop-fk + +// drop-index + drop-column TRIPLE where the dropped column is both the FK's +// REFERENCED side and the index's target. +const V2 = JSON.stringify({ + "metadata.root": { + package: "acme", + children: [ + { + "object.entity": { + name: "Program", + children: [ + { "source.rdb": {} }, + { "field.long": { name: "id" } }, + { "identity.primary": { name: "pk", "@fields": ["id"], "@generation": "increment" } }, + ], + }, + }, + { + "object.entity": { + name: "Week", + children: [ + { "source.rdb": {} }, + { "field.long": { name: "id" } }, + { "field.string": { name: "programCode", "@required": true } }, + { "identity.primary": { name: "pk", "@fields": ["id"], "@generation": "increment" } }, + ], + }, + }, + ], + }, +}); + +let k: Kysely>; +let pool: Pool; + +if (PG_URL) { + beforeAll(() => { + pool = new Pool({ connectionString: PG_URL }); + k = new Kysely>({ dialect: new PostgresDialect({ pool }) }); + }); + afterAll(async () => { + await cleanup(); + await k.destroy(); + }); +} + +async function cleanup(): Promise { + await sql.raw(`DROP TABLE IF EXISTS "weeks" CASCADE`).execute(k); + await sql.raw(`DROP TABLE IF EXISTS "programs" CASCADE`).execute(k); +} + +async function applyRaw(sqlText: string): Promise { + for (const stmt of sqlText.split(";").map((s) => s.trim()).filter(Boolean)) { + await sql.raw(stmt).execute(k); + } +} + +async function loadRoot(json: string) { + return (await new MetaDataLoader().load([new InMemoryStringSource(json)])).root; +} + +realDescribe("PG #255 — drop-fk/drop-check/drop-index before drop-column applies cleanly", () => { + test("dropping an FK's referenced+indexed column, the FK, and the index together: emitted SQL applies, re-diff empty", async () => { + await cleanup(); + + // Establish v1 (Program + Week, FK referencing Program.code) against a real, empty DB. + const v1Root = await loadRoot(V1); + const expected1 = buildExpectedSchema(v1Root, { dialect: "postgres" }); + const actual0 = await introspectPostgres(k); + const initial = await diff({ expected: expected1, actual: actual0, dialect: "postgres" }); + expect(initial.blocked).toEqual([]); + const emit1 = emit(initial.changes, { dialect: "postgres", expectedSchema: expected1 }); + await applyRaw(emit1.up); + + // Sanity: the FK is live and both columns exist. + await sql.raw(`INSERT INTO "programs" ("id", "code") VALUES (1, 'P1')`).execute(k); + await sql.raw(`INSERT INTO "weeks" ("id", "program_code") VALUES (1, 'P1')`).execute(k); + + // Evolve to v2: drop Program.code (the FK's referenced + indexed + // column), the FK, AND the backing unique index, all in one change-set — + // the COMBINED scenario. Pre-fix, this fails on either the FK dependency + // or the auto-cascaded index (whichever the (buggy) order hits first); + // post-fix, both drops run before the column drop and it all applies. + const v2Root = await loadRoot(V2); + const expected2 = buildExpectedSchema(v2Root, { dialect: "postgres" }); + const actual1 = await introspectPostgres(k); + const evolve = await diff({ + expected: expected2, actual: actual1, dialect: "postgres", + allow: { dropColumn: true, dropFk: true, dropIndex: true }, + }); + expect(evolve.blocked).toEqual([]); + const kinds = evolve.changes.map((c) => c.kind).sort(); + expect(kinds).toEqual(["drop-column", "drop-fk", "drop-index"]); + + const emit2 = emit(evolve.changes, { dialect: "postgres", expectedSchema: expected2 }); + + // The fix under test: DROP CONSTRAINT must precede DROP INDEX (the FK + // depends on the index backing its target column) and both must precede + // DROP COLUMN, in the emitted UP SQL — or the applyRaw below fails + // against the real engine. + const idxDropConstraint = emit2.up.indexOf("DROP CONSTRAINT"); + const idxDropIndex = emit2.up.indexOf("DROP INDEX"); + const idxDropColumn = emit2.up.indexOf("DROP COLUMN"); + expect(idxDropConstraint).toBeGreaterThanOrEqual(0); + expect(idxDropIndex).toBeGreaterThanOrEqual(0); + expect(idxDropColumn).toBeGreaterThanOrEqual(0); + expect(idxDropConstraint).toBeLessThan(idxDropIndex); + expect(idxDropIndex).toBeLessThan(idxDropColumn); + + // The real-engine gate: pre-fix (drop-fk/drop-index tied at the same + // stage, both after drop-column, or drop-index ordered before drop-fk), + // this throws one of: + // `error: cannot drop column code of table programs because other + // objects depend on it` (FK still referencing the column) + // `error: index "uniqueCode" does not exist` (index auto-cascaded away + // by an earlier DROP COLUMN) + // `error: cannot drop index "uniqueCode" because other objects depend + // on it` / `constraint weeks_program_code_fk … depends on index …` + // (index dropped before the FK constraint that depends on it) + // Post-fix, it applies cleanly. + await applyRaw(emit2.up); + + // Idempotence: re-introspecting must show no drift against expected2. + const followup = await diff({ expected: expected2, actual: await introspectPostgres(k), dialect: "postgres" }); + if (followup.changes.length > 0) { + console.error("ROUND-TRIP FAILURE — a second `meta migrate` would emit:"); + for (const c of followup.changes) console.error(" -", JSON.stringify(c)); + } + expect(followup.changes).toEqual([]); + }); +}); diff --git a/server/typescript/packages/migrate-ts/test/unit/emit-postgres.test.ts b/server/typescript/packages/migrate-ts/test/unit/emit-postgres.test.ts index 8f2b903f6..cbe5b52ff 100644 --- a/server/typescript/packages/migrate-ts/test/unit/emit-postgres.test.ts +++ b/server/typescript/packages/migrate-ts/test/unit/emit-postgres.test.ts @@ -205,6 +205,108 @@ describe("renderPostgres — statement ordering", () => { expect(idxCreate).toBeLessThan(idxAdd); expect(idxAdd).toBeLessThan(idxDrop); }); + + // #255 — a DROP CONSTRAINT for an FK still referencing a column must run + // before that column's DROP COLUMN, or Postgres refuses at apply time with + // "cannot drop column … because other objects depend on it". + test("drop-fk runs before drop-column for the column it references (#255)", () => { + const changes: Change[] = [ + { kind: "drop-column", table: "weeks", column: "program_id", status: ALLOWED }, + { kind: "drop-fk", table: "weeks", fk: "weeks_program_id_fk", status: ALLOWED }, + ]; + const { up } = emit(changes, { dialect: "postgres" }); + const idxDropConstraint = up.indexOf("DROP CONSTRAINT"); + const idxDropColumn = up.indexOf("DROP COLUMN"); + expect(idxDropConstraint).toBeGreaterThanOrEqual(0); + expect(idxDropColumn).toBeGreaterThanOrEqual(0); + expect(idxDropConstraint).toBeLessThan(idxDropColumn); + }); + + // Same invariant for drop-check — a CHECK constraint on a column must be + // dropped before that column is dropped. + test("drop-check runs before drop-column for the column it constrains (#255)", () => { + const changes: Change[] = [ + { kind: "drop-column", table: "orders", column: "qty", status: ALLOWED }, + { kind: "drop-check", table: "orders", check: "orders_qty_chk", status: ALLOWED }, + ]; + const { up } = emit(changes, { dialect: "postgres" }); + const idxDropConstraint = up.indexOf("DROP CONSTRAINT"); + const idxDropColumn = up.indexOf("DROP COLUMN"); + expect(idxDropConstraint).toBeGreaterThanOrEqual(0); + expect(idxDropColumn).toBeGreaterThanOrEqual(0); + expect(idxDropConstraint).toBeLessThan(idxDropColumn); + }); + + // add-fk must still run AFTER column mutation (the referenced column must + // already exist) — only the DROP direction was hoisted. + test("add-fk still runs after add-column (adds are unaffected by #255)", () => { + const changes: Change[] = [ + { kind: "add-fk", table: "weeks", + fk: { name: "weeks_program_id_fk", columns: ["program_id"], refTable: "programs", refColumns: ["id"] }, + status: ALLOWED }, + { kind: "add-column", table: "weeks", column: { name: "program_id", sqlType: { kind: "integer", bits: 64 }, nullable: true }, status: ALLOWED }, + ]; + const { up } = emit(changes, { dialect: "postgres" }); + const idxAddColumn = up.indexOf("ADD COLUMN"); + const idxAddConstraint = up.indexOf("ADD CONSTRAINT"); + expect(idxAddColumn).toBeGreaterThanOrEqual(0); + expect(idxAddConstraint).toBeGreaterThanOrEqual(0); + expect(idxAddColumn).toBeLessThan(idxAddConstraint); + }); + + // #255 (generalized) — dropping a column that has a backing index (e.g. a + // UNIQUE index also targeted by another table's FK) auto-cascades that + // index away in Postgres, so a standalone DROP INDEX emitted AFTER the + // DROP COLUMN fails ("index … does not exist"). drop-index must run before + // drop-column, same as drop-fk/drop-check. + test("drop-index runs before drop-column for the column it indexes (#255)", () => { + const changes: Change[] = [ + { kind: "drop-column", table: "programs", column: "code", status: ALLOWED }, + { kind: "drop-index", table: "programs", index: "uniqueCode", status: ALLOWED }, + ]; + const { up } = emit(changes, { dialect: "postgres" }); + const idxDropIndex = up.indexOf("DROP INDEX"); + const idxDropColumn = up.indexOf("DROP COLUMN"); + expect(idxDropIndex).toBeGreaterThanOrEqual(0); + expect(idxDropColumn).toBeGreaterThanOrEqual(0); + expect(idxDropIndex).toBeLessThan(idxDropColumn); + }); + + // #255 (sub-ordering within the drops group) — an FK constraint depends on + // the unique/PK index backing its target column (that's how Postgres + // enforces the target must be unique), so dropping the index BEFORE the FK + // that depends on it fails: "cannot drop index … because other objects + // depend on it" / "constraint … depends on index …". drop-fk must run + // before drop-index, not just before drop-column. + test("drop-fk runs before drop-index that backs its target column (#255)", () => { + const changes: Change[] = [ + { kind: "drop-index", table: "programs", index: "uniqueCode", status: ALLOWED }, + { kind: "drop-fk", table: "weeks", fk: "weeks_program_code_fk", status: ALLOWED }, + ]; + const { up } = emit(changes, { dialect: "postgres" }); + const idxDropConstraint = up.indexOf("DROP CONSTRAINT"); + const idxDropIndex = up.indexOf("DROP INDEX"); + expect(idxDropConstraint).toBeGreaterThanOrEqual(0); + expect(idxDropIndex).toBeGreaterThanOrEqual(0); + expect(idxDropConstraint).toBeLessThan(idxDropIndex); + }); + + // add-index must still run AFTER column mutation (the indexed column must + // already exist) — only the DROP direction was hoisted. + test("add-index still runs after add-column (adds are unaffected by #255)", () => { + const changes: Change[] = [ + { kind: "add-index", table: "users", + index: { name: "users_phone_idx", columns: ["phone"], unique: false }, + status: ALLOWED }, + { kind: "add-column", table: "users", column: { name: "phone", sqlType: { kind: "text" }, nullable: true }, status: ALLOWED }, + ]; + const { up } = emit(changes, { dialect: "postgres" }); + const idxAddColumn = up.indexOf("ADD COLUMN"); + const idxCreateIndex = up.indexOf("CREATE INDEX"); + expect(idxAddColumn).toBeGreaterThanOrEqual(0); + expect(idxCreateIndex).toBeGreaterThanOrEqual(0); + expect(idxAddColumn).toBeLessThan(idxCreateIndex); + }); }); describe("renderPostgres — down statements", () => { diff --git a/server/typescript/packages/migrate-ts/test/unit/emit-sqlite.test.ts b/server/typescript/packages/migrate-ts/test/unit/emit-sqlite.test.ts index 231b9c5f4..0a20d9b8f 100644 --- a/server/typescript/packages/migrate-ts/test/unit/emit-sqlite.test.ts +++ b/server/typescript/packages/migrate-ts/test/unit/emit-sqlite.test.ts @@ -285,4 +285,102 @@ describe("renderSqlite — add-fk / drop-fk via recreate", () => { expect(up).toContain('CREATE TABLE "__new_weeks"'); expect(up).not.toContain('REFERENCES'); }); + + // #255 — drop-fk + drop-column on the SAME table both fold into ONE + // recreate-and-copy bundle regardless of STAGE_ORDER (bundling is by table, + // not by kind order), so the correct outcome — recreated table missing both + // the FK and the dropped column — already held pre-fix here. This pins that + // invariant so a future bundling change can't silently regress it. + test("drop-fk + drop-column on the same table: recreated table has neither (#255)", () => { + const newCols: ColumnDescriptor[] = [ + { name: "id", sqlType: { kind: "integer", bits: 64 }, nullable: false, identity: "increment" }, + ]; + const expectedSchema: SchemaSnapshot = { + tables: [table("weeks", newCols, ["id"])], + views: [], + }; + const { up } = emit( + [ + { kind: "drop-column", table: "weeks", column: "program_id", status: ALLOWED }, + { kind: "drop-fk", table: "weeks", fk: "weeks_program_id_fk", status: ALLOWED }, + ], + { dialect: "sqlite", expectedSchema }, + ); + expect(up).toContain('CREATE TABLE "__new_weeks"'); + expect(up).not.toContain("REFERENCES"); + expect(up).not.toContain("program_id"); + }); + + // #255 — the STAGE_ORDER-observable case for SQLite: a drop-fk on table B + // (always recreate-triggering) must sequence BEFORE a native drop-column on + // an unrelated table A that the dropped FK used to reference. Pre-fix, + // drop-column (stage 2) sorted before drop-fk (stage 5), so table A's + // column drop ran while table B's schema still declared the FK against it. + test("drop-fk on one table runs before a native drop-column on another table it referenced (#255)", () => { + const newProgramsCols: ColumnDescriptor[] = [ + { name: "id", sqlType: { kind: "integer", bits: 64 }, nullable: false, identity: "increment" }, + ]; + const newWeeksCols: ColumnDescriptor[] = [ + { name: "id", sqlType: { kind: "integer", bits: 64 }, nullable: false, identity: "increment" }, + ]; + const expectedSchema: SchemaSnapshot = { + tables: [ + table("programs", newProgramsCols, ["id"]), + table("weeks", newWeeksCols, ["id"]), + ], + views: [], + }; + const { up } = emit( + [ + // drop-column on "programs" (native — not recreate-triggering on modern SQLite) + { kind: "drop-column", table: "programs", column: "code", status: ALLOWED }, + // drop-fk on "weeks" (always recreate-triggering) + { kind: "drop-fk", table: "weeks", fk: "weeks_program_code_fk", status: ALLOWED }, + ], + { dialect: "sqlite", expectedSchema }, + ); + const idxWeeksRecreate = up.indexOf('CREATE TABLE "__new_weeks"'); + const idxProgramsDropColumn = up.indexOf('ALTER TABLE "programs" DROP COLUMN "code"'); + expect(idxWeeksRecreate).toBeGreaterThanOrEqual(0); + expect(idxProgramsDropColumn).toBeGreaterThanOrEqual(0); + expect(idxWeeksRecreate).toBeLessThan(idxProgramsDropColumn); + }); +}); + +describe("renderSqlite — drop-index vs drop-column (#255 generalized)", () => { + // Unlike drop-fk/drop-check, drop-index is NOT recreate-triggering on + // SQLite — it's a plain native `DROP INDEX` — so, unlike the drop-fk case + // above, the SAME-table ordering IS directly observable here as two + // separate emitted statements. + test("drop-index runs before drop-column for the column it indexes, same table", () => { + const { up } = emit( + [ + { kind: "drop-column", table: "programs", column: "code", status: ALLOWED }, + { kind: "drop-index", table: "programs", index: "uniqueCode", status: ALLOWED }, + ], + { dialect: "sqlite" }, + ); + const idxDropIndex = up.indexOf('DROP INDEX "uniqueCode"'); + const idxDropColumn = up.indexOf('ALTER TABLE "programs" DROP COLUMN "code"'); + expect(idxDropIndex).toBeGreaterThanOrEqual(0); + expect(idxDropColumn).toBeGreaterThanOrEqual(0); + expect(idxDropIndex).toBeLessThan(idxDropColumn); + }); + + // add-index must still run AFTER column mutation — only the DROP direction + // was hoisted. + test("add-index still runs after add-column (adds are unaffected)", () => { + const { up } = emit( + [ + { kind: "add-index", table: "users", index: { name: "users_phone_idx", columns: ["phone"], unique: false }, status: ALLOWED }, + { kind: "add-column", table: "users", column: { name: "phone", sqlType: { kind: "text" }, nullable: true }, status: ALLOWED }, + ], + { dialect: "sqlite" }, + ); + const idxAddColumn = up.indexOf('ADD COLUMN "phone"'); + const idxCreateIndex = up.indexOf("CREATE INDEX"); + expect(idxAddColumn).toBeGreaterThanOrEqual(0); + expect(idxCreateIndex).toBeGreaterThanOrEqual(0); + expect(idxAddColumn).toBeLessThan(idxCreateIndex); + }); });