Fix various places in PerfStatsAnalysis.sql - minor issues

[PiJoCoder]

This PR applies a set of small correctness and robustness fixes to PerfStatsAnalysis.sql, primarily around wait stats calculations and a few stored-procedure logic issues.

Changes:

• Harden wait stats outputs (e.g., clamp negative wait deltas; exclude zero/negative waits from “top” category selection; use BIGINT for available CPU seconds).
• Fix query logic issues (e.g., correct boolean grouping in blocking-chain time filtering; fix cursor loop condition).
• Prevent runtime errors / logic inversions (e.g., NULLIF to avoid divide-by-zero; correct COL_LENGTH null check; remove stray debug SELECT).

Test Case Plan — PR: Fix Various Issues in PerfStatsAnalysis.sql

---

Prerequisites

• A SQL Server instance with SqlNexus data already imported (or a test database populated with the relevant tables/views).
• Run PerfStatsAnalysis.sql against the target database before executing any test case.

---

Test Case 1 — wait_time_ms_per_sec clamped to 0 on counter rollover

Fix: Negative wait_time_ms_per_sec values (e.g. after a SQL Server restart mid-collection) are now returned as 0.

Steps:

1. Update a rows into the underlying wait stats source such that w_end.wait_time_ms < w_start.wait_time_ms for at least one wait category.
2. Execute DataSet_WaitStats_WaitStatsTopCategoriesOther for that time range.

Expected result: wait_time_ms_per_sec = 0 for the affected category — not a negative value.

---

Test Case 2 — No divide-by-zero when @StartTime equals @EndTime

Fix: NULLIF(DATEDIFF(s, @StartTime, @EndTime) + 1, 0) prevents a divide-by-zero when the interval is zero seconds.

Steps:

1. Execute either stored proc below with identical start and end timestamps:

   EXEC DataSet_WaitStats_WaitStatsTop5Categories @StartTime = '2024-01-01 10:00:00', @EndTime   = '2024-01-01 10:00:00';
   EXEC DataSet_WaitStats_WaitStatsTopCategoriesOther @StartTime = '2024-01-01 10:00:00', @EndTime   = '2024-01-01 10:00:00';

Expected result: Both queries complete without error; wait_time_ms_per_sec returns NULL or 0.

---

Test Case 3 — No integer overflow for @avail_cpu_time_sec on high-CPU systems

Fix: @avail_cpu_time_sec widened from INT to BIGINT.

Steps:

1. Set a high CPU count in the server properties table:

   UPDATE dbo.tbl_ServerProperties SET PropertyValue = '128' WHERE PropertyName = 'cpu_count';

2. Execute either wait stats proc with a long collection window (e.g. 50,000+ seconds).

Expected result: CPU seconds are calculated correctly with no arithmetic overflow error.

---

Test Case 4 — Top-5 wait categories excludes zero-wait entries and SOS_SCHEDULER_YIELD

Fix: The subquery alias collision (cat → cat2) is resolved, zero-wait rows are filtered out, and SOS_SCHEDULER_YIELD is explicitly excluded from the Top 5 exclusion list.

Steps:

1. Ensure SOS_SCHEDULER_YIELD has the highest wait_time_ms_per_sec in the dataset.
2. Ensure at least one wait category has wait_time_ms_per_sec = 0 and total_wait_time_ms = 0.
3. Execute DataSet_WaitStats_WaitStatsTopCategoriesOther.

Expected result:

• SOS_SCHEDULER_YIELD is not in the Top 5 exclusion list and appears in the "Other" results.
• Zero-wait categories do not displace real wait types from the Top 5.

---

Test Case 5 — Blocking chains correctly filtered by time window boundary

Fix: Missing parentheses around the OR condition caused incorrect AND/OR precedence in the WHERE clause.

Steps:

1. Insert a row into vw_BLOCKING_CHAINS (or its source) where:

• blocking_start is before @StartTime, and
• blocking_end falls within @StartTime and @EndTime.

2. Run the blocking chains query for that window - DataSet_WaitStats_BlockingChains

Expected result: The blocking event is returned. Confirm that events entirely outside the window are not returned.

---

Test Case 6 — Cursor loop processes all blocking rows

Fix: WHILE (@@FETCH_STATUS <> -1) corrected to WHILE (@@FETCH_STATUS = 0) so the loop stops on any non-success status.

Steps:

1. Ensure vw_BLOCKING_CHAINS has 3 or more rows within the selected time range.
2. Execute the section of the script that builds the @txtout blocking summary string.

Expected result: All rows are represented in the output string — no rows are skipped or duplicated.

---

Test Case 7 — No divide-by-zero for zero-rowtables in stats properties

