Code Review – RockyPC/SQLStorm – 2026-09-27 17:05:17 UTC

Executive summary

Only the most recent objects provided in the input were reviewed. All 100 supplied stored-procedure definitions were labelled complete. No functions, triggers, or views were provided. No objects were marked NOT INCLUDED or NOT AVAILABLE.

1. Correct the high-impact data and cardinality errors in dbo.sp00060. Its line 79 join compares a user ID to a post ID, which can produce incorrect results. The procedure also performs a non-sargable tag search at lines 21–25 and has an expensive multi-join aggregate at lines 36–45. Confidence: High.
2. Remove the five-second transaction-held lock interval in dbo.usp_UpdateTablesWithDelay. Lines 13–36 keep locks from the first update while executing WAITFOR DELAY at line 25. Move the delay before BEGIN TRANSACTION. Confidence: High.
3. Do not use the supplied procedures as production workload evidence without correcting their fixed dates and cross joins. Many procedures use literal dates based on 2024-10-01, and several intentionally or accidentally create Cartesian-style joins. Query Store shows zero executions for most supplied objects, so prioritization is based primarily on code risk rather than measured runtime. Confidence: High.
4. Defer index changes. Every displayed index has zero seeks, scans, lookups, and updates during the 37.5-hour observation window. The window is too short and workload coverage is not established; no index should be dropped solely because of these counters. Confidence: High.

Review scope and inventory

The largest tables are dbo.Votes with 926,084 rows, dbo.PostHistory with 847,593 rows, dbo.Badges with 439,352 rows, dbo.Comments with 351,440 rows, dbo.Users with 267,193 rows, and dbo.Posts with 246,672 rows. These sizes make repeated full-table aggregation particularly important in procedures that scan votes, history, comments, posts, or badges.

Detailed prioritized recommendations

1. Priority 1 — Correct dbo.sp00060 user/post join and reduce duplicate aggregation

   At lines 36–45 of dbo.sp00060, Posts, Votes, and Badges are joined before aggregation. This multiplies rows when a user has multiple posts, votes, and badges; COUNT(DISTINCT p.Id) masks only one part of the multiplication, while the vote sums can be overstated. At line 79, tp.UserId = rp.PostId joins unrelated key domains and is almost certainly incorrect.

   Lines 21–25 also match tags using p.Tags LIKE '%' + t.TagName + '%'. A leading wildcard prevents a normal B-tree seek and can scan the 246,672-row dbo.Posts table and compare against the 1,232-row dbo.Tags table. The supplied index IX_Posts_Question_TagScan is not an effective solution for this predicate because its key is only PostTypeId and the tag predicate remains non-sargable.

   The revised definition in the Scripts section pre-aggregates posts, votes, and badges independently, includes OwnerUserId in the ranked-post CTE, and joins users to their own posts. It preserves the original result columns and read-only behavior while correcting the key mismatch.

   Confidence: High for the join and aggregation defects; medium for the intended tag result semantics because the original description and query shape do not fully specify tag ownership behavior.

2. Priority 2 — Never hold update locks during WAITFOR

   dbo.usp_UpdateTablesWithDelay begins a transaction at line 13, updates dbo.Users at lines 16–19, and then waits five seconds at line 25 before updating dbo.Posts at lines 28–31. Under normal read-committed locking, the first row/key locks remain held until line 36 commits. Concurrent sessions can therefore wait on the user row for the entire delay, and sessions that update the tables in the opposite order can deadlock.

   The safest code-only change is to move the deliberate delay before the transaction begins. The revised procedure in the Scripts section retains the same two updates, transaction atomicity, error handling, result sets, and five-second delay, but does not hold transaction locks while waiting.

   Confidence: High. No actual blocking or deadlock graph was supplied, so the observed impact cannot be quantified.

3. Priority 3 — Validate loop parameters and avoid wasteful random scans

   dbo.usp_UpdateTablesInLoop uses ORDER BY NEWID() at lines 27–29 and 31–33. This generally requires reading and sorting a large portion of dbo.Users and dbo.Posts for every iteration. With 267,193 users and 246,672 posts, repeated executions can be expensive even though Query Store reports zero executions in the last 30 days.

   Line 77 divides by @LoopCount. A caller passing zero causes a divide-by-zero error, while a negative count silently skips the loop and still reaches the percentage calculation. The revised definition validates that @LoopCount is positive and returns a safe zero success rate when appropriate. This is a correctness and defensive-programming improvement; a truly efficient random-row strategy requires statistics or a known contiguous key range that were not supplied.

   Confidence: High for parameter validation; medium for the exact cost of ORDER BY NEWID() because no execution plan or duration was provided.

