PR Review Demo

This page shows a realistic before-and-after review flow: one risky migration, the PR-style markdown pgfence emits today, and the safer rollout a reviewer would ask for before merge.

This demo is based on the shipped --output github reporter. The formatting and findings mirror the OSS CLI, with the temporary file path replaced by a realistic repo path.
Want the repo-backed version? The same scenario lives in examples/pr-review-demo with the risky migration, safer rollout files, and generated reporter artifacts.

Before Merge: The Risky Migration

At a glance this looks like one extra column and one index. On a busy users table, it is exactly the kind of migration that blocks traffic and creates a painful rollback window.

sql
ALTER TABLE users ADD COLUMN last_seen_at TIMESTAMPTZ NOT NULL DEFAULT now();
CREATE INDEX idx_users_last_seen_at ON users (last_seen_at);

What the Reviewer Sees

If your CI posts the GitHub markdown artifact, the review comment looks like this:

md
## pgfence Migration Safety Report

### <code>migrations/20260415_add_last_seen_at.sql</code> :red_circle: HIGH

| # | Statement | Lock Mode | Blocks | Risk | Message |
|---|-----------|-----------|--------|------|---------|
| 1 | <code>ALTER TABLE users ADD COLUMN last_seen_at TIMESTAMPTZ NOT NULL DEFAULT now()</code> | <code>ACCESS EXCLUSIVE</code> | <code>reads, writes, DDL</code> | :red_circle: HIGH | <code>ADD COLUMN &quot;last_seen_at&quot; with non-constant DEFAULT: causes table rewrite under ACCESS EXCLUSIVE lock. Column is also NOT NULL, requiring an additional constraint step</code> |
| 2 | <code>CREATE INDEX idx_users_last_seen_at ON users (last_seen_at)</code> | <code>SHARE</code> | <code>writes, DDL</code> | :warning: MEDIUM | <code>CREATE INDEX &quot;idx_users_last_seen_at&quot; without CONCURRENTLY: acquires SHARE lock, blocking all writes on &quot;users&quot;</code> |

<details>
<summary>Safe Rewrite Recipes</summary>

#### <code>add-column-non-constant-default</code> <code>Add column without default, backfill in batches, then set default</code>
<pre><code class="language-sql">
ALTER TABLE users ADD COLUMN IF NOT EXISTS last_seen_at timestamptz;
-- Backfill out-of-band in batches (repeat until 0 rows updated):
-- WITH batch AS (
--   SELECT ctid FROM users WHERE last_seen_at IS NULL LIMIT 1000 FOR UPDATE SKIP LOCKED
-- )
-- UPDATE users t SET last_seen_at = &lt;fill_value&gt; FROM batch WHERE t.ctid = batch.ctid;
ALTER TABLE users ALTER COLUMN last_seen_at SET DEFAULT &lt;fill_value&gt;;
ALTER TABLE users ADD CONSTRAINT chk_last_seen_at_nn CHECK (last_seen_at IS NOT NULL) NOT VALID;
ALTER TABLE users VALIDATE CONSTRAINT chk_last_seen_at_nn;
ALTER TABLE users ALTER COLUMN last_seen_at SET NOT NULL;
ALTER TABLE users DROP CONSTRAINT chk_last_seen_at_nn;
</code></pre>

#### <code>create-index-not-concurrent</code> <code>Use CREATE INDEX CONCURRENTLY to allow reads and writes during index build.</code>
<pre><code class="language-sql">
CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_users_last_seen_at ON users (last_seen_at);
-- Note: CONCURRENTLY must run outside a transaction block (disable ORM transaction wrappers)
</code></pre>

</details>

**Policy Violations:**

