Skip to content

fix(db): add the missing columns when a migration hits "duplicate column name" - #15236

Merged
diegosouzapw merged 1 commit into
cheaper-inference:release/v3.8.52from
maxmad64bis:fix/migration-duplicate-column
Oct 6, 2026
Merged

diegosouzapw merged 1 commit into
cheaper-inference:release/v3.8.52from
maxmad64bis:fix/migration-duplicate-column

Conversation

@maxmad64bis

@maxmad64bis maxmad64bis commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

⚠️ base-red inherited: #15306

Summary

  • When a migration file fails with "duplicate column name", the runner used to record the whole file as applied, so a column added later in that file was never created and the routes reading it returned a 500. The runner now replays the file without the ADD COLUMN statements whose column already exists, then records it. This is what fix(migrations): treat duplicate-column ALTER as no-op #1886 asked for (treat a duplicate ALTER as a no-op, per statement).

Related Issues

Validation

  • Change type: DB
  • Focused tests and category gates from the golden path
  • npm run lint — ESLint reports two unused-function errors in migrationRunner.ts (isSearchRequestTypeMigration, applySearchRequestTypeMigration) that are already on the base and not touched here; nothing new in the changed files
  • Reconciled with the current active release base; focused checks rerun afterward
  • Production-code changes include a new or updated automated test in this PR

Tests Added Or Updated

  • tests/unit/db-migrationrunner-duplicate-column.test.ts (new, 5 cases): a file whose first column already exists ends with all its columns; the CREATE TABLE and CREATE INDEX around the duplicate are kept; a second run changes nothing; the helper skips only existing columns, ignores commented-out statements and leaves statements on a table that doesn't exist yet. Four of the five cases fail without the change.
  • tests/unit/db-migrationrunner-duplicate-column-leftovers.test.ts (new, 2 cases, maintainer): a replay that fails on an existing table or index falls back to recording the ledger row only, and a second run applies nothing. Both fail without the change (assertion on the fallback helper).
  • Re-run unchanged and passing: tests/unit/db/migration-071.test.ts, tests/unit/db-server-tool-executions-migration.test.ts, tests/unit/memory-vec-meta.test.ts, tests/unit/db-migration-runner-extra-dirs.test.ts, tests/unit/db-migrationrunner-constants-split.test.ts, tests/unit/migration-135-numbering-collision.test.ts.

Coverage Notes

  • src/lib/db/migrationRunner/schemaState.ts: stripExistingAddColumns and applyMigrationSkippingExistingColumns are covered by db-migrationrunner-duplicate-column.test.ts (helper cases and the two end-to-end runs).
  • src/lib/db/migrationRunner.ts: the catch branch that now calls the helper is covered by the same test's end-to-end cases; the other paths keep their existing tests.

Reviewer Notes

  • Behavior: Only a file that contains an already-present ADD COLUMN changes behavior: if nothing can be skipped, or the migration doesn't go through the plain-SQL path, the runner still only records the file, as before. If the replay without the existing columns fails for any reason (another duplicate column, a CREATE TABLE or CREATE INDEX that already exists), it logs a warning and records the file as before instead of stopping startup.
  • Follow-up: Not covered on purpose: the isSchemaAlreadyApplied cases whose only job is a hasColumn check could probably be simplified later; I left them alone.

…umn name"

Squash 3729b9e9bc + b1f6c59ab0 (maintainer fallback kept).

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
@maxmad64bis
maxmad64bis force-pushed the fix/migration-duplicate-column branch from 7548fff to 5ddfb89 Compare October 4, 2026 15:52
@diegosouzapw
diegosouzapw merged commit 9cff483 into cheaper-inference:release/v3.8.52 Oct 6, 2026
76 of 97 checks passed
@maxmad64bis
maxmad64bis deleted the fix/migration-duplicate-column branch October 7, 2026 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(backend): migration runner marks a file as applied on the first "duplicate column name", even when the rest of the file never ran

2 participants