4. Priority 4 — Replace hard-coded historical dates with runtime semantics where the report is intended to be relative

   Numerous procedures use fixed expressions such as DATEADD(YEAR, -1, '2024-10-01 12:34:56') or DATEADD(DAY, -30, '2024-10-01'). Examples include dbo.sp09995 lines 22–24, dbo.sp09994 lines 24–26, dbo.sp09993 lines 20–22, and dbo.sp09900 lines 21–23. These reports become stale and may return no current data.

   Do not mechanically replace every literal: some may be intentional test fixtures. Where the report is intended to be relative, use a deterministic local variable initialized once, for example inline @AsOfDate = SYSUTCDATETIME(), and calculate all boundaries from that value. No scripts are supplied for this broad change because each procedure's intended reporting period and contract must be preserved independently.

   Confidence: High that the dates are stale for current reporting; low-to-medium regarding the intended business semantics.

5. Priority 5 — Eliminate accidental Cartesian joins and joins on display names

   Several procedures join unrelated row sets using predicates that are not relational keys. Examples include dbo.sp09990 lines 69–74, where popular tags are joined to users on UA.PostsChanged > 0; dbo.sp09974 lines 48–50, which cross joins every qualifying user to every popular post; dbo.sp09895 lines 48–65, which cross joins top users to popular posts; and dbo.sp09881 lines 69–74, which cross joins user statistics and popular tags before joining history by display name.

   Display-name joins also occur in procedures such as dbo.sp09909 lines 65–68, dbo.sp09828 lines 46–49, and dbo.sp09827 lines 53–58. Display names are not guaranteed to be unique or immutable. Use Users.Id, Posts.OwnerUserId, and PostHistory.UserId wherever the intended relationship is available.

   These procedures are marked unused in the supplied 30-day Query Store snapshot, so no replacement definitions are included. They should be corrected before being enabled for production use.

   Confidence: High for the cardinality and key-quality risks.

6. Priority 6 — Pre-aggregate one-to-many relationships before joining

   A recurring pattern joins posts to comments, votes, badges, and sometimes post history in one query, then aggregates afterward. Examples include dbo.sp09991 lines 6–21, dbo.sp09941 lines 6–27, dbo.sp09982 lines 6–27, and dbo.sp09883 lines 51–85. Because comments, votes, badges, and history are independent one-to-many relationships, the join can create a multiplicative intermediate result. COUNT(DISTINCT ...) may repair counts but does not avoid the memory, sort, and spill cost.

   Use separate grouped CTEs or derived tables keyed by the parent ID, then join the compact aggregates. This is the approach used in the revised dbo.sp00060 definition.

   Confidence: High as a general code-review finding; exact benefit is not measurable without plans and runtime statistics.

7. Priority 7 — Treat non-sargable tag searches as a schema-design issue

   Tag searches using expressions such as LIKE '%' + t.TagName + '%' appear in dbo.sp00060 lines 21–25, dbo.sp09990 lines 29–36, dbo.sp09982 lines 47–57, and dbo.sp09881 lines 25–36. STRING_SPLIT usage appears in dbo.sp09915 lines 26–36 and dbo.sp09886 lines 13–20.

   The durable fix is a normalized bridge table such as PostTags(PostId, TagId), populated transactionally with the post. That object was not supplied, and introducing it would require a data migration and application changes, so no executable script is recommended in this report.

   Confidence: High for the access-path limitation; medium for migration suitability.

Locking and blocking analysis

No blocking-chain data, wait statistics, deadlock graph, transaction log evidence, or execution plans were supplied. Consequently, no blocking diagram can be rendered and no wait duration or deadlock frequency can be stated from the input.

The procedure's SET XACT_ABORT ON at line 10 is appropriate for an all-or-nothing update. The issue is transaction duration, not the use of an explicit transaction.

Indexes and storage impact

The usage summary covers only the 37.5 hours since startup. All 16 displayed indexes report zero seeks, scans, lookups, and updates during that interval. The largest reported indexes are the clustered PostHistory primary key at 820.3 MB, the clustered Posts primary key at 400.3 MB, and the clustered Comments primary key at 78.1 MB.

New-index estimate: no new index is recommended in this run. Therefore, estimated new storage is 0 MB.

Dropped-index estimate: no index is recommended for removal. Therefore, estimated reclaimed storage is 0 MB.

Modified-index estimate: no index is recommended for modification. Therefore, before size, after size, and net change are not applicable.

Net storage change for all recommendations: 0 MB. This is based on making no physical index changes because the input lacks a representative observation period, index definitions for all possible access paths, column-width statistics for missing candidates, and measured query demand.

