A Self-Review Checklist for Stored Procedure Changes

The easiest bug to fix is the one you catch before another reviewer sees it. A self-review checklist gives a stored procedure change ten honest minutes before it becomes somebody else's emergency.

Start the Self-Review Checklist With the Same Workload

I run the old and new procedures against a restored copy that resembles production in volume and distribution. A tiny test database hides sorts, spills, poor estimates, and lock pressure. Keep the parameter set that caused the change, then add common and unusual values. Capture the output before touching the code. If the procedure writes rows, capture the affected state as well. Compare results in both directions and pay attention to duplicates. A check that only counts rows can miss a wrong value replacing a right one.

Ask yourself which input is least convenient to test. That is usually the one worth trying next. Save the chosen parameters with the review notes. A future reviewer should be able to rerun the same cases without recreating your thought process.

Read IO Before You Read Percentages

Turn on SET STATISTICS IO and execute the old and new versions under comparable conditions. Record the logical reads by table, not a single grand total. A change that helps the main table but turns a small lookup into repeated scans deserves a closer look. Physical reads vary with cache state, so do not treat one cold run as a verdict. Actual execution plans expose estimates, actual rows, joins, sorts, and spills. The graphical cost percentages are estimates even in an actual plan. I check the operators that process far more rows than expected. The block below wraps a simple catalog query, so put your old and new procedure calls in its place.

SET STATISTICS IO ON;
SET STATISTICS TIME ON;
SELECT name FROM sys.objects WHERE type = 'P';
SET STATISTICS IO OFF;
SET STATISTICS TIME OFF;

Try Inputs the Happy Path Avoids

Pass NULL where the contract permits it. Pass an empty string, an empty table-valued parameter, a boundary date, and a value with no matching rows. Confirm whether NULL means all rows, no rows, or an error. Do not let implicit conversion decide that contract for you. Test duplicate values and values with unusual characters when the procedure builds dynamic SQL. Check output parameters and return codes, since a caller can use them even when the result grid looks correct.

I have seen an optional filter work for every demonstrated value and fail on NULL. The query plan was fine. The predicate was not. Make the expected behavior explicit in an assertion or test note so reviewers can distinguish an intended empty result from a missed branch.

Make Failure Roll Back Cleanly

Use a disposable database copy to trigger a predictable error after work has begun. Confirm that the procedure leaves no open transaction and no partial changes. SET XACT_ABORT ON is a useful default for procedures that own a transaction, paired with TRY…CATCH and a rollback when XACT_STATE() reports an active transaction. Rethrow the original error after cleanup. Do not swallow it into a cheerful success message. That is the database version of hiding smoke behind a curtain. The block below ends with error 50001 on purpose. First it shows transaction state 0 after the rollback, then THROW hands the original error back to the caller.

SET XACT_ABORT ON;
BEGIN TRY
    BEGIN TRANSACTION;
    CREATE TABLE #ReviewRollback (id int NOT NULL);
    INSERT #ReviewRollback (id) VALUES (1);
    THROW 50001, 'Review failure path', 1;
    COMMIT TRANSACTION;
END TRY
BEGIN CATCH
    IF XACT_STATE() <> 0 ROLLBACK TRANSACTION;
    SELECT XACT_STATE() AS transaction_state_after_rollback;
    THROW;
END CATCH;

Inspect Dependencies and Permissions

Check the procedure's callers, referenced objects, ownership chain, EXECUTE permissions, and scheduled jobs. A rename inside a procedure can be harmless, while a changed output column name breaks an application that reads by name. Search application code where available. Dynamic SQL and cross-database references do not always appear cleanly in dependency views. Compare the procedure definition and its grants after deployment to a test environment. If a deployment drops and recreates the procedure, permissions and object identifiers can change. ALTER is usually kinder to the existing contract.

Do not approve a performance change solely because its test case improved. Check plans for selective and broad parameter values. Parameter sensitivity can turn one fast test into a slow production branch. Record which values you tried and why.

Add the Rollback Script to the Self-Review Checklist

Keep the previous definition in a reviewed rollback script. Include any schema or data reversal needed, and say when reversal stops being safe. If the procedure writes new data, restoring code alone cannot erase those writes. Agree on the observation period and who decides to reverse. I write this before the change window because emergency typing is not a reliable deployment method.

A rollback script should be executable in the target environment, with expected permissions and dependencies present. Test it on a copy. Then redeploy the new version so both directions are known. Record the exact commands the operator will use, not only a sentence that says "roll back if needed."

End the Self-Review Checklist With Your Own Diff

Read every changed line as if another person wrote it. Look for predicates that changed from AND to OR, joins that lost keys, row-limiting changes, and error handlers that mask failures. Check that temporary objects are scoped and cleaned up naturally. Search for debug SELECT statements and hard-coded test values. Review comments for accuracy after the code moved. A misleading comment is worse than no comment because it sends the next maintainer in the wrong direction.

I also read the procedure once without looking at the diff. Does the complete flow still make sense? Small changes can create a large contradiction when two branches meet. A final end-to-end run is worth more than a tidy-looking patch.

Keep the review short enough to repeat on every change. A checklist that takes a full day will be skipped under pressure. I separate mandatory checks from deeper investigation triggered by a plan change or new write path. The author should finish the basic pass before asking another person to review. Then the reviewer can focus on intent, not discover that NULL was never tried.

Hand Over Evidence, Not Confidence

Give the reviewer the baseline case list, output comparison, IO and plan observations, failure-path result, deployment script, and rollback script. Keep each item brief and reproducible. State any test you could not perform. "Works on my machine" is a location, not evidence. The reviewer can focus on judgment when the mechanical checks are already done.

The self-review checklist should evolve after defects. Add a missed case when an issue escapes, and remove steps that produce no useful signal. Self-review is not paperwork. It is a habit of trying to prove your change wrong before production gets a turn.

Related reading on this blog: AI Code Review Bottleneck: Nobody Owns It and Pre-Code Review Tips: Tips For Enforcing Coding Standards.

A review is not a signature, it is a deliberate attempt to break your own change.

Published by Pinal Dave on SQLAuthority. More of my work at pinaldave.com.