Skip to content

Commit d4b3db6

Browse files
dmealingclaude
andcommitted
feat(migrate-ts): diffTableChecks — evolve CHECKs on existing PG tables (offline)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent e8e5ab8 commit d4b3db6

3 files changed

Lines changed: 73 additions & 5 deletions

File tree

server/typescript/packages/migrate-ts/src/diff/index.ts

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
import type {
22
SchemaSnapshot, TableDescriptor, ColumnDescriptor, IndexDescriptor, FkDescriptor,
33
ViewDescriptor,
4-
Change, ChangeStatus, DiffResult, AllowOptions, AmbiguousCallback,
4+
Change, ChangeStatus, DiffResult, AllowOptions, AmbiguousCallback, Dialect,
55
} from "../types.js";
66
import type { SqlType } from "../sql-type.js";
77
import { sqlTypeEquals } from "../sql-type.js";
88
import { applyStatus } from "./status.js";
99
import { detectColumnRenames, detectTableRenames } from "./rename-heuristic.js";
1010
import { viewSqlEquals } from "../view-sql-compare.js";
11+
import { checkExprEquals } from "../check-expr-compare.js";
1112
import { DEFAULT_DB_SCHEMA_POSTGRES } from "@metaobjectsdev/metadata";
1213

1314
export interface DiffArgs {
@@ -26,6 +27,8 @@ export interface DiffArgs {
2627
* the default. Pass additional patterns to extend.
2728
*/
2829
ignoreTables?: string[];
30+
/** Dialect; CHECK-constraint evolution on existing tables is emitted for postgres only. */
31+
dialect?: Dialect;
2932
}
3033

3134
const ALLOWED: ChangeStatus = { state: "allowed" };
@@ -156,10 +159,10 @@ export async function diff(
156159
diffTableColumns(expectedTable, actualTable, changes);
157160
diffTableIndexes(expectedTable, actualTable, changes);
158161
diffTableForeignKeys(expectedTable, actualTable, changes);
159-
// CHECK constraints on existing tables are intentionally NOT diffed:
160-
// checks are a create-time-only concern (inlined in CREATE TABLE). Evolving
161-
// an enum's values on a live table (which would alter its CHECK) is deferred
162-
// until check introspection lands. No add-check / drop-check is produced here.
162+
// CHECK constraints on existing tables are evolved for postgres only (SQLite
163+
// evolves checks via table recreate, not ALTER). Gated on `actual.checks`
164+
// being populated — by the snapshot offline, or pg_constraint introspection.
165+
if (args.dialect === "postgres") diffTableChecks(expectedTable, actualTable, changes);
163166
}
164167

165168
// Pass 2b: views. Identity is (schema, name). A name present on both sides
@@ -293,6 +296,26 @@ function diffTableForeignKeys(
293296
}
294297
}
295298

299+
function diffTableChecks(expected: TableDescriptor, actual: TableDescriptor, changes: Change[]): void {
300+
const sx = schemaSpread(expected.schema);
301+
const expectedChk = new Map(expected.checks.map((c) => [c.name, c]));
302+
const actualChk = new Map(actual.checks.map((c) => [c.name, c]));
303+
for (const [name, ec] of expectedChk) {
304+
const ac = actualChk.get(name);
305+
if (!ac) {
306+
changes.push({ kind: "add-check", table: expected.name, ...sx, check: ec, status: ALLOWED });
307+
} else if (!checkExprEquals(ec.expression, ac.expression)) {
308+
changes.push({ kind: "drop-check", table: expected.name, ...sx, check: name, restore: ac, status: ALLOWED });
309+
changes.push({ kind: "add-check", table: expected.name, ...sx, check: ec, status: ALLOWED });
310+
}
311+
}
312+
for (const [name, ac] of actualChk) {
313+
if (!expectedChk.has(name)) {
314+
changes.push({ kind: "drop-check", table: expected.name, ...sx, check: name, restore: ac, status: ALLOWED });
315+
}
316+
}
317+
}
318+
296319
function viewIdentity(v: { name: string; schema?: string }): string {
297320
return (v.schema ?? DEFAULT_DB_SCHEMA_POSTGRES) + "." + v.name;
298321
}

