Code Review for SQL
EXPLAIN evidence and rollback notes required for PostgreSQL 18.4 pull requests - reviewer checklist for migrations, ORM queries, and ad hoc SQL in application repos.
Recipe
## SQL PR Review Checklist (copy into review comment)
### Migrations
- [ ] Version unique; filename has ticket ID
- [ ] Lock risk: low / medium / high (Migration Safety Skill)
- [ ] CONCURRENTLY for prod index on large tables
- [ ] Rollback SQL in db/rollback/ or PR body
- [ ] Deploy order: migration before app
### Queries
- [ ] EXPLAIN (ANALYZE, BUFFERS) on staging attached
- [ ] No seq scan regression on large tables
- [ ] Parameterized queries ($1 / named binds)
- [ ] LIMIT on list endpoints
### Security
- [ ] Role least privilege (not superuser)
- [ ] search_path set or schema-qualified names
- [ ] RLS policies if tenant tableWhen to reach for this:
- Every PR touching
db/migrations/,*.sql, repository query files - Generated SQL from Prisma, SQLAlchemy, Django ORM changes
- Analytics SQL joining PII tables
Working Example
PR: Add repository method findOpenOrdersByTenant.
Author provides:
EXPLAIN (ANALYZE, BUFFERS)
SELECT id, total, created_at
FROM orders
WHERE tenant_id = $1 AND status = 'open'
ORDER BY created_at DESC
LIMIT 100;Index Scan using idx_orders_tenant_status on orders
Buffers: shared hit=120 read=4
Execution Time: 1.2 msReviewer approves - index exists, LIMIT present, buffers reasonable.
Bad PR example (reject):
SELECT * FROM orders WHERE tenant_id = $1 OR tenant_id = $2;
-- No EXPLAIN, no LIMIT, OR prevents index on some plannersReviewer comment template:
Request changes:
1. Attach EXPLAIN (ANALYZE, BUFFERS) from staging
2. Replace OR with IN (...) or UNION ALL pattern
3. List columns explicitly; drop SELECT *
4. See EXPLAIN Review Skill for pass/fail rubricDeep Dive
Migration Review Table
| Signal | Approve | Request changes |
|---|---|---|
CREATE INDEX large table | CONCURRENTLY separate file | Single transaction index |
NOT NULL new column | expand-contract or PG default fast path documented | Table rewrite risk unexplained |
| FK add | NOT VALID + VALIDATE split | Inline validate on huge table |
| Data backfill | batched UPDATE with commit | One huge transaction |
ORM-Specific
# BAD: N+1
for order in orders:
items = session.query(Item).filter_by(order_id=order.id).all()
# GOOD: joinedload evidence in PR
orders = session.query(Order).options(joinedload(Order.items)).filter(...)- Require query count or SQL log snippet for hot path changes
Rollback Notes
-- rollback/V20260709_128__orders_priority_down.sql
DROP INDEX CONCURRENTLY IF EXISTS idx_orders_priority;
ALTER TABLE orders DROP COLUMN IF EXISTS priority;- Even forward-only teams store rollback for incident use
Gotchas
- Approving without running EXPLAIN - trust but verify on staging clone. Fix: reviewer spot-check weekly.
- Migration only in app PR - missing from platform repo. Fix: single source of truth path in CONTRIBUTING.
- Security definer functions without search_path - privilege escalation. Fix:
SET search_path = pg_catalog, app. - Granting ALL to app role - violates least privilege. Fix: table-level CRUD only.
- Copy-paste advisor index - duplicate index. Fix:
pg_indexesscreenshot in PR.
Alternatives
| Alternative | Use When | Don't Use When |
|---|---|---|
| Pairing session | Junior first SQL PR | Routine trivial migration |
| Automated sqlfluff/sql-lint | Style and obvious anti-patterns | EXPLAIN judgment |
| DBA office hours | High volume team | Blocking small fixes |
FAQs
Require EXPLAIN for one-line fix?
If query shape unchanged and not hot path, note exemption. Any filter/join/index change on large table requires EXPLAIN.
Who is mandatory reviewer for HIGH lock migrations?
At least one DBA or staff engineer from rotation list. Two-person rule for prod apply.
Review seed/fixture SQL?
Yes for PII and prod-like volume. Dev seeds still should not use superuser patterns that prod forbids.
Related
- Git for Database Work Basics - branch flow
- EXPLAIN Review Skill - plan rubric
- Migration Safety Skill - lock score
- Pairing on Query PRs - live review
- Query Review Standards - team policy
Stack versions: This page was written for PostgreSQL 18.4 and application stacks using Flyway 10+ migrations.