Skip to content
Merged
Show file tree
Hide file tree
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
70 changes: 48 additions & 22 deletions Dashboard/Services/PlanAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -236,18 +236,30 @@
// Rule 5: Large estimate vs actual row gaps (actual plans only)
if (node.HasActualStats && node.EstimateRows > 0)
{
var ratio = node.ActualRows / node.EstimateRows;
if (ratio >= 10.0 || ratio <= 0.1)
if (node.ActualRows == 0)
{
var direction = ratio >= 10.0 ? "underestimated" : "overestimated";
var factor = ratio >= 10.0 ? ratio : 1.0 / ratio;
node.Warnings.Add(new PlanWarning
{
WarningType = "Row Estimate Mismatch",
Message = $"Estimated {node.EstimateRows:N0} rows, actual {node.ActualRows:N0} ({factor:F0}x {direction}). May cause poor plan choices.",
Severity = factor >= 100 ? PlanWarningSeverity.Critical : PlanWarningSeverity.Warning
Message = $"Estimated {node.EstimateRows:N0} rows, actual 0 rows returned. May cause poor plan choices.",
Severity = node.EstimateRows >= 100 ? PlanWarningSeverity.Critical : PlanWarningSeverity.Warning
});
}
else
{
var ratio = node.ActualRows / node.EstimateRows;
if (ratio >= 10.0 || ratio <= 0.1)
{
var direction = ratio >= 10.0 ? "underestimated" : "overestimated";
var factor = ratio >= 10.0 ? ratio : 1.0 / ratio;
node.Warnings.Add(new PlanWarning
{
WarningType = "Row Estimate Mismatch",
Message = $"Estimated {node.EstimateRows:N0} rows, actual {node.ActualRows:N0} ({factor:F0}x {direction}). May cause poor plan choices.",
Severity = factor >= 100 ? PlanWarningSeverity.Critical : PlanWarningSeverity.Warning
});
}
}
}

// Rule 6: Scalar UDF references (works on estimated plans too)
Expand All @@ -270,10 +282,12 @@
}

