How to Review Schema Changes in a Pull Request
Reviewing a schema migration in a pull request is a different task than reviewing application code, and most review habits built for the latter don't transfer cleanly. A migration file is a diff of instructions — ALTER TABLE, CREATE INDEX — not a diff of the resulting schema. Reading the instructions and mentally reconstructing what the schema looks like before and after is exactly the kind of task humans are bad at and reviews routinely skip.
What a migration diff hides
A three-line ALTER TABLE migration can hide a lot: whether the new column is nullable and what backfills it, whether an index is being built concurrently or will lock the table, whether a foreign key being added will fail outright because existing data already violates it, and whether the change is even reversible if it needs to roll back mid-deploy. None of that is visible from the SQL text alone without already knowing the current state of the table it's modifying.
A practical review checklist
- Locking behavior. Does this migration take a long-held lock on a large, actively-written table?
ADD COLUMN ... DEFAULTon older Postgres versions rewrites every row; adding aNOT NULLconstraint requires a full table scan unless it's done in two steps (constraint asNOT VALID, thenVALIDATE CONSTRAINTseparately). - Index creation. Is a new index built with
CONCURRENTLYon a table that takes production writes? Without it, the index build blocks writes for its duration. - Backward compatibility with the currently-deployed code. If the migration and the application-code change deploy separately (most CI/CD pipelines do), does the schema work with both the old and new code for the window in between?
- Data integrity on new constraints. Does a new foreign key or unique constraint get checked against existing rows first, or will the migration fail on deploy because production data already violates it?
- Reversibility. Is there a down-migration, and does it actually restore the prior state, or does it silently drop data that can't be recovered (dropped columns, dropped tables)?
- What it does to the diagram, not just the table. Does this migration add a relationship that should have a foreign key but doesn't (see detecting missing foreign keys), or leave behind a table nothing else references anymore (see finding orphan tables)?
Review the resulting schema, not just the instructions
The single highest-leverage habit here is refusing to review a migration in isolation from the schema it's modifying. Pull the current schema, apply the migration mentally (or literally, against a scratch database), and review the result — new column, new index, new constraint, in context — rather than the five lines of ALTER TABLE syntax on their own. This is also where a rendered diagram earns its keep: seeing a new table appear with its relationships drawn in is a much faster correctness check than reading a CREATE TABLE statement and tracing every REFERENCES clause by eye.
Turning this into something attachable to the PR
None of the checklist above is specific to any one tool — it's a way of thinking about migrations that applies whether you're reviewing a Rails migration, a Flyway script, or raw SQL. But writing up the risk/impact/rollback summary by hand for every PR gets skipped under deadline pressure, which is usually when it's needed most. ERD Factory keeps a version history for every saved schema and can diff any two versions directly — and its Change Request generation turns that diff into a structured summary (title, per-table change list, risk/impact, rollback plan) that's meant to be pasted straight into a PR description, instead of written from scratch under a deadline.