postgres-agents-migration-reviewer

Quick Reference :

This skill turns Claude into a migration safety reviewer. It is an orchestration skill, not a DDL reference: it defines a fixed 14-point checklist, a severity grading scheme, and which sibling skill to cite per finding. Run every check against every statement in the migration. Never approve a migration by inspection alone.

Review workflow :
  1. PARSE   -- split the migration into statements; identify each DDL/DML kind
               and its target table; note transaction boundaries
  2. CHECK   -- run all 14 checks (M-1..M-14), record each hit with a location
  3. LOCK    -- for any locking concern, recommend a rehearsal on a copy of
               production-sized data, timed under concurrent load
  4. REPORT  -- emit structured findings + one overall verdict

Severity grades :

CRITICAL : blocks production traffic, rewrites a table, loses data, or breaks the running app. Verdict becomes UNSAFE.
WARNING : safe in principle but a foot-gun at scale or under load (long lock, bloat, no timeout guard). Verdict becomes NEEDS-CHANGES.
INFO : process or hygiene note; the author decides.

Verdict :

SAFE : no CRITICAL and no WARNING.
NEEDS-CHANGES : one or more WARNING, no CRITICAL.
UNSAFE : one or more CRITICAL.

The core principle : lock danger is about CONCURRENT TRAFFIC, not table size. A plain CREATE INDEX on a 500-row table still blocks every write to that table for the build's duration. ALWAYS judge a lock by what it blocks and for how long, NEVER by row count.

When To Use This Skill :

ALWAYS use this skill when :

A user asks "is this migration safe", "review this migration", "will this migration lock the table", or pastes a migration file for feedback
Auditing a migration file in a pull request or a deploy diff
Another agent generated schema-change SQL that must run against production

NEVER use this skill for :

Reviewing plain queries for performance (use postgres-agents-query-reviewer)
Writing the migration from scratch (use postgres-impl-zero-downtime-migrations)
Reviewing table design / normalization (use postgres-core-schema-design)

Decision Trees :

Grading a finding :

A check fired. What severity?
  |
  |-- Does it block reads or writes to a hot table, rewrite a table,
  |   lose data, or break the running app?
  |     YES --> CRITICAL. Verdict becomes UNSAFE.
  |
  |-- Is it correct in isolation but a foot-gun under concurrent load
  |   or at scale (long lock, bloat, missing timeout guard)?
  |     YES --> WARNING. Verdict becomes NEEDS-CHANGES (if no CRITICAL).
  |
  |-- Is it a process / hygiene gap (no down path, no rehearsal note)?
        YES --> INFO. Does not change the verdict.

Does this statement need an online-safe rewrite :

What does the statement do?
  |
  |-- CREATE INDEX (no CONCURRENTLY)
  |     --> blocks writes for the whole build. Require CONCURRENTLY,
  |         outside any transaction block.  [M-1, M-11]
  |
  |-- ALTER COLUMN TYPE
  |     --> full table rewrite unless binary-coercible with no USING.
  |         Require expand-contract.  [M-2]
  |
  |-- ADD COLUMN
  |     --> volatile DEFAULT rewrites the table; NOT NULL with no DEFAULT
  |         on a populated table fails.  [M-3, M-4]
  |
  |-- SET NOT NULL / ADD FOREIGN KEY / ADD CHECK
  |     --> scans (and FK/CHECK block writes) unless the
  |         NOT VALID + VALIDATE two-step is used.  [M-5, M-6]
  |
  |-- DROP COLUMN / RENAME COLUMN / RENAME TABLE
  |     --> breaks the running app unless app deploy is sequenced
  |         (expand-contract).  [M-7]
  |
  |-- Large UPDATE / DELETE backfill
        --> long lock + bloat unless chunked.  [M-9]

Is this lock actually dangerous :