Object-specific findings

The remaining supplied procedures were reviewed for the same classes of issues: non-sargable predicates, repeated full-table aggregates, one-to-many join multiplication, display-name joins, fixed dates, unnecessary ranking, and broad cross joins. They are not individually rewritten because the input provides no measured plans or execution metrics and changing their output semantics safely would require a procedure-by-procedure contract.

Scripts

Correct dbo.sp00060 aggregation and user-to-post relationship — Recommendation 1

USE [SQLStorm];
GO

CREATE OR ALTER PROCEDURE [dbo].[sp00060]
AS
BEGIN
    SET NOCOUNT ON;

    ;WITH RankedPosts AS
    (
        SELECT
            p.Id AS PostId,
            p.OwnerUserId,
            p.Title,
            p.CreationDate,
            ROW_NUMBER() OVER
            (
                PARTITION BY t.Id
                ORDER BY p.CreationDate DESC, p.Id DESC
            ) AS rn,
            t.TagName
        FROM dbo.Posts AS p
        INNER JOIN dbo.Tags AS t
            ON p.Tags LIKE '%' + t.TagName + '%'
        WHERE p.PostTypeId = 1
    ),
    PostCounts AS
    (
        SELECT
            p.OwnerUserId AS UserId,
            COUNT_BIG(*) AS TotalPosts
        FROM dbo.Posts AS p
        WHERE p.OwnerUserId IS NOT NULL
        GROUP BY p.OwnerUserId
    ),
    VoteCounts AS
    (
        SELECT
            p.OwnerUserId AS UserId,
            SUM(CASE WHEN v.VoteTypeId = 2 THEN CONVERT(bigint, 1) ELSE CONVERT(bigint, 0) END) AS UpVotes,
            SUM(CASE WHEN v.VoteTypeId = 3 THEN CONVERT(bigint, 1) ELSE CONVERT(bigint, 0) END) AS DownVotes
        FROM dbo.Posts AS p
        LEFT JOIN dbo.Votes AS v
            ON v.PostId = p.Id
           AND v.VoteTypeId IN (2, 3)
        WHERE p.OwnerUserId IS NOT NULL
        GROUP BY p.OwnerUserId
    ),
    BadgeCounts AS
    (
        SELECT
            b.UserId,
            COUNT_BIG(*) AS BadgeCount
        FROM dbo.Badges AS b
        GROUP BY b.UserId
    ),
    TopUsers AS
    (
        SELECT
            u.Id AS UserId,
            u.DisplayName,
            CONVERT(int, ISNULL(pc.TotalPosts, 0)) AS TotalPosts,
            CONVERT(int, ISNULL(vc.UpVotes, 0)) AS UpVotes,
            CONVERT(int, ISNULL(vc.DownVotes, 0)) AS DownVotes,
            CONVERT(int, ISNULL(bc.BadgeCount, 0)) AS BadgeCount,
            DENSE_RANK() OVER
            (
                ORDER BY
                    ISNULL(pc.TotalPosts, 0) DESC,
                    ISNULL(vc.UpVotes, 0) - ISNULL(vc.DownVotes, 0) DESC
            ) AS UserRank
        FROM dbo.Users AS u
        LEFT JOIN PostCounts AS pc
            ON pc.UserId = u.Id
        LEFT JOIN VoteCounts AS vc
            ON vc.UserId = u.Id
        LEFT JOIN BadgeCounts AS bc
            ON bc.UserId = u.Id
        WHERE ISNULL(pc.TotalPosts, 0) > 10
    )
    SELECT
        tu.UserRank,
        tu.DisplayName,
        tu.TotalPosts,
        tu.UpVotes,
        tu.DownVotes,
        tu.BadgeCount,
        rp.Title AS LatestPostTitle,
        rp.CreationDate AS LatestPostDate,
        CASE
            WHEN rp.rn = 1 THEN 'Latest'
            ELSE 'Older'
        END AS PostStatus
    FROM TopUsers AS tu
    LEFT JOIN RankedPosts AS rp
        ON rp.OwnerUserId = tu.UserId
    ORDER BY
        tu.UserRank,
        rp.CreationDate DESC,
        rp.PostId DESC;
END;
GO

Move the deliberate delay outside the transaction — Recommendation 2

USE [SQLStorm];
GO

CREATE OR ALTER PROCEDURE dbo.usp_UpdateTablesWithDelay
    @UserId INT,
    @NewReputation INT,
    @PostId INT,
    @NewScore INT
