From 6c5e00d8cb1bf9b691bf98f27b49605fc9882c9b Mon Sep 17 00:00:00 2001 From: shin-core <153108882+shin-core@users.noreply.github.com> Date: Wed, 29 Jul 2026 22:17:30 +0900 Subject: [PATCH] =?UTF-8?q?fix(db):=20track=20ALTER=20TABLE=20=E2=80=A6=20?= =?UTF-8?q?RENAME=20TO=20so=20column-collision=20detection=20survives=20a?= =?UTF-8?q?=20rebuild?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `detectColumnCollisions` replays every migration's schema events to catch a same-table/same-column definition across two files. But `extractSchemaEvents` emitted nothing for `ALTER TABLE RENAME TO ` — the standard SQLite rebuild-and-rename (migrations/0201). So every column tracked under the temporary `_new` table stayed keyed to it, and the detector went blind on the real table: a later duplicate ADD COLUMN read as a fresh, non-colliding definition. Add a `rename_table` SchemaEvent and parse the RENAME TO form — matched before the column-rename rule and requiring `RENAME` immediately followed by `TO`, which the column form (an identifier between RENAME and TO) never has, so the two are disambiguated both directions. `detectColumnCollisions` handles it by re-keying every column tracked under the old table name to the new one (and vacating the old name, so a fresh table reusing it does not falsely collide). Closes #9647 --- src/db/migration-column-extraction.ts | 19 ++++++++++ test/unit/migration-column-extraction.test.ts | 37 +++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/src/db/migration-column-extraction.ts b/src/db/migration-column-extraction.ts index 0e3c96006d..14c0c4a12c 100644 --- a/src/db/migration-column-extraction.ts +++ b/src/db/migration-column-extraction.ts @@ -154,6 +154,7 @@ function stripSqlComments(text: string): string { export type SchemaEvent = | { type: "define_column"; table: string; column: string } | { type: "drop_table"; table: string } + | { type: "rename_table"; from: string; to: string } | { type: "remove_column"; table: string; column: string }; /** Extract the schema-affecting events a single SQL statement produces. Statements that don't affect table @@ -163,6 +164,13 @@ export function extractSchemaEvents(rawStatement: string): SchemaEvent[] { const dropTableMatch = /^\s*DROP\s+TABLE\s+(?:IF\s+EXISTS\s+)?(\w+)/i.exec(statement); if (dropTableMatch) return [{ type: "drop_table", table: dropTableMatch[1]!.toLowerCase() }]; + // `ALTER TABLE RENAME TO ` — a whole-table rebuild: every column tracked under must move to + // so a collision detector does not go blind on the renamed table (#9647). Checked BEFORE the column + // rename below: `RENAME\s+TO` requires TO immediately after RENAME, which the column form (RENAME [COLUMN] + // TO , an identifier between RENAME and TO) never has, so the two can't be confused either direction. + const renameTableMatch = /\bALTER\s+TABLE\s+(\w+)\s+RENAME\s+TO\s+(\w+)/i.exec(statement); + if (renameTableMatch) return [{ type: "rename_table", from: renameTableMatch[1]!.toLowerCase(), to: renameTableMatch[2]!.toLowerCase() }]; + const renameColumnMatch = /\bALTER\s+TABLE\s+(\w+)\s+RENAME\s+(?:COLUMN\s+)?(\w+)\s+TO\s+(\w+)/i.exec(statement); if (renameColumnMatch) { const table = renameColumnMatch[1]!.toLowerCase(); @@ -221,6 +229,17 @@ export function detectColumnCollisions(orderedFileContents: ReadonlyArray { ]); }); + it("extracts ALTER TABLE … RENAME TO as a single rename_table event, disambiguated from column rename (#9647)", () => { + // The exact rebuild-and-rename statement from migrations/0201_ledger_anchor_bittensor.sql. + expect(extractSchemaEvents("ALTER TABLE decision_ledger_anchors_new RENAME TO decision_ledger_anchors;")).toEqual([ + { type: "rename_table", from: "decision_ledger_anchors_new", to: "decision_ledger_anchors" }, + ]); + // The column-rename forms (with and without the COLUMN keyword) must NOT be misparsed as a table rename — + // they still produce the remove+define pair. + expect(extractSchemaEvents("ALTER TABLE widgets RENAME color TO hue;")).toEqual([ + { type: "remove_column", table: "widgets", column: "color" }, + { type: "define_column", table: "widgets", column: "hue" }, + ]); + // And a table named like the "to" keyword must not confuse either regex. + expect(extractSchemaEvents("ALTER TABLE t RENAME TO t2;")).toEqual([{ type: "rename_table", from: "t", to: "t2" }]); + }); + it("extracts ADD COLUMN's terser SQLite form that omits the COLUMN keyword (#8368)", () => { expect(extractSchemaEvents("ALTER TABLE widgets ADD color TEXT;")).toEqual([{ type: "define_column", table: "widgets", column: "color" }]); }); @@ -210,6 +225,28 @@ describe("detectColumnCollisions (#2551)", () => { expect(detectColumnCollisions(files)).toEqual([]); }); + it("REGRESSION: re-keys columns across a rebuild-and-rename so a later duplicate is still caught (#9647)", () => { + // The standard SQLite rebuild-and-rename (as migrations/0201 does): a new table is built with `backend`, + // then RENAMEd over the old one. Before #9647 the detector tracked the column under the *_new name and + // went blind — a later ADD COLUMN backend on the real table read as a fresh, non-colliding definition. + const files: Array<[string, string]> = [ + ["0001_a.sql", "CREATE TABLE t (id INTEGER, backend TEXT);"], + ["0002_b.sql", "CREATE TABLE t_new (id INTEGER, backend TEXT); ALTER TABLE t_new RENAME TO t;"], + ["0003_c.sql", "ALTER TABLE t ADD COLUMN backend TEXT;"], + ]; + // Exactly one collision on t.backend — the re-keyed column from 0002's renamed table collides with 0003's. + expect(detectColumnCollisions(files)).toEqual([{ table: "t", column: "backend", files: ["0002_b.sql", "0003_c.sql"] }]); + }); + + it("does not flag the renamed table's columns against the now-vacated old name (#9647)", () => { + const files: Array<[string, string]> = [ + ["0001_a.sql", "CREATE TABLE t_new (id INTEGER, c TEXT); ALTER TABLE t_new RENAME TO t;"], + // A brand-new table reusing the OLD (vacated) name must not collide with the columns that moved off it. + ["0002_b.sql", "CREATE TABLE t_new (id INTEGER, c TEXT);"], + ]; + expect(detectColumnCollisions(files)).toEqual([]); + }); + it("returns [] for an empty file list", () => { expect(detectColumnCollisions([])).toEqual([]); });