PostgreSQL code review standards

# PostgreSQL code review standards

Follow these standards when writing or changing code in this repository. Code review flags changes that break them.

## Create PostgreSQL Indexes Concurrently on Live Tables

Applies to `**/*.sql`, `**/migrations/**`, `**/migrate/**`.

Review PostgreSQL migrations for index builds and drops that block reads or writes on live tables.

Flag changes that:
- Run `CREATE INDEX` or `CREATE UNIQUE INDEX` on an existing table without `CONCURRENTLY`. A plain build holds a SHARE lock that blocks INSERT, UPDATE, and DELETE until the transaction ends.
- Put `CREATE INDEX CONCURRENTLY`, `DROP INDEX CONCURRENTLY`, or `REINDEX ... CONCURRENTLY` inside a transaction block: an explicit `BEGIN`, a framework's per-migration transaction (Rails without `disable_ddl_transaction!`, Django without `atomic = False`), or a multi-statement file the runner sends as one query string, which PostgreSQL runs as one implicit transaction.
- Use `CREATE INDEX CONCURRENTLY IF NOT EXISTS` with no cleanup of a failed earlier build. A failed concurrent build leaves an INVALID index that queries ignore and writes still maintain, and `IF NOT EXISTS` skips the rebuild because the name exists.
- Drop an index on a live table with plain `DROP INDEX`, which takes an ACCESS EXCLUSIVE lock on the table, instead of `DROP INDEX CONCURRENTLY`.
- Rebuild an index with plain `REINDEX`, which blocks writes to the table and reads that use the index, instead of `REINDEX ... CONCURRENTLY`.

Do not flag:
- Indexes on tables created earlier in the same migration.
- Schema dumps, seed files, and test fixtures, such as `structure.sql` or `schema.sql`, that describe the schema and do not run against a live database.
- Lookup or enum tables that the schema or PR shows are small and rarely written.

## Set lock_timeout Before Blocking DDL in PostgreSQL

Applies to `**/*.sql`, `**/migrations/**`, `**/migrate/**`.

Review PostgreSQL migrations for DDL that can wait in the lock queue and stall all traffic to a table.

Flag changes that:
- Run `ALTER TABLE` on an existing table without setting `lock_timeout` earlier in the same session or transaction. Most forms, including ADD COLUMN, DROP COLUMN, ALTER COLUMN TYPE, SET DEFAULT, SET NOT NULL, ADD CHECK, and RENAME, take an ACCESS EXCLUSIVE lock.
- Run other lock-heavy DDL on live tables without `lock_timeout`: ADD FOREIGN KEY (which also locks the referenced table), `CREATE TRIGGER`, `TRUNCATE`, `DROP TABLE`, or `LOCK TABLE`.
- Weaken the limit: `SET LOCAL lock_timeout` outside a transaction block (it has no effect there), `lock_timeout = 0` (no limit), or a value in minutes.
- Hold an ACCESS EXCLUSIVE lock across slow work in the same transaction, such as a backfill, a full-table scan, or DDL on several busy tables in a row.
- Retry a lock timeout in a loop with no attempt limit or delay between attempts.

Do not flag:
- DDL on tables created earlier in the same migration.
- Migrations where the runner configuration, the database role settings, or a shared migration helper already sets `lock_timeout`.
- `CREATE INDEX CONCURRENTLY`, `DROP INDEX CONCURRENTLY`, and `ALTER TABLE ... VALIDATE CONSTRAINT`, whose locks do not conflict with reads or writes.

## Add PostgreSQL Constraints Without Long Table Locks

Applies to `**/*.sql`, `**/migrations/**`, `**/migrate/**`.

Review PostgreSQL migrations for constraint and column changes that scan or rewrite a table while holding a blocking lock.

Flag changes that:
- Add a FOREIGN KEY or CHECK constraint to an existing table without `NOT VALID`. The validation scan runs while writes are blocked, on both tables for a foreign key.
- Run `VALIDATE CONSTRAINT` in the same transaction as the `NOT VALID` add, which keeps the add's lock held through the scan.
- Run `ALTER COLUMN ... SET NOT NULL` on an existing large table without a validated `CHECK (col IS NOT NULL)` constraint. PostgreSQL 12 and later skip the full scan when that constraint exists.
- Add a UNIQUE or PRIMARY KEY constraint to an existing table with `ALTER TABLE ... ADD CONSTRAINT`, which builds the index under ACCESS EXCLUSIVE. Build it with `CREATE UNIQUE INDEX CONCURRENTLY`, then attach it with `ADD CONSTRAINT ... UNIQUE USING INDEX` (or `PRIMARY KEY USING INDEX` on columns that are already `NOT NULL`).
- Change a column type in a way that rewrites the table and rebuilds its indexes, such as `integer` to `bigint` or a `USING` expression that transforms values.
- Add a column with a volatile default, such as `gen_random_uuid()` or `clock_timestamp()`, which rewrites every row.