AS
BEGIN
    SET NOCOUNT ON;
    SET XACT_ABORT ON;

    BEGIN TRY
        WAITFOR DELAY '00:00:05';

        BEGIN TRANSACTION;

        UPDATE dbo.Users
        SET Reputation = @NewReputation,
            LastAccessDate = GETDATE()
        WHERE Id = @UserId;

        DECLARE @RowsAffected1 INT = @@ROWCOUNT;

        UPDATE dbo.Posts
        SET Score = @NewScore,
            LastActivityDate = GETDATE()
        WHERE Id = @PostId;

        DECLARE @RowsAffected2 INT = @@ROWCOUNT;

        COMMIT TRANSACTION;

        SELECT
            'Success' AS Status,
            'Both tables updated successfully' AS Message,
            @RowsAffected1 AS UsersRowsAffected,
            @RowsAffected2 AS PostsRowsAffected;
    END TRY
    BEGIN CATCH
        IF @@TRANCOUNT > 0
            ROLLBACK TRANSACTION;

        SELECT
            'Error' AS Status,
            ERROR_NUMBER() AS ErrorNumber,
            ERROR_MESSAGE() AS ErrorMessage,
            ERROR_LINE() AS ErrorLine,
            ERROR_PROCEDURE() AS ErrorProcedure;

        THROW;
    END CATCH
END;
GO

Validate loop count and make success-rate calculation safe — Recommendation 3

USE [SQLStorm];
GO

CREATE OR ALTER PROCEDURE dbo.usp_UpdateTablesInLoop
    @LoopCount INT = 10,
    @DelayBetweenCalls VARCHAR(8) = '00:00:01'
AS
BEGIN
    SET NOCOUNT ON;

    IF @LoopCount < 1
        THROW 50001, 'LoopCount must be greater than zero.', 1;

    DECLARE @Counter INT = 0;
    DECLARE @RandomUserId INT;
    DECLARE @RandomPostId INT;
    DECLARE @NewReputation INT;
    DECLARE @NewScore INT;
    DECLARE @SuccessCount INT = 0;
    DECLARE @ErrorCount INT = 0;

    PRINT 'Starting loop execution at ' + CONVERT(varchar(50), GETDATE());
    PRINT 'Will execute ' + CONVERT(varchar(10), @LoopCount) + ' iterations';
    PRINT REPLICATE('-', 80);

    WHILE @Counter < @LoopCount
    BEGIN
        SET @Counter += 1;

        BEGIN TRY
            SET @RandomUserId = NULL;
            SET @RandomPostId = NULL;

            SELECT TOP (1)
                @RandomUserId = Id
            FROM dbo.Users
            ORDER BY NEWID();

            SELECT TOP (1)
                @RandomPostId = Id
            FROM dbo.Posts
            ORDER BY NEWID();

            SET @NewReputation = ABS(CHECKSUM(NEWID())) % 10000;
            SET @NewScore = ABS(CHECKSUM(NEWID())) % 100 - 10;

            EXEC dbo.usp_UpdateTablesWithDelay
                @UserId = @RandomUserId,
                @NewReputation = @NewReputation,
                @PostId = @RandomPostId,
                @NewScore = @NewScore;

            SET @SuccessCount += 1;
        END TRY
        BEGIN CATCH
            SET @ErrorCount += 1;
            PRINT 'Iteration ' + CONVERT(varchar(10), @Counter) +
                  ' failed: ' + ERROR_MESSAGE();
        END CATCH;

        IF @Counter < @LoopCount
            WAITFOR DELAY @DelayBetweenCalls;
    END;

    SELECT
        @LoopCount AS TotalIterations,
        @SuccessCount AS Successful,
        @ErrorCount AS Errors,
        CONVERT(decimal(5,2), @SuccessCount * 100.0 / NULLIF(@LoopCount, 0)) AS SuccessRate_Percent;
END;
GO

Confidence and limitations

• High confidence: dbo.sp00060 contains an invalid user-ID-to-post-ID relationship at line 79 and multiplies independent one-to-many joins at lines 36–45.
• High confidence: dbo.usp_UpdateTablesWithDelay holds locks across the five-second wait at line 25.
• High confidence: leading-wildcard tag predicates are not ordinarily seekable through the supplied B-tree indexes.
• Medium confidence: the precise runtime cost of the reviewed procedures, because Query Store durations, plans, reads, CPU, memory grants, spills, and wait data are absent.
• Low-to-medium confidence: the intended business output of procedures containing cross joins or fixed dates; those procedures are not rewritten without a reliable contract.

No functions, triggers, views, NOT INCLUDED objects, or NOT AVAILABLE objects were supplied. Recommendations are limited to code that was shown. The report does not recommend changes to unseen code.