Code review db

Code Review — Database & Persistence Layer

Focus on performance, correctness, and safety at the data layer.

---

Query Correctness

• N+1 queries — a loop that loads related records per item. Look for for item in items: item.related. Fix with eager loading (select_related, include, JOIN) or a batch fetch outside the loop.
• Unbounded queries — find_all(), SELECT * with no LIMIT, WHERE 1=1. Every read from a growing table needs pagination or a LIMIT.
• Missing WHERE clause — an UPDATE or DELETE with no filter deletes/updates every row.
• Implicit full-table scan — a WHERE on a column with no index. Check EXPLAIN output or migration history.
• Fetching more columns than needed — SELECT * when only 2 columns are used. Unnecessary data transfer and harder to cache.

---

Index Strategy

• Missing index on foreign key — FK columns not in an index cause full scans on JOIN and cascade operations.
• Missing index on WHERE/ORDER BY columns — any column used in a filter, sort, or join should have an index unless the table is tiny (<1k rows).
• Index on every column — over-indexing slows writes. Indexes serve reads; add them only for proven query patterns.
• Composite index column order — a composite index on (A, B, C) helps queries filtering A, or A+B, but NOT B alone. Confirm the index covers the actual query.
• Index on high-cardinality boolean — a boolean column index is almost never useful (50/50 selectivity).

---

Transactions & Consistency

• Two writes without a transaction — if the second write fails, the first is orphaned. Wrap in a transaction.
• Transaction too wide — a transaction that spans an HTTP call, a file system operation, or a sleep holds locks and causes contention. Keep transactions as short as possible.
• Read-your-own-writes — reading back a record inside a write transaction without using the same transaction/session. Causes stale reads in read-replica setups.
• Lost update — two concurrent reads followed by two concurrent writes without optimistic or pessimistic locking.
• Missing rollback on error — exception handling that logs and continues without rolling back a partially-committed transaction.

---

Migration Safety

• Adding NOT NULL column without default — will fail on a live table with existing rows. Add with a default, then remove the default in a separate migration.
• Renaming a column — breaks code still pointing to the old name. Dual-write then clean up.
• Dropping a column — ensure no code still reads it before dropping; deploy code first, then migrate.
• Adding an index on a large live table — use CREATE INDEX CONCURRENTLY (Postgres) or equivalent to avoid table lock.
• Missing index on new FK — verify the migration adds an index alongside the foreign key constraint.

---

Connection & Performance

• Connection leak — opening a DB connection without a with/using/defer or a try/finally. Connections should always be returned to the pool.
• Query in a hot path — a query inside a render loop, a per-request middleware, or a tight retry loop. Cache or move upstream.
• No query timeout — long-running queries block connections. Always set statement_timeout / command_timeout in production.

---

See Also

• .claude/skills/code-review/SKILL.md — general review checklist for all layers
• .claude/skills/design-patterns/SKILL.md — Repository pattern for isolating DB access
• .claude/skills/clean-architecture/SKILL.md — keeping domain logic out of the persistence layer