// Rule 8: Parallel thread skew (actual plans with per-thread stats)
// Only warn when there are enough rows to meaningfully distribute across threads
if (node.PerThreadStats.Count > 1)
{
var totalRows = node.PerThreadStats.Sum(t => t.ActualRows);
if (totalRows > 0)
var minRowsForSkew = node.PerThreadStats.Count * 1000;
if (totalRows >= minRowsForSkew)
{
var maxThread = node.PerThreadStats.OrderByDescending(t => t.ActualRows).First();
var skewRatio = (double)maxThread.ActualRows / totalRows;
Expand Down Expand Up @@ -426,7 +440,7 @@

// Rule 22: Table variables (Object name starts with @)
if (!string.IsNullOrEmpty(node.ObjectName) &&
node.ObjectName.Contains("@"))

Check warning on line 443 in Dashboard/Services/PlanAnalyzer.cs

View workflow job for this annotation

GitHub Actions / build

Use 'string.Contains(char)' instead of 'string.Contains(string)' when searching for a single character (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1847)

Check warning on line 443 in Dashboard/Services/PlanAnalyzer.cs

View workflow job for this annotation

GitHub Actions / build

Use 'string.Contains(char)' instead of 'string.Contains(string)' when searching for a single character (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1847)

Check warning on line 443 in Dashboard/Services/PlanAnalyzer.cs

View workflow job for this annotation

GitHub Actions / build

Use 'string.Contains(char)' instead of 'string.Contains(string)' when searching for a single character (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1847)

Check warning on line 443 in Dashboard/Services/PlanAnalyzer.cs

View workflow job for this annotation

GitHub Actions / build

Use 'string.Contains(char)' instead of 'string.Contains(string)' when searching for a single character (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1847)

Check warning on line 443 in Dashboard/Services/PlanAnalyzer.cs

View workflow job for this annotation

GitHub Actions / build

Use 'string.Contains(char)' instead of 'string.Contains(string)' when searching for a single character (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1847)

Check warning on line 443 in Dashboard/Services/PlanAnalyzer.cs

View workflow job for this annotation

GitHub Actions / build

Use 'string.Contains(char)' instead of 'string.Contains(string)' when searching for a single character (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1847)
{
node.Warnings.Add(new PlanWarning
{
Expand All @@ -448,25 +462,37 @@
});
}

// Rule 24: Top above a scan (linear search pattern)
if (node.PhysicalOp == "Top" && node.Children.Count > 0)
// Rule 24: Top above a scan on the inner side of Nested Loops
// This pattern means the scan executes once per outer row, and the Top
// limits each iteration — but with no supporting index the scan is a
// linear search repeated potentially millions of times.
if (node.PhysicalOp == "Nested Loops" && node.Children.Count >= 2)
{
// Walk through pass-through operators (Compute Scalar, etc.)
var child = node.Children[0];
while (child.PhysicalOp == "Compute Scalar" && child.Children.Count > 0)
child = child.Children[0];
var inner = node.Children[1];

if (IsRowstoreScan(child))
// Walk through pass-through operators to find Top
while (inner.PhysicalOp == "Compute Scalar" && inner.Children.Count > 0)
inner = inner.Children[0];

if (inner.PhysicalOp == "Top" && inner.Children.Count > 0)
{
var predInfo = !string.IsNullOrEmpty(child.Predicate)
? " The scan has a residual predicate, so it may read many rows before the Top is satisfied."
: "";
node.Warnings.Add(new PlanWarning
// Walk through pass-through operators below the Top to find the scan
var scanCandidate = inner.Children[0];
while (scanCandidate.PhysicalOp == "Compute Scalar" && scanCandidate.Children.Count > 0)
scanCandidate = scanCandidate.Children[0];

if (IsRowstoreScan(scanCandidate))
{
WarningType = "Top Above Scan",
Message = $"Top operator reads from {child.PhysicalOp} (Node {child.NodeId}).{predInfo} An index supporting the filter and ordering may convert this to a seek.",
Severity = PlanWarningSeverity.Warning
});
var predInfo = !string.IsNullOrEmpty(scanCandidate.Predicate)
? " The scan has a residual predicate, so it may read many rows before the Top is satisfied."
: "";
inner.Warnings.Add(new PlanWarning
{
WarningType = "Top Above Scan",
Message = $"Top operator reads from {scanCandidate.PhysicalOp} (Node {scanCandidate.NodeId}) on the inner side of Nested Loops (Node {node.NodeId}).{predInfo} An index supporting the filter and ordering may convert this to a seek.",
Severity = PlanWarningSeverity.Warning
});
}
}
}
}
Expand Down Expand Up @@ -548,7 +574,7 @@
var refPattern = new Regex(
$@"\b(FROM|JOIN)\s+{Regex.Escape(cteName)}\b",
RegexOptions.IgnoreCase);
var refCount = refPattern.Matches(text).Count;

Check warning on line 577 in Dashboard/Services/PlanAnalyzer.cs

View workflow job for this annotation

GitHub Actions / build

Use 'Regex.Count' instead of 'Regex.Matches(...).Count' (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1875)

Check warning on line 577 in Dashboard/Services/PlanAnalyzer.cs

View workflow job for this annotation

GitHub Actions / build

Use 'Regex.Count' instead of 'Regex.Matches(...).Count' (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1875)

Check warning on line 577 in Dashboard/Services/PlanAnalyzer.cs

View workflow job for this annotation

GitHub Actions / build

Use 'Regex.Count' instead of 'Regex.Matches(...).Count' (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1875)

Check warning on line 577 in Dashboard/Services/PlanAnalyzer.cs

View workflow job for this annotation

GitHub Actions / build

Use 'Regex.Count' instead of 'Regex.Matches(...).Count' (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1875)

Check warning on line 577 in Dashboard/Services/PlanAnalyzer.cs

View workflow job for this annotation

GitHub Actions / build

Use 'Regex.Count' instead of 'Regex.Matches(...).Count' (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1875)

Check warning on line 577 in Dashboard/Services/PlanAnalyzer.cs

View workflow job for this annotation

GitHub Actions / build

Use 'Regex.Count' instead of 'Regex.Matches(...).Count' (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1875)

if (refCount > 1)
{
Expand Down
Loading
Loading