Skip to content

Commit ea545a4

Browse files
dmealingclaude
andcommitted
fix(migrate-ts): review round 1 — comment/test-name accuracy, dialect arg
Three one-line fixes from review of the serial-identity default-diff fix: - diff/index.ts: the replacement comment inverted its own causal clause ("Left undiffed, that surfaced as ... DROP DEFAULT" claims skipping the diff produced the bug -- the exact inverse; the bug was the case being left DIFFED). Fixed to "Left diffed". - diff-uuid-identity-default.test.ts: the increment-PK regression-guard test name asserted "it never had a DEFAULT", the same false doctrine this change disproved. Renamed to "(no live default at all)". - diff-serial-identity-default.test.ts: the new unit tests called diff() with no `dialect` for a Postgres-only bug. This repo's standing rule is that omitting `dialect` runs a different pipeline -- pass it always. Both diff() call sites now pass `dialect: "postgres"`, matching the integration test. No production behavior change beyond the comment; the two test edits are name/argument-only and do not change what either test asserts. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015TqsuDye2SfXGf43vuoD3n
1 parent 3a3317a commit ea545a4

3 files changed

Lines changed: 4 additions & 4 deletions

File tree

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -398,7 +398,7 @@ function diffTableColumns(
398398
// but a legacy Postgres `serial`/`bigserial` column is historical sugar for
399399
// `integer` + a sequence + a REAL `DEFAULT nextval(...)` clause — introspection
400400
// correctly reads that back as a live default even though the expected side
401-
// correctly declares none. Left undiffed, that surfaced as
401+
// correctly declares none. Left diffed, that surfaced as
402402
// `ALTER COLUMN … DROP DEFAULT` with no replacement generation mechanism —
403403
// destructive against a live table, since every insert that doesn't supply the
404404
// PK explicitly then starts failing. So an `increment` PK skips the default-diff

server/typescript/packages/migrate-ts/test/unit/diff-serial-identity-default.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ const expectedIncrementPk: ColumnDescriptor = {
3737
};
3838

3939
async function defaultChanges(expected: ColumnDescriptor, actual: ColumnDescriptor) {
40-
const r = await diff(snap(expected), snap(actual));
40+
const r = await diff(snap(expected), snap(actual), { dialect: "postgres" });
4141
return r.changes.filter((c) => c.kind === "change-column-default");
4242
}
4343

@@ -56,7 +56,7 @@ describe("diff — legacy Postgres serial PK default is not diffed as drift", ()
5656
...expectedIncrementPk,
5757
default: { kind: "expr", value: "nextval('work_item_id_seq'::regclass)" },
5858
};
59-
const r = await diff(snap(expectedIncrementPk), snap(actual));
59+
const r = await diff(snap(expectedIncrementPk), snap(actual), { dialect: "postgres" });
6060
expect(r.changes).toEqual([]);
6161
});
6262

server/typescript/packages/migrate-ts/test/unit/diff-uuid-identity-default.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ describe("diff — uuid-identity PK default is not diffed", () => {
5353
expect(r.changes).toEqual([]);
5454
});
5555

56-
test("regression guard: increment-identity PK still no-ops (it never had a DEFAULT)", async () => {
56+
test("regression guard: increment-identity PK still no-ops (no live default at all)", async () => {
5757
const inc: ColumnDescriptor = {
5858
name: "id", sqlType: { kind: "integer", bits: 64 }, nullable: false, identity: "increment",
5959
};

0 commit comments

Comments
 (0)