what experienced reviewers actually look for
An SQL review that only looks at indentation and variable names misses the bugs that become truly expensive in production later. A systematic checklist covering readability, correctness, performance red flags and security checks makes SQL code reviews reproducible instead of dependent on a reviewer's gut feeling.
Table of Contents
- 1. Why SQL code reviews need their own rules
- 2. Readability and formatting as the basis of every review
- 3. Correctness: checking NULL handling, joins and aggregation
- 4. Spotting performance red flags in the review
- 5. Security: checking injection and privilege grants in the review
- 6. Assessing transaction boundaries and concurrency
- 7. Safeguarding migrations and schema changes in the review
- 8. Establishing the checklist as a reusable team tool
- 9. Automated linters versus manual review compared
- 10. Summary
- 11. FAQ
1. Why SQL code reviews need their own rules
An SQL code review differs fundamentally from a review for application code, because the exact same statement can show completely different behavior depending on data volume, index situation and concurrent access. An application function that runs correctly in a unit test behaves identically in production. An SQL query that runs against an empty test database in milliseconds can block for minutes against a production table with ten million rows, without the code itself being syntactically wrong.
This peculiarity makes a dedicated SQL code review checklist necessary, one that goes beyond the usual criteria for application code: readability is necessary but not sufficient. An SQL review must additionally cover NULL semantics, index usage, transaction boundaries and injection risks systematically, otherwise exactly the bugs that nobody looks for in an application code review, because they simply do not occur there, stay undetected. The sections below build this checklist step by step.
2. Readability and formatting as the basis of every review
Before a reviewer digs into the content of an SQL change, a quick look at formatting and naming pays off, because poorly readable SQL code additionally obscures content errors. Consistent capitalization of keywords, speaking alias names instead of a, b, c, and clear indentation in multi-line JOIN chains make a query reviewable in the first place. A reviewer who needs ten minutes to decipher the structure of a query has less capacity left to question the actual logic.
A second aspect of readability concerns explicit column lists instead of SELECT *. A query with SELECT * is not only harder to follow, because it stays unclear which columns the application actually needs, but also more fragile: if a column is later added to the table, the query's result changes silently, without the code itself being touched. A reviewer should generally question SELECT * in production code, except in explicit exceptions like existence checks with EXISTS.
-- REVIEW FLAG: unreadable, unmaintainable
SELECT a.*, b.*
FROM orders a, customers b
WHERE a.customer_id = b.id AND a.status = 'paid';
-- BETTER: explicit columns, explicit JOIN, readable aliases
SELECT
orders.id AS order_id,
orders.total_amount,
customers.email AS customer_email
FROM orders
INNER JOIN customers ON customers.id = orders.customer_id
WHERE orders.status = 'paid';
3. Correctness: checking NULL handling, joins and aggregation
NULL values are the most common source of silent logic errors in SQL, because they do not behave like a normal value. A comparison WHERE column = NULL never returns true, even if the column actually is NULL, and a reviewer who does not know this bug lets it through. Also, NOT IN with a subquery that returns even a single NULL row surprisingly returns no rows at all, a classic among SQL traps that every checklist should explicitly cover.
For joins, it must be checked whether the chosen join type actually matches the intended semantics. An INNER JOIN where a LEFT JOIN was actually meant silently filters out rows whose associated child rows are missing, for example customers without orders in a report that is supposed to show all customers. For aggregations with GROUP BY, the reviewer should check whether all non-aggregated columns appear in the GROUP BY clause, because some databases do not enforce this and unpredictable results can arise from ambiguous groupings.
-- REVIEW FLAG: NOT IN with a subquery that can return NULL
-- If ANY row in the subquery has a NULL customer_id, this returns ZERO rows
SELECT * FROM customers
WHERE id NOT IN (SELECT customer_id FROM orders);
-- BETTER: NOT EXISTS handles NULL correctly
SELECT * FROM customers c
WHERE NOT EXISTS (
SELECT 1 FROM orders o WHERE o.customer_id = c.id
);
-- REVIEW FLAG: comparing to NULL never matches
SELECT * FROM customers WHERE deleted_at = NULL; -- always empty result
-- CORRECT: use IS NULL
SELECT * FROM customers WHERE deleted_at IS NULL;
4. Spotting performance red flags in the review
A reviewer does not need to run a full EXPLAIN plan analysis on every query, but should recognize certain patterns as immediate red flags. A function on the left side of a WHERE condition, for example WHERE YEAR(created_at) = 2026, prevents index usage on created_at in most database systems, because the index is built on the raw value, not on the function result. A LIKE '%searchterm%' with a leading wildcard also prevents index usage and should be questioned on larger tables.
A second common pattern is the N+1 problem: a loop in application code that issues a separate database query per element, instead of loading all needed data in a single query with JOIN or WHERE id IN (...). This pattern is often not visible in the SQL code itself, but only in the surrounding application code, which is why an SQL review should ideally include the calling context, not just the isolated query.
-- REVIEW FLAG: function on indexed column prevents index usage
SELECT * FROM orders WHERE YEAR(created_at) = 2026;
-- BETTER: range condition keeps the index usable
SELECT * FROM orders
WHERE created_at >= '2026-01-01' AND created_at < '2027-01-01';
-- REVIEW FLAG: leading wildcard prevents index usage
SELECT * FROM products WHERE name LIKE '%widget%';
-- BETTER: full-text index or trailing wildcard where semantics allow it
SELECT * FROM products WHERE name LIKE 'widget%';
5. Security: checking injection and privilege grants in the review
SQL injection remains one of the most common critical vulnerabilities, even though the fix has been known for decades: parameterized queries instead of string concatenation of user input. A reviewer must identify every spot in the diff where a variable is inserted directly into an SQL string instead of being passed as a bound parameter, regardless of how unlikely an attack at that spot seems. Dynamically composed table or column names that cannot be parameterized must also be checked against a fixed allowlist instead of accepting user input unchecked.
Besides the injection check, privilege grants belong in every SQL code review checklist. A new database user or a new script that works with far-reaching privileges like GRANT ALL, even though only read access to two tables is needed, violates the principle of least privilege and unnecessarily widens the attack surface in case of a compromised account. A reviewer should explicitly check every GRANT statement against actual need.
-- REVIEW FLAG: overly broad privilege grant for a reporting service account
GRANT ALL PRIVILEGES ON ALL TABLES IN SCHEMA public TO reporting_service;
-- BETTER: least privilege — only what the service actually reads
GRANT SELECT ON orders, order_items, customers TO reporting_service;
REVOKE ALL ON audit_log FROM reporting_service;
-- REVIEW FLAG: string concatenation, classic injection vector
-- query = "SELECT * FROM customers WHERE email = '" + userInput + "'"
-- BETTER: parameterized query, user input never touches the SQL string
-- query = "SELECT * FROM customers WHERE email = $1"; params = [userInput]
6. Assessing transaction boundaries and concurrency
Transaction boundaries are easy to overlook in code review because they are often set implicitly by the framework, but they have a massive impact on concurrency and data integrity. A transaction that is scoped too broadly and includes external, slow operations like an HTTP call holds database locks unnecessarily long and blocks other transactions. A transaction that is scoped too narrowly and spreads related write operations across separate transactions risks inconsistencies if the second operation fails after the first has already been committed.
For concurrent write operations, a reviewer should check whether race conditions are possible, for example a SELECT followed by a separate UPDATE, between which another process can change the same row. In such cases, either an atomic UPDATE ... WHERE with a condition on the expected starting value, or an explicit lock with SELECT ... FOR UPDATE is needed, depending on the database's isolation level.
-- REVIEW FLAG: read-then-write race condition
SELECT stock_quantity FROM products WHERE id = 42;
-- ... application checks stock_quantity > 0, then in a SEPARATE statement:
UPDATE products SET stock_quantity = stock_quantity - 1 WHERE id = 42;
-- Between the two statements, another transaction can also decrement stock
-- BETTER: atomic conditional update, no race condition possible
UPDATE products
SET stock_quantity = stock_quantity - 1
WHERE id = 42 AND stock_quantity > 0;
-- Check affected row count in the application: 0 rows means out of stock
7. Safeguarding migrations and schema changes in the review
Schema migrations need a separate, stricter check in review than plain data queries, because a mistake here can mean an outage or data loss instead of just a wrong result. A reviewer should check whether the migration has a rollback path, whether destructive operations like DROP COLUMN happen in a separate, later migration after a transition period, and whether new NOT NULL constraints bring a sensible default value for existing rows.
Additionally, an assessment of locking behavior belongs in every migration review: for large tables, the reviewer should explicitly ask whether the migration triggers a short metadata change or a long table rewrite, and whether that fact has been coordinated with the operations team before scheduling the migration in a maintenance window or during low-traffic hours.
8. Establishing the checklist as a reusable team tool
A checklist that only exists in the head of the most experienced developer does not scale to a growing team. The next step is to turn the named criteria into a documented, versioned pull request template that appears automatically as a checkbox list for every SQL change. This reduces dependency on individual reviewers and makes the review process traceable for new team members, without knowledge having to be passed on verbally.
It is important to keep the checklist alive: every production incident that traces back to an overlooked SQL error should feed into the checklist as a new item, so the same type of error is guaranteed to be caught next time. This continuous expansion turns a static list into a learning system that grows with the team's actual error patterns.
9. Automated linters versus manual review compared
Not every point on the checklist needs to be checked manually. Static SQL linters like sqlfluff or database-specific analyzers automatically cover part of the criteria and free up the human reviewer for the points that require real contextual knowledge.
| Criterion | Checkable automatically | Needs manual review |
|---|---|---|
| Formatting, naming conventions | Yes, via linter | No |
| SELECT * usage | Yes, via linter rule | Assess exceptions |
| NULL handling logic errors | Partially | Yes, always |
| Index usage, EXPLAIN analysis | No, without live data | Yes, always |
| SQL injection risk | Partially, static analysis | Yes, always |
| Transaction boundaries, race conditions | No | Yes, always |
The pragmatic approach combines both layers: linters catch the mechanical, clearly rule-based violations automatically, before a human reviewer even looks at the pull request. That frees the reviewer to focus on the points that require real understanding of data distribution, concurrency and system architecture, instead of spending time on formatting discussions.
Mironsoft
SQL quality assurance, code reviews and database architecture for Magento and beyond
SQL reviews that do not wave critical bugs through?
We establish documented SQL review checklists, combine automated linters with targeted manual checks, and train teams on the most common SQL traps.
Checklist design
Developing individual SQL review checklists for pull request templates
Linter integration
Embedding sqlfluff and static analysis tools into existing CI pipelines
Team training
Workshops on NULL handling, injection risks and performance red flags
10. Summary
An SQL code review checklist turns a subjective gut feeling into a reproducible, team-wide standard. Readability and formatting are the entry point, but the real impact comes from systematically checking NULL handling, join semantics, performance red flags such as functions on indexed columns, injection risks and transaction boundaries.
Migrations and schema changes deserve their own, stricter review stage, because mistakes here can mean data loss instead of just wrong results. Automated linters take over the mechanical, rule-based criteria and free the human reviewer for the points that require real contextual knowledge. Anyone who documents, versions, and expands the checklist after every production-relevant incident builds a learning system that grows with the team.
SQL Code Review Checklist — The Essentials at a Glance
NULL handling
NOT IN with subqueries that can contain NULL, and comparisons with NULL, are classic SQL traps.
Performance red flags
Functions on indexed columns and leading wildcards in LIKE prevent index usage.
Security
Parameterized queries instead of string concatenation, privilege grants following least privilege.
Automation
Linters for mechanical criteria, manual review for contextual knowledge like concurrency.