Skip to content

No down/rollback migrations exist — migration 010's enum ADD VALUE is not even revertible in Postgres #58

Description

@abayomicornelius

Overview

All 10 migration files under migrations/ are plain, forward-only SQL scripts (001_initial.sql through 010_batch_reconciliation.sql) — none has a corresponding "down"/rollback script, and sqlx::migrate! (used in src/db.rs::run_migrations) applies them with no reverse-migration mechanism available at all:

// src/db.rs:21-29
pub async fn run_migrations(pool: &PgPool) -> Result<()> {
    tracing::info!("Running database migrations…");
    sqlx::migrate!("./migrations").run(pool).await.context("Failed to run database migrations")?;
    tracing::info!("Migrations complete");
    Ok(())
}

sqlx's migration tooling does support reversible migrations (paired <N>_name.up.sql/<N>_name.down.sql files, with sqlx migrate revert to step backward) — this project's migrations are named as plain <N>_name.sql (simple, non-reversible format) throughout, meaning there is currently no tooling-supported way to undo a bad migration once applied to a real database; the only recourse is a hand-written, manually-run corrective SQL script or a full restore from backup.

This is a real, not merely theoretical, operational risk for at least one migration already in this repository: migrations/010_batch_reconciliation.sql:31 runs ALTER TYPE transaction_status ADD VALUE IF NOT EXISTS 'submitted_unconfirmed';. Adding a value to a Postgres enum type is not reversible by any ALTER TYPE ... DROP VALUE — Postgres has no such statement; it does not support removing a value from an enum type at all, short of the much more invasive workaround of creating a new type, migrating every dependent column to it, and dropping the old one. This means even if a .down.sql were written for every other migration in this repository, migration 010 specifically could never be cleanly reverted by a simple inverse script — a fact worth documenting explicitly rather than discovering during an actual incident.

Separately, migration 005_add_indexes.sql is already known (#4) to reference a non-existent payments table and fail outright when run — meaning sqlx::migrate!'s all-or-nothing sequential application model means no migration past 005 can currently be applied to a fresh database at all, which itself underscores why some tooling-supported recovery path (or at minimum, a documented manual runbook) matters: today, a broken migration blocks the entire chain with no supported way to skip, patch-in-place, or roll back to a known-good state without hand-editing the _sqlx_migrations tracking table directly.

Requirements

  • Adopt sqlx's reversible-migration format (<N>_name.up.sql / <N>_name.down.sql) going forward for new migrations, and write .down.sql scripts for the existing migrations where a clean reversal is actually possible.
  • For migration 010 specifically (and any other migration found to contain an irreversible operation, e.g. an enum ADD VALUE), explicitly document that it cannot be cleanly reverted via a .down.sql, and write a runbook note (in the migration file's own header comment, matching this project's existing practice of thorough migration-header documentation, e.g. 010_batch_reconciliation.sql's own detailed comment block) describing what an operator must actually do if this migration needs to be undone in production (e.g. the multi-step "create new enum type, migrate columns, swap" procedure).
  • Wire sqlx migrate revert (or the project's equivalent) into whatever local developer tooling/documentation covers running migrations, so reversibility is actually exercised and not just theoretically available.

Acceptance Criteria

  • New migrations going forward use the reversible .up.sql/.down.sql pair format.
  • Existing migrations that can be cleanly reverted (i.e. don't involve an irreversible operation like enum-value addition) have .down.sql scripts added.
  • Migration 010 (and any other irreversible migration) has explicit documentation of why it can't be simply reverted and what the actual recovery procedure looks like.
  • sqlx migrate revert is demonstrated to work against a fresh test database for at least one of the newly-added reversible migrations, as a smoke test that the tooling is actually wired up correctly.

Additional Notes

Edge cases

  • Retrofitting .down.sql for migrations that have already been applied to any existing deployed environment needs care — sqlx's migration tracking table records migrations by checksum, so renaming/restructuring existing NNN_name.sql files into the .up.sql/.down.sql pair format could invalidate already-applied migration checksums on a live database; this needs verifying against sqlx's actual behavior (or migrating only new migrations to the reversible format, leaving historical ones as-is with a documented manual-rollback note instead) before rolling this out to any environment with real applied migration history.
  • 009_add_batch_payments.sql (adding nullable batch_id/batch_index columns) and most of the earlier CREATE INDEX IF NOT EXISTS-style migrations are straightforward to reverse (DROP COLUMN/DROP INDEX) — start with these as the easy, low-risk cases before tackling the harder ones like 010.

Testing strategy

Cross-references

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

GrantFox OSSIssue tracked in GrantFox OSSMaybe RewardedIssue may be eligible for a GrantFox rewardOfficial Campaign | FWC26Campaign: Official Campaign | FWC26Third CampaignCampaign: Third CampaigndatabaseDatabase/migrationsdevopsDeployment/infravery hardVery difficult / senior-level bounty issue

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions