Reviewing AI-Written Postgres Migrations Before They Run

The migration was three statements long and every one of them was correct. An agent had added a status column, renamed a column that had been misspelled ammount_cents since 2023, and created an index to support the new filter — exactly what I asked for, in valid SQL, applied cleanly to a test database holding twelve rows. The same file went to a table with over 40 million rows, and the API stopped answering before the deploy had finished rolling out.

This is the review I now run on every migration diff, whoever or whatever wrote it. Every lock level and version-specific behaviour below is checked against the PostgreSQL manual rather than recalled: which ALTER TABLE forms take ACCESS EXCLUSIVE, what stopped rewriting the table in Postgres 11 and 12, what a failed concurrent index build leaves behind, and why renaming a column safely is four deploys instead of one.

Review the locks, not the logic

A review that reads migration SQL for correctness will pass almost everything an agent produces, because the SQL is correct. The question that separates a safe migration from an outage is not what a statement does, it is which lock it takes and how long it holds it. The manual states the default plainly: ALTER TABLE acquires an ACCESS EXCLUSIVE lock unless a subform is explicitly noted otherwise, and when several subcommands are given, the lock acquired is the strictest one any of them requires.

The strictest-lock rule is the one agents trip over, because folding several changes into one ALTER TABLE looks like an optimisation, and on a small table it is one: the manual's own justification for allowing it is that multiple scans or rewrites collapse into a single pass. On a large table it is a trap. A cheap ADD COLUMN sitting next to an ALTER COLUMN TYPE inherits that type change's lock and its full table rewrite, and the diff still reads as one tidy statement.

The lock it waits for is the lock that hurts

Most migration outages are not caused by the statement running. They are caused by the statement waiting. ACCESS EXCLUSIVE conflicts with every other lock mode, so an ALTER TABLE cannot begin while any open transaction still holds so much as an ACCESS SHARE on the table — a long analytics SELECT, a forgotten psql session, a connection sitting idle inside a transaction. The manual is blunt about what happens next: a transaction seeking a table-level or row-level lock waits indefinitely for the conflicting locks to be released.

That would be survivable if the waiter waited alone. It does not. PostgreSQL queues lock requests rather than granting them out of order, so every SELECT that arrives while the ALTER is queued stops behind it, and the table looks frozen even though the migration has not done a single byte of work yet. Only an ACCESS EXCLUSIVE lock blocks a plain SELECT, which is why this failure belongs to schema changes and to nothing else. The defence is to refuse to wait: bound the wait with lock_timeout, then retry.

-- Every migration session starts with this. A migration that cannot get the
-- lock in three seconds is not "slow", it is queued in front of the whole
-- table, and every reader that arrives behind it is queued too.
SET lock_timeout = '3s';        -- a bare number means milliseconds, so name the unit
SET statement_timeout = '0';    -- lock_timeout at or above this would never fire

BEGIN;
ALTER TABLE invoices ADD COLUMN status text;   -- catalog write, lock held for milliseconds
COMMIT;

lock_timeout aborts any statement that waits longer than the limit while attempting to acquire a lock, and the limit applies separately to each lock acquisition attempt. Retry the whole file rather than the statement, because the abort rolls back the transaction that contained it. Five attempts with a growing pause is usually enough. If it is not, the honest reading is that something is holding a long transaction open, and the migration is not the thing that needs fixing.

#!/usr/bin/env bash
# Retry the FILE, not the statement. lock_timeout aborts one statement, and an
# aborted statement rolls back the transaction around it, so a half-applied
# migration is not a state you can be in. Back off; the blocker is transient.
set -euo pipefail

for attempt in 1 2 3 4 5; do
  if psql -v ON_ERROR_STOP=1 "$DATABASE_URL" -f 001_add_status.sql; then
    exit 0
  fi
  echo "attempt $attempt lost the lock race, backing off"
  sleep $(( attempt * 10 ))
done

echo "five attempts, still blocked. Something is holding a long transaction."
exit 1

NOT NULL, and what really changed in Postgres 11, 12 and 18

The most common shape I get from an agent is a new column that is NOT NULL with a default, added in a single statement. On a current server this is nearly free and the review should pass it: since Postgres 11, whose release notes list the change as allowing a column with a non-null default to be added without a table rewrite, the default is evaluated once and stored in the table's metadata, then returned for existing rows on read. What the manual excludes from that fast path is the part worth checking — a volatile default such as clock_timestamp, a stored generated column, an identity column, or a domain type carrying constraints each still rewrite the entire table and all of its indexes.

Adding NOT NULL to a column that already exists is the harder case, and it is the one an agent writes without hesitating. It takes ACCESS EXCLUSIVE and scans every row to prove no NULL is present: no rewrite, but a full scan with reads and writes locked out for its duration. Postgres 12 added the escape hatch, and the manual documents the rule — if a valid CHECK constraint exists that proves no NULL can exist, and it is not dropped in the same command, the table scan is skipped.

-- Wrong on a large table: ACCESS EXCLUSIVE held for a scan of every row,
-- which means no reads and no writes until the scan finishes.
ALTER TABLE invoices ALTER COLUMN status SET NOT NULL;

-- Right: three statements, and the only slow one takes a lock writers ignore.
ALTER TABLE invoices
  ADD CONSTRAINT invoices_status_not_null
  CHECK (status IS NOT NULL) NOT VALID;          -- commits immediately, no scan

ALTER TABLE invoices
  VALIDATE CONSTRAINT invoices_status_not_null;  -- SHARE UPDATE EXCLUSIVE, scans,
                                                 -- does not lock out concurrent updates

ALTER TABLE invoices ALTER COLUMN status SET NOT NULL;
-- the scan is skipped: a valid CHECK already proves no NULL can exist

The middle statement is what makes the sequence safe. A constraint added NOT VALID commits immediately without scanning, and validating it afterwards acquires only a SHARE UPDATE EXCLUSIVE lock, which does not lock out concurrent updates. Postgres 18 shortens this further by letting ALTER TABLE set the NOT VALID attribute on a not-null constraint directly. Which of those paths applies depends on the server version running in production, not the one on the laptop the migration was tested against, and no agent can read that from your repository.

The column the running release is still selecting

A rolling deploy means two versions of your application talk to one database at the same time, for as long as the rollout takes. That window is the thing an agent has no way to know about, and it is why the cheapest statement in a migration is so often the one that causes the outage. Dropping or renaming a column is a catalog operation that finishes in milliseconds. The old release, still serving traffic, goes on asking for a column that no longer exists.

-- What the PREVIOUS release is still sending while the new pods roll out.
-- No ORM selects a wildcard: it lists every column it has mapped, including
-- the misspelled one you just "fixed" in the same deploy.
SELECT "invoices"."id",
       "invoices"."customer_id",
       "invoices"."ammount_cents",
       "invoices"."created_at"
FROM "invoices"
WHERE "invoices"."id" = $1;

-- ERROR:  column invoices.ammount_cents does not exist
-- LINE 3:        "invoices"."ammount_cents",

-- The rename took an ACCESS EXCLUSIVE lock for about a millisecond.
-- The outage lasted as long as the rolling deploy did.

It is worth reading what an ORM actually sends, because the usual objection — we stopped using that field months ago — misses the mechanism entirely. Mapped entities generate explicit column lists, so a column your code never references by name is still in every SELECT the previous release issues. The fix is not a faster deploy. It is to never remove anything in the same release that stops using it.

Two smaller facts belong in the same review. A dropped column does not hand back its space: the manual notes that immediate reclamation requires one of the rewriting forms of ALTER TABLE, so the disk saving people expect from a drop is not there until something rewrites the table. And a rename is strictly worse than a drop, because it breaks the old release and the new one in opposite directions at the same moment, which is precisely what expand-then-contract exists to prevent.

CREATE INDEX without CONCURRENTLY, and the invalid index it leaves

A standard index build locks out writes on the table, though not reads, until it finishes. CONCURRENTLY avoids that by building without taking locks that prevent inserts, updates or deletes, and the manual is careful to list what it costs: two scans of the table, a wait for existing transactions to terminate before each scan, significantly more total work, and only one concurrent build on a table at a time. It also cannot run inside a transaction block, which is the detail that breaks migration tools, since most of them wrap each migration file in one by default.

-- CREATE INDEX CONCURRENTLY cannot run inside a transaction block, and most
-- migration tools wrap each file in one. Turning that wrapper off for this
-- file is the review item, not the CONCURRENTLY keyword.
CREATE INDEX CONCURRENTLY IF NOT EXISTS invoices_status_created_idx
  ON invoices (status, created_at DESC);

-- A concurrent build that fails leaves an INVALID index behind. It is ignored
-- for querying, still costs update overhead on every write, and a failed
-- unique build goes on enforcing its uniqueness. Look before you retry:
SELECT c.relname AS index_name, i.indisvalid
FROM pg_index i
JOIN pg_class c ON c.oid = i.indexrelid
WHERE NOT i.indisvalid;

-- Recovery is to drop it and build again, or REINDEX INDEX CONCURRENTLY.
DROP INDEX CONCURRENTLY IF EXISTS invoices_status_created_idx;

The failure mode is the part I have never seen an agent mention. If the build hits a deadlock or a uniqueness violation, CREATE INDEX fails and leaves an invalid index behind in the catalog. It is ignored for querying because it might be incomplete, it still consumes update overhead on every write to the table, and if it was a unique index it goes on enforcing its uniqueness constraint afterwards — so a failed migration can start rejecting legitimate inserts. Recovery is manual: drop the index and build it again, or rebuild it with REINDEX INDEX CONCURRENTLY.

A backfill is not one statement

The last thing an agent hands you is usually a single UPDATE that sets the new column for every row. It is correct, it is idempotent, and on a large table it is the most expensive statement in the file. PostgreSQL does not remove the old version of an updated row immediately — that is what makes MVCC work — so an update across the whole table writes a new version of every row and leaves a dead one behind it.

The manual's guidance on what follows is unambiguous: after massive update activity, plain vacuuming may not be satisfactory, and reclaiming the space can require VACUUM FULL or CLUSTER, both of which rewrite the entire table. There is a second cost that only appears in production. The whole update is one transaction, so it commits as one burst of WAL, and every standby has to receive and replay all of it. A read replica falls behind by however long that takes, which is when the tickets about stale data arrive.

#!/usr/bin/env bash
# Backfill in bounded primary-key ranges. Each batch is its own transaction, so
# row locks are released as it goes and autovacuum can reclaim the dead row
# versions behind it instead of meeting all of them at the end.
set -euo pipefail

step=20000
lo=$(psql -At "$DATABASE_URL" -c "SELECT min(id) FROM invoices")
max=$(psql -At "$DATABASE_URL" -c "SELECT max(id) FROM invoices")

while [ "$lo" -le "$max" ]; do
  psql -v ON_ERROR_STOP=1 "$DATABASE_URL" -c "
    SET lock_timeout = '3s';
    UPDATE invoices SET status = 'draft'
     WHERE id >= $lo AND id < $lo + $step AND status IS NULL;"
  lo=$(( lo + step ))
  sleep 0.2   # give autovacuum and the replicas room between batches
done

# Do NOT batch with OFFSET. Each batch would re-scan and discard everything
# before it, so the job gets slower the further it gets. A key range does not.

Batching over a bounded primary-key range fixes both problems. Each batch is its own transaction, so row locks and dead row versions are released as the job runs and autovacuum keeps pace instead of meeting the whole table at the end, and the WAL reaches the replicas in pieces they can replay between batches. The pause matters as much as the batch size. A backfill that finishes in an hour without anyone noticing is a better outcome than one that finishes in five minutes and pages someone.

Expand then contract: a rename is four deploys

Every failure above shares one root cause: a migration and the code that depends on it shipped together. Expand-then-contract removes that coupling by making the schema hold the old and the new shape at once, so no running release ever disagrees with the database about what exists. Renaming ammount_cents to amount_cents is the canonical example, and doing it safely takes four deploys.

1. Deploy one, migration only. Add amount_cents as a nullable column. Nothing reads it, nothing writes it, and rolling back means dropping a column no code has ever touched.
2. Deploy two, code only. Write both columns on every insert and update. Once every instance is on this release, run the batched backfill: new rows are already correct, so the job only has to catch up the old ones.
3. Deploy three, code only. Read from amount_cents, keep writing both. This is the reversible cutover — if the numbers look wrong, the next deploy flips reads back and nothing was lost, because the old column has been maintained the whole time.
4. Deploy four, code and then migration. Stop writing ammount_cents, confirm nothing in the codebase or in any report still references it, and only then drop it. The drop can trail by a release; waiting costs nothing.

Four deploys sounds like ceremony until you count what each one buys. Every step is independently reversible, and no step asks the schema and a running release to agree about a name at the same instant. The backfill is deliberately not a deploy: it is a job that runs between two of them, and it can be stopped and resumed without blocking anything. If a change cannot be expressed in this shape, that is a finding in itself, and it is much better to know it before the migration runs than during it.

The checklist I run on every migration diff

This is what I ask of a migration now, and it is the same list whether a person or an agent wrote it. It takes about two minutes on a diff, and it has caught something on roughly every third one.

1. Which lock does each statement take, and does any subcommand upgrade the lock for the whole statement? Combined ALTER TABLE subcommands take the strictest lock any one of them needs.
2. Does the session set lock_timeout, and is the file safe to rerun from the top after an aborted attempt?
3. Does anything scan or rewrite the table — a SET NOT NULL with no valid CHECK behind it, an ALTER COLUMN TYPE, a volatile default, a stored generated column, an identity column?
4. Is every index build CONCURRENTLY, with the tool's transaction wrapper disabled for that file, and is there a check afterwards for an invalid index left behind?
5. Does the diff drop or rename anything the currently deployed release still selects, including columns the code no longer mentions but the ORM still maps?
6. Is any backfill batched over a bounded key range, one transaction per batch, with a pause between them?

The honest note to finish on is that an agent is good at this, better than that first draft suggested. Ask for the expand-then-contract sequence by name, give it the row count, the server version and the fact that deploys are rolling, and what comes back is four files that are close to right, with the index build already outside a transaction. The failure was never the model's SQL. It was the request: I asked for a migration, and a migration is exactly what I got.

The rule I carry now is short enough to apply while reading a diff: a migration is safe when it can run without the currently deployed release noticing, and finish without a reader waiting. Everything above is a way of testing those two properties. Review for the locks and for the other release, and it stops mattering who wrote the SQL.