| Severity | Rule | Message | Suggestion |
|----------|------|---------|------------|
| :red_circle: error | <code>missing-lock-timeout</code> | <code>Missing SET lock_timeout: without this, an ACCESS EXCLUSIVE lock will queue behind running queries and every new query queues behind it, causing a lock queue death spiral</code> | <code>Add SET lock_timeout = &#39;2s&#39;; at the start of the migration</code> |
| :warning: warning | <code>missing-statement-timeout</code> | <code>Missing SET statement_timeout: long-running operations can block other queries indefinitely</code> | <code>Add SET statement_timeout = &#39;5min&#39;; at the start of the migration</code> |
| :warning: warning | <code>missing-application-name</code> | <code>Missing SET application_name: makes it harder to identify migration locks in pg_stat_activity</code> | <code>Add SET application_name = &#39;migrate:&lt;migration_name&gt;&#39;;</code> |
| :warning: warning | <code>missing-idle-timeout</code> | <code>Missing SET idle_in_transaction_session_timeout: orphaned connections with open transactions can hold locks indefinitely</code> | <code>Add SET idle_in_transaction_session_timeout = &#39;30s&#39;;</code> |

### Coverage

Analyzed **2** SQL statements. **0** dynamic statements not analyzable. Coverage: **100%**

---
*Generated by [pgfence](https://pgfence.com)*

What pgfence Caught

  • DEFAULT now() is non-constant, so adding the column can rewrite the table under an ACCESS EXCLUSIVE lock.
  • The same statement also tries to make the new column NOT NULL immediately, which needs a safer contract step.
  • A plain CREATE INDEX takes a SHARE lock and blocks writes for the duration of the index build.
  • The migration is also missing the session guardrails pgfence expects by default: lock_timeout, statement_timeout, application_name, and idle_in_transaction_session_timeout.

What Gets Merged Instead

The usual fix is not one magical replacement statement. It is a safer rollout: expand first, backfill out of band, then contract and add the concurrent index in the right place.

1. Expand Step

sql
-- migrations/20260415_add_last_seen_at_expand.sql
SET lock_timeout = '2s';
SET statement_timeout = '5min';
SET application_name = 'migrate:20260415_add_last_seen_at_expand';
SET idle_in_transaction_session_timeout = '30s';

ALTER TABLE users ADD COLUMN IF NOT EXISTS last_seen_at TIMESTAMPTZ;

2. Backfill Step

Run the backfill outside the migration transaction so the application can keep serving traffic while you fill existing rows in batches.

sql
-- run out of band, repeat until 0 rows update
WITH batch AS (
  SELECT ctid
  FROM users
  WHERE last_seen_at IS NULL
  LIMIT 1000
  FOR UPDATE SKIP LOCKED
)
UPDATE users u
SET last_seen_at = now()
FROM batch
WHERE u.ctid = batch.ctid;

3. Contract Step

sql
-- migrations/20260415_add_last_seen_at_contract.sql
SET lock_timeout = '2s';
SET statement_timeout = '5min';
SET application_name = 'migrate:20260415_add_last_seen_at_contract';
SET idle_in_transaction_session_timeout = '30s';

ALTER TABLE users ALTER COLUMN last_seen_at SET DEFAULT now();
ALTER TABLE users ADD CONSTRAINT chk_last_seen_at_nn CHECK (last_seen_at IS NOT NULL) NOT VALID;
ALTER TABLE users VALIDATE CONSTRAINT chk_last_seen_at_nn;
ALTER TABLE users ALTER COLUMN last_seen_at SET NOT NULL;
ALTER TABLE users DROP CONSTRAINT chk_last_seen_at_nn;

4. Concurrent Index Step

CREATE INDEX CONCURRENTLY must run outside a transaction block, so teams usually keep it in its own migration or disable the ORM transaction wrapper for that file.

sql
-- migrations/20260415_add_last_seen_at_index.sql
-- run outside the ORM transaction wrapper
CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_users_last_seen_at
ON users (last_seen_at);

Reproduce This Demo

You can generate the same style of review artifact locally with:

bash
pgfence analyze --output github migrations/20260415_add_last_seen_at.sql > pgfence-report.md
Want the terminal, SARIF, or GitLab version of the same findings? See Output Formats. Want the underlying rewrite guidance? Start with ADD COLUMN + NOT NULL and CREATE INDEX.