Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 53 additions & 1 deletion src/PlanViewer.Core/Services/PlanAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,7 @@

if (unsnifffedParams.Count > 0)
{
var hasRecompile = stmt.StatementText.Contains("RECOMPILE", StringComparison.OrdinalIgnoreCase);

Check warning on line 406 in src/PlanViewer.Core/Services/PlanAnalyzer.cs

View workflow job for this annotation

GitHub Actions / build-and-test

Dereference of a possibly null reference.

Check warning on line 406 in src/PlanViewer.Core/Services/PlanAnalyzer.cs

View workflow job for this annotation

GitHub Actions / build-and-test

Dereference of a possibly null reference.
if (!hasRecompile)
{
var names = string.Join(", ", unsnifffedParams.Select(p => p.Name));
Expand Down Expand Up @@ -433,6 +433,49 @@
});
}

// Rule 36: Dynamic cursor (#215 E1). Dynamic cursors can prevent index usage
// because they must tolerate underlying data changes between fetches, forcing
// scans and extra work per fetch. Switching to FAST_FORWARD, STATIC, or KEYSET
// often delivers a dramatic improvement.
if (!cfg.IsRuleDisabled(36)
&& string.Equals(stmt.CursorActualType, "Dynamic", StringComparison.OrdinalIgnoreCase))
{
var cursorLabel = string.IsNullOrEmpty(stmt.CursorName) ? "Cursor" : $"Cursor \"{stmt.CursorName}\"";
stmt.PlanWarnings.Add(new PlanWarning
{
WarningType = "Dynamic Cursor",
Message = $"{cursorLabel} is a dynamic cursor. Dynamic cursors tolerate underlying data changes between fetches, which prevents many index uses and forces extra work per fetch. If you don't need that semantic, switching to FAST_FORWARD (or STATIC / KEYSET, depending on requirements) typically gives a large performance improvement.",
Severity = PlanWarningSeverity.Warning
});
}

// Rule 37: CURSOR declaration without LOCAL (#215 E3). Default cursor scope
// is GLOBAL in SQL Server, which puts cursors in a shared namespace and can
// bloat the plan cache (Erik's writeup:
// https://erikdarling.com/cursor-declarations-that-use-openjson-can-bloat-your-plan-cache/).
if (!cfg.IsRuleDisabled(37) && !string.IsNullOrEmpty(stmt.StatementText))
{
// DECLARE <name> [qualifier(s)] CURSOR ... FOR
// Flags the declaration if LOCAL isn't among the qualifiers before CURSOR.
var cursorDeclMatch = Regex.Match(
stmt.StatementText,
@"\bDECLARE\s+\w+\s+((?:\w+\s+)*)CURSOR\b",
RegexOptions.IgnoreCase | RegexOptions.Singleline);
if (cursorDeclMatch.Success)
{
var qualifiers = cursorDeclMatch.Groups[1].Value;
if (!Regex.IsMatch(qualifiers, @"\bLOCAL\b", RegexOptions.IgnoreCase))
{
stmt.PlanWarnings.Add(new PlanWarning
{
WarningType = "Cursor Missing LOCAL",
Message = "CURSOR declaration is missing the LOCAL keyword. Default cursor scope is GLOBAL, which puts the cursor in a shared namespace and can bloat the plan cache (see https://erikdarling.com/cursor-declarations-that-use-openjson-can-bloat-your-plan-cache/). Adding LOCAL is cheap and usually right.",
Severity = PlanWarningSeverity.Warning
});
}
}
}

// Rules 25 (Ineffective Parallelism) and 31 (Parallel Wait Bottleneck) were removed.
// The CPU:Elapsed ratio is now shown in the runtime summary, and wait stats speak
// for themselves — no need for meta-warnings guessing at causes.
Expand Down Expand Up @@ -883,7 +926,16 @@
var message = "Scan with residual predicate — SQL Server is reading every row and filtering after the fact.";
if (!string.IsNullOrEmpty(details.Summary))
message += $" {details.Summary}";
message += " Check that you have appropriate indexes.";

// #215 E2: if the statement is executing a dynamic cursor, that's usually
// the reason an index didn't get used. Call it out so the user looks there
// first rather than hunting for a missing index.
var isDynamicCursor = string.Equals(stmt.CursorActualType, "Dynamic",
StringComparison.OrdinalIgnoreCase);
if (isDynamicCursor)
message += " This query is running inside a dynamic cursor, which can prevent index usage; changing the cursor type (FAST_FORWARD / STATIC / KEYSET) often fixes scans like this without any indexing change.";
else
message += " Check that you have appropriate indexes.";

// I/O waits specifically confirm the scan is hitting disk — elevate
if (HasSignificantIoWaits(stmt.WaitStats) && details.CostPct >= 50
Expand Down Expand Up @@ -1269,7 +1321,7 @@
// Rule 28: Row Count Spool — NOT IN with nullable column
// Pattern: Row Count Spool with high rewinds, child scan has IS NULL predicate,
// and statement text contains NOT IN
if (!cfg.IsRuleDisabled(28) && node.PhysicalOp.Contains("Row Count Spool"))

Check warning on line 1324 in src/PlanViewer.Core/Services/PlanAnalyzer.cs

View workflow job for this annotation

GitHub Actions / build-and-test

Dereference of a possibly null reference.

Check warning on line 1324 in src/PlanViewer.Core/Services/PlanAnalyzer.cs

View workflow job for this annotation

GitHub Actions / build-and-test

Dereference of a possibly null reference.
{
var rewinds = node.HasActualStats ? (double)node.ActualRewinds : node.EstimateRewinds;
if (rewinds > 10000 && HasNotInPattern(node, stmt))
Expand Down
Loading