Fix: NULLIF([rows], 0) prevents divide-by-zero in usp_SmallSampledStats.

Steps:

1. Insert a test row:

   INSERT INTO dbo.tbl_dm_db_stats_properties (rows, rows_sampled, ...) VALUES (0, 0, ...);

2. Execute usp_SmallSampledStats.

Expected result: Proc completes without a divide-by-zero error. The zero-row entry is handled gracefully.

---

Test Case 8 — Query plan file lookup executes when xml_plan column exists

Fix: Condition was inverted (IS NULL → IS NOT NULL), causing the lookup to be skipped when the column was present.

Steps:

1. Confirm the column exists:

   SELECT COL_LENGTH('dbo.tblTopSqlPlan', 'xml_plan'); -- Must return a non-NULL value

2. Run the query plan lookup section of PerfStatsAnalysis.sql.

Expected result: The @FileName variable is populated and the query plan lookupblock executes. Previously it was silently skipped.

---

Test Case 9 — Script is idempotent (safe to re-run)

Fix: Added DROP PROC IF EXISTS guard before CREATE PROC DataSet_WaitStats_WaitStatsTopCategoriesOther.

Steps:

1. Run PerfStatsAnalysis.sql against the target database.
2. Run PerfStatsAnalysis.sql a second time without dropping any objects.

Expected result: The script completes successfully on both runs with no "object already exists" error.

---

Pass Criteria

All 9 test cases must produce their expected results with no SQL errors, no incorrect data silently returned, and no arithmetic overflow or divide-by-zero exceptions.

[Copilot]

Pull request overview

This PR applies a set of small correctness and robustness fixes to PerfStatsAnalysis.sql, primarily around wait stats calculations and a few stored-procedure logic issues.

Changes:

• Harden wait stats outputs (e.g., clamp negative wait deltas; exclude zero/negative waits from “top” category selection; use BIGINT for available CPU seconds).
• Fix query logic issues (e.g., correct boolean grouping in blocking-chain time filtering; fix cursor loop condition).
• Prevent runtime errors / logic inversions (e.g., NULLIF to avoid divide-by-zero; correct COL_LENGTH null check; remove stray debug SELECT).

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[PiJoCoder]

@copilot apply changes based on the comments in this thread

[Copilot]

@copilot apply changes based on the comments in this thread

Applied in commit 9e57bde — updated the usp_SmallSampledStats description text to use NULLIF([rows], 0) so it matches the safe predicate and users following the guidance won't hit a divide-by-zero.

[PiJoCoder]

@copilot: can you summarize and update the description in the PR of all the changes/commits in this PR

[Copilot]

@copilot: can you summarize and update the description in the PR of all the changes/commits in this PR

Updated the PR description with a full summary of all changes across the three commits. Here's the breakdown:

Wait stats hardening

• Clamp negative wait-time deltas to 0 before computing wait_time_ms_per_sec
• Use NULLIF(DATEDIFF(...)+1, 0) as divisor to avoid divide-by-zero on invalid intervals
• Change @avail_cpu_time_sec from INT → BIGINT (both wait stats procs)
• Exclude zero/negative wait entries and SOS_SCHEDULER_YIELD from the TOP 5 subquery
• Add missing DROP PROC guard before CREATE PROC DataSet_WaitStats_WaitStatsTopCategoriesOther

Query logic fixes

• Fix boolean precedence in blocking-chain time filter (AND (...OR...))
• Fix cursor WHILE condition from <> -1 to = 0

Runtime error / logic-inversion fixes

• usp_SmallSampledStats: use NULLIF([rows], 0) in both the predicate and the recommendation text
• usp_BatchSort: fix inverted COL_LENGTH null check (IS NULL → IS NOT NULL)
• usp_AutoUpdateStats: remove stray debug SELECT * that leaked a result set

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[hacitandogan]

PerfStatsAnalysis.sql hash needs to be updated;

{ Application.StartupPath + "\" + "PerfStatsAnalysis.sql", "37111E6F2052A2A7B15E26329475738B4E543FE66AFB7BA426639C062D9E81A1" }, vs { "PerfStatsAnalysis.sql", "841466E4E5F7AB980BB6492D3AEB636C4E55749107DCF391A585601B6263A5E5" },

I am not sending a modification so we don’t end up with the same approver / modifier loop as before, we can check this together in the call.

DataSet_WaitStats_WaitStatsTopCategories does not exists , is it DataSet_WaitStats_WaitStatsTopCategoriesOther or DataSet_WaitStats_WaitStatsTop5Categories

DataSet_WaitStats_WaitStatsTop5Categories has <= 0 check but DataSet_WaitStats_WaitStatsTopCategoriesOther does not, I believe we may need the same check there as well.

Other modifications looks good.