SQL Code Review Checklist: Correctness, Performance, Safety, and Style
SQL slips through code review more easily than application code. It often arrives embedded in strings, migrations, or ORM escape hatches, and many reviewers skim it because "it's just a query". Yet a single bad query can leak data, lock a table, or silently return wrong numbers to a dashboard for months. This checklist gives you a repeatable order of attack: correctness first, then performance, then safety, then style — with concrete examples of what to look for at each stage.
1. Correctness: Does It Return the Right Rows?
- Is the JOIN type intentional? An INNER JOIN silently drops rows with no match. If the requirement is "all customers, with orders if any", it must be a LEFT JOIN.
- Do WHERE conditions on the right table of a LEFT JOIN belong in the ON clause? A filter like
o.status = 'paid'in WHERE turns a LEFT JOIN back into an INNER JOIN, because NULL rows fail the condition. - Are NULLs handled explicitly?
col = NULLis never true; it must becol IS NULL. Watch for NOT IN against a subquery that can return NULL — it returns no rows at all. - Is the GROUP BY complete? Every non-aggregated column in the SELECT list must appear in GROUP BY. MySQL's legacy mode used to pick arbitrary values silently.
-- BUG: WHERE filter defeats the LEFT JOIN SELECT c.name, o.total FROM customers c LEFT JOIN orders o ON o.customer_id = c.id WHERE o.status = 'paid'; -- drops customers with no orders -- FIX: move the filter into the ON clause SELECT c.name, o.total FROM customers c LEFT JOIN orders o ON o.customer_id = c.id AND o.status = 'paid';
NULL traps deserve their own review pass — our guide on SQL NULL handling best practices covers the full list.
2. Performance: Will It Scale Past the Test Database?
- SELECT * in production code — fetches columns nobody uses, breaks when the schema changes, and can prevent index-only scans. Ask for an explicit column list.
- Missing or too-broad WHERE clause — a query that scans a whole table works fine on 10,000 test rows and times out on 50 million production rows.
- Can an index actually be used? Wrapping an indexed column in a function —
WHERE UPPER(email) = ...orWHERE DATE(created_at) = ...— usually disables the index. Rewrite as a range: created_at >= start AND created_at < end. - Implicit type casts — comparing a VARCHAR column to a number (
WHERE phone = 9876543210) forces a cast on every row and skips the index. Match the types. - Leading-wildcard LIKE —
LIKE '%term%'cannot use a normal B-tree index; flag it on large tables.
-- Index on created_at is unusable: WHERE DATE(created_at) = '2026-07-01' -- Index-friendly rewrite: WHERE created_at >= '2026-07-01' AND created_at < '2026-07-02'
When in doubt, ask the author to attach an EXPLAIN plan from a realistic dataset. More techniques in SQL optimization for large tables.
3. Safety: Can It Destroy Data or Leak It?
- SQL injection via string concatenation — any query built by gluing user input into a string is a blocking issue, full stop. Require parameterised queries or prepared statements.
- UPDATE or DELETE without WHERE — even when intentional (a full-table backfill), it should be called out in the PR description and ideally wrapped in a transaction with a row-count sanity check.
- Migrations that lock big tables — adding a NOT NULL column with a default, or building an index without CONCURRENTLY in PostgreSQL, can freeze production traffic.
- Overly broad grants or exposed PII — does the query return columns (emails, phone numbers) the consuming service does not need?
-- BLOCK THIS: injection via concatenation query = "SELECT * FROM users WHERE name = '" + userInput + "'"; -- REQUIRE THIS: parameterised query query = "SELECT id, name FROM users WHERE name = ?"; db.execute(query, [userInput]); -- BLOCK THIS unless explicitly justified: DELETE FROM sessions; -- no WHERE clause
4. Style: Is It Readable and Consistent?
- Consistent keyword casing per your team standard (see the casing debate).
- Meaningful table aliases —
customers cis fine;t1, t2, t3is not. - One JOIN and one condition per line for anything non-trivial.
- CTEs instead of deeply nested subqueries in long queries.
- Comments that explain business rules ("refunds excluded per finance policy"), not syntax.
Style feedback is the cheapest to automate: agree on a format, run every query through a formatter before the PR, and reviewers never need to comment on layout again.
The Checklist in One Place
CORRECTNESS [ ] JOIN types match the requirement (INNER vs LEFT) [ ] Right-table filters in ON, not WHERE, for LEFT JOINs [ ] NULLs: IS NULL / COALESCE used; no "= NULL"; NOT IN checked [ ] GROUP BY lists every non-aggregated column PERFORMANCE [ ] No SELECT * in production code [ ] WHERE clause present and selective [ ] No functions wrapping indexed columns [ ] No implicit casts (types match on both sides) [ ] EXPLAIN plan attached for heavy queries SAFETY [ ] Parameterised queries only — no string concatenation [ ] UPDATE/DELETE have WHERE (or explicit justification) [ ] Migrations checked for long locks [ ] No unnecessary PII columns returned STYLE [ ] Formatted with the team formatter before review [ ] Meaningful aliases, comments explain "why"
Frequently Asked Questions
Check correctness first: is the JOIN type right, are NULLs handled with IS NULL and COALESCE rather than equality, and does the GROUP BY include every non-aggregated column? A fast wrong query is worse than a slow correct one, so correctness always comes before performance.
Two candidates: SQL injection through string concatenation of user input, and UPDATE or DELETE statements without a WHERE clause. The first is a security breach waiting to happen; the second can wipe or corrupt an entire table in one statement. Both should block a merge immediately.
Formatting should be automated, not debated in review. Run queries through a formatter such as the free Dev Brains AI SQL Formatter before opening a pull request, so reviewers spend their time on logic, performance, and safety instead of indentation.