Do not flag:
- Constraints and column changes on tables created earlier in the same migration.
- `VALIDATE CONSTRAINT` in its own migration, which is the safe second step.
- Changes PostgreSQL applies without a rewrite, such as `varchar(n)` to `text`, raising a `varchar` length limit, or ADD COLUMN with a constant default.

## Batch Large Backfills and Deletes in PostgreSQL

Applies to `**/*.sql`, `**/migrations/**`, `**/migrate/**`.

Review PostgreSQL migrations, deploy scripts, and one-off data scripts for UPDATE and DELETE statements that touch millions of rows in one transaction.

Flag changes that:
- Run a single `UPDATE` or `DELETE` over most or all of a large table, such as a backfill with no `WHERE` clause or a retention delete on `created_at < now() - interval '90 days'`, in a migration or deploy step.
- Backfill in the same transaction as `ALTER TABLE` on that table, so the ACCESS EXCLUSIVE lock stays held until the whole backfill commits.
- Batch inside one transaction, such as a PL/pgSQL loop in a function (functions cannot `COMMIT`) or a loop that never commits, which keeps every row lock and old row version until the end.
- Page through batches with `OFFSET`, or with a filter like `WHERE col IS NULL LIMIT n` that no index supports, so each batch scans more rows than the last. Walk the primary key in ranges instead.
- Cannot resume after a failure: no condition that skips finished rows and no saved progress, so a rerun repeats non-idempotent changes such as `SET amount_cents = amount_cents * 100`.

Do not flag:
- Statements bounded to a small, known set of rows, such as a `WHERE id IN (...)` list or a small lookup table.
- Data changes on tables created earlier in the same migration.
- Backfills handed to a background job that already processes rows in committed batches.

## Index Foreign Keys and New Query Filters in PostgreSQL

Applies to `**/*.sql`, `**/migrations/**`, `**/migrate/**`.

Review PostgreSQL schema changes and SQL queries for foreign keys and new query patterns that lack a usable index.

Flag changes that:
- Add a foreign key whose referencing columns are not the leading columns of any index on that table. PostgreSQL does not index the referencing side automatically, so each parent delete or key update, including an `ON DELETE CASCADE`, scans the child table.
- Drop or replace the only index whose leading columns cover a foreign key.
- Add a query, view, or function that filters, joins, or sorts on columns of a large table with no index whose leading columns match, such as `WHERE status = $1 ORDER BY created_at DESC LIMIT 50`.
- Add an index the new query cannot use: columns in the wrong order for the filter and sort, an index on `email` for a filter on `lower(email)`, or a partial index whose predicate the query's `WHERE` clause does not imply.
- Search a large table with `LIKE` or `ILIKE` and a leading wildcard, which a B-tree index cannot serve; a `pg_trgm` GIN or GiST index can.

Do not flag:
- Foreign keys covered by the leading columns of an existing index, including a composite primary key such as `PRIMARY KEY (order_id, product_id)` for a key on `order_id`.
- Child tables that stay small, such as lookup or configuration tables.
- One-off maintenance or analytics queries that do not run on a request path.

## Block Unsafe Database Migrations

Review database migrations for operations that can lock large tables, rewrite full tables, run long backfills, create blocking indexes, or require full table scans. Prefer phased rollouts, concurrent indexes, nullable columns followed by backfills, batched data changes, and explicit rollback or recovery plans for risky schema changes.

## Prevent Database Query Performance Regressions

Flag database access patterns that can degrade production performance, including N+1 queries, unbounded list endpoints, missing pagination, full-table scans, missing indexes on selective filters, per-row writes in loops, and queries that load more columns or rows than needed. Prefer bounded reads, batching, selective indexes, and query shapes that scale with table size.

## Enforce Rollback Steps in Database Migrations

Each database migration file must include a "down" or "rollback" section that reverses the changes. This ensures all database changes can be safely undone if deployment issues occur. Migration files should clearly separate "up" and "down" migrations using appropriate method names or comments.

## Flag Security Vulnerabilities

Scan for security flaws: hardcoded secrets, SQL injection, XSS, weak auth, exposed data, poor crypto, unsafe deserialization, insecure HTTP calls in production, and more. Ensure all network requests use HTTPS/TLS. Identify attack vectors and security violations to maintain robust defenses.

## Prevent Sensitive Data in Logs

Logging, error reporting, analytics events, and audit trails must not expose sensitive data. Flag code that logs PII, emails, passwords, API keys, auth tokens, session identifiers, payment data, or raw request/response bodies without redaction. Error serialization must preserve enough context to debug while sanitizing secrets and user data.

## Enforce Pragmatic Test Coverage

Be reasonable when evaluating tests. Flag changes when important behavior is untested, but do not require tests for every export, type, config, metadata file, or mechanical change.

Flag PRs that:
- Change core business logic without covering the main success and failure paths.
- Add UI behavior without testing the user-visible interaction or state change.
- Add tests that only assert rendering, existence, snapshots, or that a function returns any value.
- Skip, mute, or weaken existing tests without a clear reason.
- Mock so much of the system that the test no longer validates the behavior the PR claims to change.