Skip to content

Rules 15-24: Pattern precision, threshold tuning, message improvements #356

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
erikdarlingdata merged 1 commit into from
Feb 27, 2026
Merged

Rules 15-24: Pattern precision, threshold tuning, message improvements #356

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
93 changes: 53 additions & 40 deletions Dashboard/Services/PlanAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -399,16 +399,16 @@
}
}

// Rule 15: Join OR clause (Concatenation + Constant Scan under join/Merge Interval)
// Best signal: Merge Interval → TopN Sort → Concatenation → Constant Scans
// Also fires under a join ancestor (broader catch)
// Rule 15: Join OR clause
// Pattern: Nested Loops → Merge Interval → TopN Sort → [Compute Scalar] → Concatenation → [Compute Scalar] → 2+ Constant Scans
if (node.PhysicalOp == "Concatenation")
{
var constantScanBranches = node.Children
.Count(c => c.PhysicalOp == "Constant Scan" ||
c.Children.Any(gc => gc.PhysicalOp == "Constant Scan"));
(c.PhysicalOp == "Compute Scalar" &&
c.Children.Any(gc => gc.PhysicalOp == "Constant Scan")));

if (constantScanBranches >= 2 && (HasMergeIntervalAncestor(node) || HasJoinAncestor(node)))
if (constantScanBranches >= 2 && IsOrExpansionChain(node))
{
node.Warnings.Add(new PlanWarning
{
Expand All @@ -426,50 +426,55 @@
{
var innerChild = node.Children[1];

if (innerChild.HasActualStats && innerChild.ActualExecutions > 1000)
if (innerChild.HasActualStats && innerChild.ActualExecutions > 100000)
{
var dop = stmt.DegreeOfParallelism > 0 ? stmt.DegreeOfParallelism : 1;
node.Warnings.Add(new PlanWarning
{
WarningType = "Nested Loops High Executions",
Message = $"Nested Loops inner side executed {innerChild.ActualExecutions:N0} times (DOP {dop}). A Hash Join or Merge Join may be more efficient for this row count.",
Severity = innerChild.ActualExecutions > 100000
Severity = innerChild.ActualExecutions > 1000000
? PlanWarningSeverity.Critical
: PlanWarningSeverity.Warning
});
}
else if (!innerChild.HasActualStats && innerChild.EstimateRebinds > 1000)
else if (!innerChild.HasActualStats && innerChild.EstimateRebinds > 100000)
{
node.Warnings.Add(new PlanWarning
{
WarningType = "Nested Loops High Executions",
Message = $"Nested Loops inner side estimated to execute {innerChild.EstimateRebinds + 1:N0} times. A Hash Join or Merge Join may be more efficient for this row count.",
Severity = innerChild.EstimateRebinds > 100000
Severity = innerChild.EstimateRebinds > 1000000
? PlanWarningSeverity.Critical
: PlanWarningSeverity.Warning
});
}
}

// Rule 17: Many-to-many Merge Join
if (node.ManyToMany && node.PhysicalOp.Contains("Merge", StringComparison.OrdinalIgnoreCase))
// In actual plans, the Merge Join operator reports logical reads when the worktable is used.
// When ActualLogicalReads is 0, the worktable wasn't hit and the warning is noise.
if (node.ManyToMany && node.PhysicalOp.Contains("Merge", StringComparison.OrdinalIgnoreCase) &&
(!node.HasActualStats || node.ActualLogicalReads > 0))
{
node.Warnings.Add(new PlanWarning
{
WarningType = "Many-to-Many Merge Join",
Message = "Many-to-many Merge Join requires a worktable to handle duplicate values. This can be expensive with large numbers of duplicates.",
Message = node.HasActualStats
? $"Many-to-many Merge Join used a worktable ({node.ActualLogicalReads:N0} logical reads). This can be expensive with large numbers of duplicates."
: "Many-to-many Merge Join requires a worktable to handle duplicate values. This can be expensive with large numbers of duplicates.",
Severity = PlanWarningSeverity.Warning
});
}

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

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

View workflow job for this annotation

GitHub Actions / build 

The behavior of 'string.StartsWith(string)' could vary based on the current user's locale settings. Replace this call in 'PerformanceMonitorDashboard.Services.PlanAnalyzer.AnalyzeNode(PerformanceMonitorDashboard.Models.PlanNode, PerformanceMonitorDashboard.Models.PlanStatement)' with a call to 'string.StartsWith(string, System.StringComparison)'. (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1310)

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

View workflow job for this annotation

GitHub Actions / build 

Use 'string.StartsWith(char)' instead of 'string.StartsWith(string)' when you have a string with a single char (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1866)

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

View workflow job for this annotation

GitHub Actions / build 

The behavior of 'string.StartsWith(string)' could vary based on the current user's locale settings. Replace this call in 'PerformanceMonitorDashboard.Services.PlanAnalyzer.AnalyzeNode(PerformanceMonitorDashboard.Models.PlanNode, PerformanceMonitorDashboard.Models.PlanStatement)' with a call to 'string.StartsWith(string, System.StringComparison)'. (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1310)

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

View workflow job for this annotation

GitHub Actions / build 

Use 'string.StartsWith(char)' instead of 'string.StartsWith(string)' when you have a string with a single char (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1866)

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

View workflow job for this annotation

GitHub Actions / build 

The behavior of 'string.StartsWith(string)' could vary based on the current user's locale settings. Replace this call in 'PerformanceMonitorDashboard.Services.PlanAnalyzer.AnalyzeNode(PerformanceMonitorDashboard.Models.PlanNode, PerformanceMonitorDashboard.Models.PlanStatement)' with a call to 'string.StartsWith(string, System.StringComparison)'. (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1310)

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

View workflow job for this annotation

GitHub Actions / build 

Use 'string.StartsWith(char)' instead of 'string.StartsWith(string)' when you have a string with a single char (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1866)

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

View workflow job for this annotation

GitHub Actions / build 

The behavior of 'string.StartsWith(string)' could vary based on the current user's locale settings. Replace this call in 'PerformanceMonitorDashboard.Services.PlanAnalyzer.AnalyzeNode(PerformanceMonitorDashboard.Models.PlanNode, PerformanceMonitorDashboard.Models.PlanStatement)' with a call to 'string.StartsWith(string, System.StringComparison)'. (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1310)

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

View workflow job for this annotation

GitHub Actions / build 

Use 'string.StartsWith(char)' instead of 'string.StartsWith(string)' when you have a string with a single char (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1866)

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

View workflow job for this annotation

GitHub Actions / build 

The behavior of 'string.StartsWith(string)' could vary based on the current user's locale settings. Replace this call in 'PerformanceMonitorDashboard.Services.PlanAnalyzer.AnalyzeNode(PerformanceMonitorDashboard.Models.PlanNode, PerformanceMonitorDashboard.Models.PlanStatement)' with a call to 'string.StartsWith(string, System.StringComparison)'. (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1310)

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

View workflow job for this annotation

GitHub Actions / build 

Use 'string.StartsWith(char)' instead of 'string.StartsWith(string)' when you have a string with a single char (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1866)

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

View workflow job for this annotation

GitHub Actions / build 

The behavior of 'string.StartsWith(string)' could vary based on the current user's locale settings. Replace this call in 'PerformanceMonitorDashboard.Services.PlanAnalyzer.AnalyzeNode(PerformanceMonitorDashboard.Models.PlanNode, PerformanceMonitorDashboard.Models.PlanStatement)' with a call to 'string.StartsWith(string, System.StringComparison)'. (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1310)

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

View workflow job for this annotation

GitHub Actions / build 

Use 'string.StartsWith(char)' instead of 'string.StartsWith(string)' when you have a string with a single char (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1866)
{
node.Warnings.Add(new PlanWarning
{
WarningType = "Table Variable",
Message = "Table variable detected. Table variables have no statistics, so the optimizer always estimates 1 row regardless of actual cardinality. Consider using a temp table (#table) for better estimates.",
Message = "Table variable detected. Table variables lack column statistics and may lead to poor join choices. Consider using a temp table (#table) for better estimates. On SQL Server 2019+, table variable deferred compilation can improve cardinality estimates.",
Severity = PlanWarningSeverity.Warning
});
}
Expand Down Expand Up @@ -505,7 +510,7 @@
while (scanCandidate.PhysicalOp == "Compute Scalar" && scanCandidate.Children.Count > 0)
scanCandidate = scanCandidate.Children[0];

if (IsRowstoreScan(scanCandidate))
if (IsScanOperator(scanCandidate))
{
var predInfo = !string.IsNullOrEmpty(scanCandidate.Predicate)
? " The scan has a residual predicate, so it may read many rows before the Top is satisfied."
Expand Down Expand Up @@ -533,6 +538,17 @@
!node.PhysicalOp.Contains("Columnstore", StringComparison.OrdinalIgnoreCase);
}

/// <summary>
/// Returns true for any scan operator including columnstore.
/// Excludes spools and constant scans.
/// </summary>
private static bool IsScanOperator(PlanNode node)
{
return node.PhysicalOp.Contains("Scan", StringComparison.OrdinalIgnoreCase) &&
!node.PhysicalOp.Contains("Spool", StringComparison.OrdinalIgnoreCase) &&
!node.PhysicalOp.Contains("Constant", StringComparison.OrdinalIgnoreCase);
}

/// <summary>
/// Detects non-SARGable patterns in scan predicates.
/// Returns a description of the issue, or null if the predicate is fine.
Expand Down Expand Up @@ -613,34 +629,31 @@
}

/// <summary>
/// Checks whether a node has a Merge Interval ancestor (OR expansion pattern).
/// </summary>
private static bool HasMergeIntervalAncestor(PlanNode node)
{
var ancestor = node.Parent;
while (ancestor != null)
{
if (ancestor.PhysicalOp == "Merge Interval")
return true;
ancestor = ancestor.Parent;
}
return false;
}

/// <summary>
/// Checks whether a node has any join ancestor.
/// Verifies the OR expansion chain walking up from a Concatenation node:
/// Nested Loops → Merge Interval → TopN Sort → [Compute Scalar] → Concatenation
/// </summary>
private static bool HasJoinAncestor(PlanNode node)
private static bool IsOrExpansionChain(PlanNode concatenationNode)
{
var ancestor = node.Parent;
while (ancestor != null)
{
if (ancestor.LogicalOp != null &&
ancestor.LogicalOp.Contains("Join", StringComparison.OrdinalIgnoreCase))
return true;
ancestor = ancestor.Parent;
}
return false;
// Walk up, skipping Compute Scalar
var parent = concatenationNode.Parent;
while (parent != null && parent.PhysicalOp == "Compute Scalar")
parent = parent.Parent;

// Expect TopN Sort
if (parent == null || parent.LogicalOp != "TopN Sort")
return false;

// Walk up to Merge Interval
parent = parent.Parent;
if (parent == null || parent.PhysicalOp != "Merge Interval")
return false;

// Walk up to Nested Loops
parent = parent.Parent;
if (parent == null || parent.PhysicalOp != "Nested Loops")
return false;

return true;
}

/// <summary>
Expand Down
93 changes: 53 additions & 40 deletions Lite/Services/PlanAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -399,16 +399,16 @@
}
}