server/typescript/packages/migrate-ts/src/snapshot/plan.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ export async function planOffline(args: PlanOfflineArgs): Promise<PlanOfflineRes
3232
const result = await diff({
3333
expected: nextSnapshot,
3434
actual: args.snapshot,
35+
dialect: args.dialect,
3536
...(args.allow ? { allow: args.allow } : {}),
3637
...(args.onAmbiguous ? { onAmbiguous: args.onAmbiguous } : {}),
3738
...(args.ignoreTables ? { ignoreTables: args.ignoreTables } : {}),
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
import { describe, test, expect } from "bun:test";
2+
import type { SchemaSnapshot, TableDescriptor, CheckDescriptor } from "../../src/types.js";
3+
import { diff } from "../../src/diff/index.js";
4+
5+
const CHK = (name: string, expression: string): CheckDescriptor => ({ name, expression });
6+
function tbl(checks: CheckDescriptor[]): TableDescriptor {
7+
return { name: "orders", columns: [{ name: "qty", sqlType: { kind: "integer", bits: 32 }, nullable: false }],
8+
indexes: [], foreignKeys: [], checks, primaryKey: ["qty"] };
9+
}
10+
const snap = (checks: CheckDescriptor[]): SchemaSnapshot => ({ tables: [tbl(checks)], views: [] });
11+
const C0 = CHK("orders_qty_numeric_chk", "qty >= 1");
12+
const C0b = CHK("orders_qty_numeric_chk", "qty >= 5"); // changed bound, same name
13+
const C1 = CHK("orders_qty_max_chk", "qty <= 100");
14+
15+
describe("diffTableChecks — postgres existing-table evolution", () => {
16+
test("added check on an existing table → add-check", async () => {
17+
const r = await diff({ expected: snap([C0, C1]), actual: snap([C0]), dialect: "postgres" });
18+
expect(r.changes.filter((c) => c.kind === "add-check").map((c) => (c as any).check.name)).toEqual(["orders_qty_max_chk"]);
19+
});
20+
test("removed check → drop-check (gated, so blocked without allow)", async () => {
21+
const r = await diff({ expected: snap([C0]), actual: snap([C0, C1]), dialect: "postgres" });
22+
const drop = r.changes.find((c) => c.kind === "drop-check");
23+
expect(drop && (drop as any).check).toBe("orders_qty_max_chk");
24+
expect(drop!.status.state).toBe("blocked"); // no allow.dropCheck
25+
});
26+
test("changed expression (same name) → drop + add", async () => {
27+
const r = await diff({ expected: snap([C0b]), actual: snap([C0]), dialect: "postgres", allow: { dropCheck: true } });
28+
expect(r.changes.some((c) => c.kind === "drop-check")).toBe(true);
29+
expect(r.changes.some((c) => c.kind === "add-check" && (c as any).check.expression === "qty >= 5")).toBe(true);
30+
});
31+
test("PG-rewritten actual expression equal to expected → NO change (idempotent)", async () => {
32+
const r = await diff({ expected: snap([CHK("orders_qty_numeric_chk", "qty >= 1")]),
33+
actual: snap([CHK("orders_qty_numeric_chk", "(qty >= 1)")]), dialect: "postgres" });
34+
expect(r.changes.some((c) => c.kind.endsWith("-check"))).toBe(false);
35+
});
36+
test("dialect gate: sqlite produces NO check changes even when they differ", async () => {
37+
const r = await diff({ expected: snap([C0, C1]), actual: snap([C0]), dialect: "sqlite" });
38+
expect(r.changes.some((c) => c.kind.endsWith("-check"))).toBe(false);
39+
});
40+
test("no dialect passed → no check evolution (back-compat)", async () => {
41+
const r = await diff({ expected: snap([C0, C1]), actual: snap([C0]) });
42+
expect(r.changes.some((c) => c.kind.endsWith("-check"))).toBe(false);
43+
});
44+
});

0 commit comments

Comments
 (0)