The statement takes a lock. Dangerous?
  |
  |-- Lock is ACCESS EXCLUSIVE or blocks writes, AND the table
  |   receives concurrent traffic?
  |     YES --> dangerous regardless of table size. The table is small
  |             but the traffic is not. Flag it.
  |
  |-- Lock is SHARE UPDATE EXCLUSIVE (CONCURRENTLY, VALIDATE CONSTRAINT)?
  |     --> does not block reads or writes. Safe; long wall-clock only.
  |
  |-- Held only for a metadata change, sub-millisecond, behind lock_timeout?
        --> acceptable. Confirm a lock_timeout guard exists. [M-8]

The Review Checklist :

Run every check. Each has a trigger, an ALWAYS/NEVER rule, a severity, and a cite-skill. Full rationale and safe rewrites are in references/methods.md and references/examples.md.

M-1 : CREATE INDEX without CONCURRENTLY

ALWAYS flag CREATE INDEX with no CONCURRENTLY on a table that takes writes. NEVER wave it through because the table looks small. Severity : CRITICAL. Cite : postgres-impl-zero-downtime-migrations. WHY : a plain CREATE INDEX locks the table against inserts, updates and deletes for the entire build; reads continue. Fix : CREATE INDEX CONCURRENTLY.

M-2 : ALTER COLUMN TYPE that rewrites the table

ALWAYS flag ALTER COLUMN ... TYPE unless the change is binary-coercible with no USING clause. NEVER assume the type change is free. Severity : CRITICAL. Cite : postgres-impl-zero-downtime-migrations. WHY : a type change rewrites the whole table and its indexes under ACCESS EXCLUSIVE, blocking all traffic. Fix : expand-contract (add a new column, backfill in chunks, swap, drop the old column).

M-3 : ADD COLUMN with a volatile DEFAULT

ALWAYS flag ADD COLUMN ... DEFAULT <volatile> such as DEFAULT now() or DEFAULT random(). Severity : WARNING. Cite : postgres-impl-zero-downtime-migrations. WHY : a non-volatile constant default is metadata-only since v11, but a volatile default rewrites the entire table and its indexes. Fix : add the column with no default, then backfill in chunks, then set the default.

M-4 : ADD COLUMN NOT NULL without a DEFAULT on a populated table

ALWAYS flag ADD COLUMN ... NOT NULL with no DEFAULT when the table has rows. Severity : CRITICAL. Cite : postgres-impl-zero-downtime-migrations. WHY : every existing row would violate NOT NULL, so the statement fails. Fix : add the column nullable, backfill in chunks, then add NOT NULL via M-5.

M-5 : SET NOT NULL without the NOT VALID CHECK path

ALWAYS flag a bare ALTER COLUMN ... SET NOT NULL on a large table. Severity : WARNING. Cite : postgres-impl-zero-downtime-migrations. WHY : SET NOT NULL scans the whole table under ACCESS EXCLUSIVE. Fix : add CHECK (col IS NOT NULL) NOT VALID, VALIDATE it (light lock), then SET NOT NULL skips its scan because the proven CHECK already guarantees it.

M-6 : ADD FOREIGN KEY or ADD CHECK without NOT VALID + VALIDATE

ALWAYS flag a new FOREIGN KEY or CHECK constraint added in one step on a large table. Severity : WARNING. Cite : postgres-impl-zero-downtime-migrations. WHY : a one-step constraint scans every existing row to validate it while holding a write-blocking lock. Fix : ADD CONSTRAINT ... NOT VALID (fast, metadata only), then VALIDATE CONSTRAINT later (takes only SHARE UPDATE EXCLUSIVE, does not block writes).

M-7 : DROP COLUMN, RENAME COLUMN, or RENAME TABLE

ALWAYS flag a DROP COLUMN, RENAME COLUMN, or RENAME TABLE. Severity : CRITICAL. Cite : postgres-impl-zero-downtime-migrations. WHY : the old app code still references the old name and breaks the instant the migration commits, before the new code is deployed. Fix : expand-contract, sequence the schema change and the app deploy so neither side ever sees a name the other does not.

M-8 : No lock_timeout guard around the migration

ALWAYS flag a locking migration that does not set lock_timeout (and usually statement_timeout). Severity : WARNING. Cite : postgres-errors-statement-timeouts. WHY : lock_timeout defaults to 0, wait forever. A migration that cannot get its lock then queues behind one slow query and blocks ALL traffic to the table. Fix : SET lock_timeout = '5s' (and a sane statement_timeout) so the migration fails fast and is retried, instead of stalling the system.

M-9 : Single large UPDATE or DELETE backfill

ALWAYS flag a backfill that updates or deletes a whole large table in one statement. Severity : WARNING. Cite : postgres-impl-zero-downtime-migrations. WHY : one huge write holds row locks for minutes and bloats the table with dead tuples. Fix : chunk it (WHERE id BETWEEN ... or ctid batches with a small LIMIT), committing each batch.

M-10 : DDL plus heavy data change in one transaction

ALWAYS flag a transaction that takes a DDL lock and then runs a long data change before commit. Severity : WARNING. Cite : postgres-impl-zero-downtime-migrations. WHY : the ACCESS EXCLUSIVE lock from the DDL is held for the entire data change, multiplying the outage window. Fix : commit the DDL first, then run the data change in its own chunked transactions.

M-11 : CONCURRENTLY inside a transaction block

ALWAYS flag CREATE INDEX CONCURRENTLY, REINDEX ... CONCURRENTLY, or DROP INDEX CONCURRENTLY wrapped in BEGIN/COMMIT or run by a tool that wraps each migration in a transaction. Severity : CRITICAL. Cite : postgres-impl-zero-downtime-migrations. WHY : CONCURRENTLY cannot run inside a transaction block; the migration errors out. Fix : run it as its own statement, outside any transaction; configure the migration tool to disable its transaction wrapper for that step.

M-12 : No rollback or down path considered

ALWAYS note when a migration has no down / rollback path and no recovery plan. Severity : INFO. Cite : postgres-impl-zero-downtime-migrations. WHY : an irreversible migration that fails mid-deploy leaves the schema in an unknown state. Fix : write a down path, or document explicitly why the change is forward-only and how to recover.

M-13 : TRUNCATE in a migration

ALWAYS flag any TRUNCATE in a migration. Severity : CRITICAL. Cite : postgres-impl-zero-downtime-migrations. WHY : TRUNCATE deletes every row irreversibly and is rarely what a schema migration should do. Fix : confirm the data loss is intended in writing; if it is a test-data reset, it does not belong in a production migration.

M-14 : Index or constraint on a partitioned table

ALWAYS check whether an index or constraint targets a partitioned-table parent or its individual partitions. Severity : WARNING. Cite : postgres-impl-partitioning. WHY : creating an index on the partitioned parent cascades to every partition and a plain (non-concurrent) form locks them all. Fix : build the index CONCURRENTLY on each partition first, then CREATE INDEX ON ONLY the parent and ATTACH the partition indexes.

Output Format :

Emit findings as a list, then exactly one verdict line. Per finding :

[SEVERITY] <check id> : <location, e.g. statement 3, line 12>
  Problem : <what is unsafe and what it blocks / rewrites / breaks>
  Fix     : <concrete safe rewrite>
  Cite    : <sibling skill>

End with one verdict :

Verdict : UNSAFE   (2 CRITICAL, 1 WARNING, 0 INFO)

See references/methods.md for the full report schema and references/examples.md for complete worked reviews.

Anti-Patterns :

(Reviewer anti-patterns. Full cause + symptom + fix in references/anti-patterns.md)

Lock-blind approval : approving a migration without judging each lock
Small-table fallacy : assuming few rows makes a lock free (it is concurrent traffic that matters, not row count)
Deploy-order blindness : passing a DROP / RENAME without checking the app deploy sequence
Severity inflation : grading a missing down path as CRITICAL, or a table rewrite as INFO
Citing nothing : reporting a finding without the sibling skill that fixes it

Reference Links :

references/methods.md : full checklist table, lock-level reference, severity rubric, report schema
references/examples.md : complete worked reviews, unsafe-to-safe rewrites per check, version-annotated
references/anti-patterns.md : reviewer anti-patterns with cause + symptom + fix