How to review database migrations

Database migrations are the changes most likely to hurt you and the hardest to undo, and they usually arrive as a five-line diff that gets approved in thirty seconds. To review database migrations well, stop reading them as code and start reading them as operations: something that will run exactly once, against production data, under production load, with no clean way back. That is blast-radius thinking applied to its most extreme case, because a bad migration does not break a feature. It breaks every feature that touches the table.

Why migrations justify breaking the PR-size rules

Normal review heuristics say right-size the PR and spend attention proportional to diff size. Migrations invert both. The schema change should ship alone, even if that means an extra PR that is three lines long, and those three lines deserve more scrutiny than the three hundred lines of application code that follow. A one-line ALTER TABLE can be the riskiest line your team merges all quarter. When a migration arrives bundled with feature code, the first review comment writes itself: split it out. Separate deploys are the whole point, because the schema and the code will never change at the same instant anyway.

The down path is part of the change

Ask what happens when this needs to be undone at 2am, because that is when it will need to be undone. Concretely:

• Does a down migration exist at all? An empty down block or a raise-irreversible marker is sometimes honest and fine, but it must be a declared decision, not an omission.
• Does it restore data or just shape? Re-adding a dropped column brings back the column, not the values. If the up path destroys data, the down path is a lie unless there is a backup step.
• Has anyone actually run it? Down migrations are the least tested code in most repos. Ask for evidence: run up, run down, run up again on a real copy.

Lock behavior: ask what happens to the table while this runs

The silent killer in migration review is the lock you cannot see in the diff. The SQL says what changes; it does not say that the table is unreadable for four minutes while it happens. Questions to ask of every schema statement:

• How big is this table in production? Row count changes everything. An instant operation on the dev database can be a full table rewrite at scale.
• Does this statement rewrite or lock the table? Adding an index without CONCURRENTLY in Postgres, changing a column type, or adding a volatile default are classic full-lock traps. Know your engine: MySQL and Postgres disagree about which operations are online.
• Is there a lock timeout? A migration waiting on a lock queues everyone behind it. Setting lock_timeout so the migration fails fast instead of stalling production is cheap insurance, and its absence is worth a review comment.

Deploy order: old code runs against the new schema

Code and schema never deploy in the same instant, so there is always a window where the previous version of the application runs against the migrated database. Most data-loss stories live in that window. Dropping a column the old code still reads throws errors until the deploy finishes. Renaming a column is worse: it is a drop and an add wearing a trench coat, and the old code writes into a column that no longer exists. The reviewable pattern is expand and contract: add the new column, dual-write, backfill, switch reads, and only then, releases later, drop the old one. When a migration renames or drops anything, the review question is not whether the SQL is correct. It is: which deployed code version reads or writes this, and what does it do during the window?

Backfills are a separate step, not a rider

A schema change that also moves data is two changes in one, and the second one is the slow, dangerous one. Google’s guidance on small changes applies with extra force here: the backfill should be its own step, reviewed on its own terms. What to look for when it is: batching (updating fifty million rows in one transaction will bloat and lock), idempotency (the job will crash midway at least once, so rerunning must be safe), throttling (the backfill competes with production traffic for I/O), and progress visibility (how does anyone know it is 40% done rather than hung?). A backfill inlined into the migration file fails most of these by construction, because migration runners assume fast, transactional, run-once semantics.

A checklist to review database migrations

Fold the above into a repeatable pass, the way a good checklist turns judgment into habit:

1. Reversible? If not, is irreversibility declared and accepted?
2. What locks does each statement take, on the production-sized table?
3. Lock timeout and statement timeout set?
4. Can the currently deployed code run against the new schema? The old schema?
5. Any dropped or renamed column still referenced by live code?
6. Backfill separated, batched, idempotent, throttled?
7. Was this tested against production-shaped data, and how long did it take?

The last item deserves emphasis because it is the cheapest to skip. A migration tested only on an empty development database has not been tested. Restore a scrubbed production copy, run the migration, and time it. The number you get is the difference between an invisible deploy and an incident review, and it is the single most persuasive line an author can put in a migration PR description.