// Rule 15: Join OR clause (Concatenation + Constant Scan under join/Merge Interval)
// Best signal: Merge Interval → TopN Sort → Concatenation → Constant Scans
// Also fires under a join ancestor (broader catch)
// Rule 15: Join OR clause
// Pattern: Nested Loops → Merge Interval → TopN Sort → [Compute Scalar] → Concatenation → [Compute Scalar] → 2+ Constant Scans
if (node.PhysicalOp == "Concatenation")
{
var constantScanBranches = node.Children
.Count(c => c.PhysicalOp == "Constant Scan" ||
c.Children.Any(gc => gc.PhysicalOp == "Constant Scan"));
(c.PhysicalOp == "Compute Scalar" &&
c.Children.Any(gc => gc.PhysicalOp == "Constant Scan")));

if (constantScanBranches >= 2 && (HasMergeIntervalAncestor(node) || HasJoinAncestor(node)))
if (constantScanBranches >= 2 && IsOrExpansionChain(node))
{
node.Warnings.Add(new PlanWarning
{
Expand All @@ -426,50 +426,55 @@
{
var innerChild = node.Children[1];

if (innerChild.HasActualStats && innerChild.ActualExecutions > 1000)
if (innerChild.HasActualStats && innerChild.ActualExecutions > 100000)
{
var dop = stmt.DegreeOfParallelism > 0 ? stmt.DegreeOfParallelism : 1;
node.Warnings.Add(new PlanWarning
{
WarningType = "Nested Loops High Executions",
Message = $"Nested Loops inner side executed {innerChild.ActualExecutions:N0} times (DOP {dop}). A Hash Join or Merge Join may be more efficient for this row count.",
Severity = innerChild.ActualExecutions > 100000
Severity = innerChild.ActualExecutions > 1000000
? PlanWarningSeverity.Critical
: PlanWarningSeverity.Warning
});
}
else if (!innerChild.HasActualStats && innerChild.EstimateRebinds > 1000)
else if (!innerChild.HasActualStats && innerChild.EstimateRebinds > 100000)
{
node.Warnings.Add(new PlanWarning
{
WarningType = "Nested Loops High Executions",
Message = $"Nested Loops inner side estimated to execute {innerChild.EstimateRebinds + 1:N0} times. A Hash Join or Merge Join may be more efficient for this row count.",
Severity = innerChild.EstimateRebinds > 100000
Severity = innerChild.EstimateRebinds > 1000000
? PlanWarningSeverity.Critical
: PlanWarningSeverity.Warning
});
}
}

// Rule 17: Many-to-many Merge Join
if (node.ManyToMany && node.PhysicalOp.Contains("Merge", StringComparison.OrdinalIgnoreCase))
// In actual plans, the Merge Join operator reports logical reads when the worktable is used.
// When ActualLogicalReads is 0, the worktable wasn't hit and the warning is noise.
if (node.ManyToMany && node.PhysicalOp.Contains("Merge", StringComparison.OrdinalIgnoreCase) &&
(!node.HasActualStats || node.ActualLogicalReads > 0))
{
node.Warnings.Add(new PlanWarning
{
WarningType = "Many-to-Many Merge Join",
Message = "Many-to-many Merge Join requires a worktable to handle duplicate values. This can be expensive with large numbers of duplicates.",
Message = node.HasActualStats
? $"Many-to-many Merge Join used a worktable ({node.ActualLogicalReads:N0} logical reads). This can be expensive with large numbers of duplicates."
: "Many-to-many Merge Join requires a worktable to handle duplicate values. This can be expensive with large numbers of duplicates.",
Severity = PlanWarningSeverity.Warning
});
}

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

Check warning on line 472 in Lite/Services/PlanAnalyzer.cs

View workflow job for this annotation

GitHub Actions / build 

The behavior of 'string.StartsWith(string)' could vary based on the current user's locale settings. Replace this call in 'PerformanceMonitorLite.Services.PlanAnalyzer.AnalyzeNode(PerformanceMonitorLite.Models.PlanNode, PerformanceMonitorLite.Models.PlanStatement)' with a call to 'string.StartsWith(string, System.StringComparison)'. (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1310)

Check warning on line 472 in Lite/Services/PlanAnalyzer.cs

View workflow job for this annotation

GitHub Actions / build 

Use 'string.StartsWith(char)' instead of 'string.StartsWith(string)' when you have a string with a single char (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1866)

Check warning on line 472 in Lite/Services/PlanAnalyzer.cs

View workflow job for this annotation

GitHub Actions / build 

The behavior of 'string.StartsWith(string)' could vary based on the current user's locale settings. Replace this call in 'PerformanceMonitorLite.Services.PlanAnalyzer.AnalyzeNode(PerformanceMonitorLite.Models.PlanNode, PerformanceMonitorLite.Models.PlanStatement)' with a call to 'string.StartsWith(string, System.StringComparison)'. (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1310)

Check warning on line 472 in Lite/Services/PlanAnalyzer.cs

View workflow job for this annotation

GitHub Actions / build 

Use 'string.StartsWith(char)' instead of 'string.StartsWith(string)' when you have a string with a single char (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1866)

Check warning on line 472 in Lite/Services/PlanAnalyzer.cs

View workflow job for this annotation

GitHub Actions / build 

The behavior of 'string.StartsWith(string)' could vary based on the current user's locale settings. Replace this call in 'PerformanceMonitorLite.Services.PlanAnalyzer.AnalyzeNode(PerformanceMonitorLite.Models.PlanNode, PerformanceMonitorLite.Models.PlanStatement)' with a call to 'string.StartsWith(string, System.StringComparison)'. (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1310)

Check warning on line 472 in Lite/Services/PlanAnalyzer.cs

View workflow job for this annotation

GitHub Actions / build 

Use 'string.StartsWith(char)' instead of 'string.StartsWith(string)' when you have a string with a single char (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1866)
{
node.Warnings.Add(new PlanWarning
{
WarningType = "Table Variable",
Message = "Table variable detected. Table variables have no statistics, so the optimizer always estimates 1 row regardless of actual cardinality. Consider using a temp table (#table) for better estimates.",
Message = "Table variable detected. Table variables lack column statistics and may lead to poor join choices. Consider using a temp table (#table) for better estimates. On SQL Server 2019+, table variable deferred compilation can improve cardinality estimates.",
Severity = PlanWarningSeverity.Warning
});
}
Expand Down Expand Up @@ -505,7 +510,7 @@
while (scanCandidate.PhysicalOp == "Compute Scalar" && scanCandidate.Children.Count > 0)
scanCandidate = scanCandidate.Children[0];

if (IsRowstoreScan(scanCandidate))
if (IsScanOperator(scanCandidate))
{
var predInfo = !string.IsNullOrEmpty(scanCandidate.Predicate)
? " The scan has a residual predicate, so it may read many rows before the Top is satisfied."
Expand Down Expand Up @@ -533,6 +538,17 @@
!node.PhysicalOp.Contains("Columnstore", StringComparison.OrdinalIgnoreCase);
}

/// <summary>
/// Returns true for any scan operator including columnstore.
/// Excludes spools and constant scans.
/// </summary>
private static bool IsScanOperator(PlanNode node)
{
return node.PhysicalOp.Contains("Scan", StringComparison.OrdinalIgnoreCase) &&
!node.PhysicalOp.Contains("Spool", StringComparison.OrdinalIgnoreCase) &&
!node.PhysicalOp.Contains("Constant", StringComparison.OrdinalIgnoreCase);
}

/// <summary>
/// Detects non-SARGable patterns in scan predicates.
/// Returns a description of the issue, or null if the predicate is fine.
Expand Down Expand Up @@ -613,34 +629,31 @@
}

/// <summary>
/// Checks whether a node has a Merge Interval ancestor (OR expansion pattern).
/// </summary>
private static bool HasMergeIntervalAncestor(PlanNode node)
{
var ancestor = node.Parent;
while (ancestor != null)
{
if (ancestor.PhysicalOp == "Merge Interval")
return true;
ancestor = ancestor.Parent;
}
return false;
}

/// <summary>
/// Checks whether a node has any join ancestor.
/// Verifies the OR expansion chain walking up from a Concatenation node:
/// Nested Loops → Merge Interval → TopN Sort → [Compute Scalar] → Concatenation
/// </summary>
private static bool HasJoinAncestor(PlanNode node)
private static bool IsOrExpansionChain(PlanNode concatenationNode)
{
var ancestor = node.Parent;
while (ancestor != null)
{
if (ancestor.LogicalOp != null &&
ancestor.LogicalOp.Contains("Join", StringComparison.OrdinalIgnoreCase))
return true;
ancestor = ancestor.Parent;
}
return false;
// Walk up, skipping Compute Scalar
var parent = concatenationNode.Parent;
while (parent != null && parent.PhysicalOp == "Compute Scalar")
parent = parent.Parent;

// Expect TopN Sort
if (parent == null || parent.LogicalOp != "TopN Sort")
return false;

// Walk up to Merge Interval
parent = parent.Parent;
if (parent == null || parent.PhysicalOp != "Merge Interval")
return false;

// Walk up to Nested Loops
parent = parent.Parent;
if (parent == null || parent.PhysicalOp != "Nested Loops")
return false;

return true;
}

/// <summary>
Expand Down
Loading