Skip to content

db: migration column-collision detection goes blind on any table rebuilt with `ALTER TABLE … #9647

Description

@JSONbored

⚠️ Definition of Done: this issue must be completed in full, in a single PR. Do not split this
work across multiple PRs, and do not defer any Deliverable below to a follow-up issue. A PR that
satisfies only some of the Deliverables, stubs a required test, or leaves a checkbox
partially-done does NOT resolve this issue and will be closed.

Context

detectColumnCollisions (src/db/migration-column-extraction.ts:211) replays every migration's schema events
in order and reports any (table, column) pair defined by more than one file. It is the only pre-merge gate for
the failure #2551 exists to catch: two independently-numbered migrations that each add the same column, which
produces no git conflict, a clean mergeable_state, and a hard failure at wrangler d1 migrations apply time
— after merge.

extractSchemaEvents (line 161) recognizes five statement shapes. It does not recognize
ALTER TABLE <old> RENAME TO <new>. The renameColumnMatch regex on line 166 requires a literal TO after the
captured identifier, so ALTER TABLE orb_signals_new RENAME TO orb_signals matches nothing and the statement
yields zero events.

That statement is not hypothetical — it is the repo's established pattern for widening a CHECK
constraint, which SQLite cannot ALTER. Two migrations already use it:

  • migrations/0176_orb_signals_superseded_reversal.sql:33ALTER TABLE orb_signals_new RENAME TO orb_signals;
  • migrations/0201_ledger_anchor_bittensor.sql:24ALTER TABLE decision_ledger_anchors_new RENAME TO decision_ledger_anchors; (its own comment calls this "the standard rebuild-and-rename")

Replaying either file leaves the tracker in a wrong state: the DROP TABLE orb_signals event clears every
column tracked for orb_signals, the CREATE TABLE orb_signals_new event tracks the columns under
orb_signals_new, and the rename is invisible — so from that file onward the checker believes orb_signals
and decision_ledger_anchors have no columns at all.

Verified by executing the module directly:

files: [0195 CREATE TABLE t(id, backend)], [0201 CREATE t_new(id, backend); DROP TABLE t; ALTER TABLE t_new RENAME TO t], [0202 ALTER TABLE t ADD COLUMN backend]
  detectColumnCollisions(...) -> []            <-- collision NOT detected
same corpus without the rebuild-and-rename file:
  detectColumnCollisions(...) -> [{table:"t", column:"backend", files:["0195_a.sql","0202_c.sql"]}]

So a future migration that re-adds an existing column to orb_signals or decision_ledger_anchors ships
green and breaks the deploy — the exact regression class this module exists to prevent, and the same family as
the already-fixed #8368 (RENAME COLUMN/DROP COLUMN without the literal COLUMN keyword).

scripts/check-migrations.ts imports this module directly (line 34) and so does the live premerge recheck, so
both are blind in the same way.

Requirements

  • SchemaEvent gains a { type: "rename_table"; from: string; to: string } variant.
  • extractSchemaEvents emits that event for ALTER TABLE <old> RENAME TO <new> (case-insensitive, tolerant of
    arbitrary whitespace), and continues to emit the existing remove_column + define_column pair for
    ALTER TABLE <t> RENAME [COLUMN] <a> TO <b>. The table-rename form must not be misparsed as a column rename,
    and the column-rename form must not be misparsed as a table rename.
  • detectColumnCollisions handles rename_table by re-keying every column currently tracked under from to
    to (dropping anything already tracked under to, since the rename target was just dropped by the
    preceding DROP TABLE in this pattern), so the rebuilt table carries its column set forward.
  • Already-recorded collisions stay recorded permanently, exactly as documented at lines 205-209 — a rename must
    not clear a collision that was already detected.

⚠️ Required pattern: mirror the existing drop_table handling in detectColumnCollisions
(src/db/migration-column-extraction.ts:218-222) — iterate tracked and rewrite matching entries, rather
than introducing a second parallel table-alias map consulted at lookup time. What does NOT satisfy this
issue: special-casing the _new suffix (nothing requires that naming); making check-migrations.ts
skip files containing RENAME TO; or adding the rename handling only in scripts/check-migrations.ts
instead of in the shared pure module (check-migrations.ts's own header, lines 9-13, states that CI and the
Worker must never disagree about what counts as a collision).

Deliverables

  • extractSchemaEvents in src/db/migration-column-extraction.ts returns a single rename_table event
    for ALTER TABLE a RENAME TO b, asserted by a new case in test/unit/ covering
    migrations/0201_ledger_anchor_bittensor.sql's exact statement text.
  • extractSchemaEvents still returns remove_column + define_column for both
    ALTER TABLE t RENAME COLUMN a TO b and ALTER TABLE t RENAME a TO b, asserted by existing-or-new cases
    so the two forms are provably disambiguated.
  • detectColumnCollisions re-keys tracked columns across a rename_table, asserted by a named regression
    test using the three-file fixture above (CREATE t → rebuild-and-rename → ALTER TABLE t ADD COLUMN)
    that expects exactly one collision on t.backend.
  • An invariant test asserting the real migrations/ corpus still reports zero collisions after the change
    (so the fix does not turn 0176/0201 themselves into false positives).

All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example adding
the rename_table event in extractSchemaEvents without the re-keying in detectColumnCollisions, so the
detector is still blind — does not resolve this issue.

Test Coverage Requirements

This repo enforces 99%+ Codecov patch coverage, branch-counted, on src/**, and
src/db/migration-column-extraction.ts is inside coverage.include. Every new regex branch and every arm of
the new rename_table handling needs a test, including the negative arms (a column-rename statement must NOT
produce a rename_table event, and a rename_table for an untracked table must be a harmless no-op). The
three-file fixture test in Deliverable 3 is the required named regression test.

Expected Outcome

A migration that re-adds a column to a table previously rebuilt via DROP TABLE + CREATE … _new +
ALTER TABLE … RENAME TO is caught by npm run db:migrations:check pre-merge, instead of passing CI and
failing at wrangler d1 migrations apply after merge.

Links & Resources

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is needed

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions