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
65 changes: 62 additions & 3 deletions src/PlanViewer.Core/Services/BenefitScorer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@ public static class BenefitScorer
"Filter Operator", // Rule 1
"Eager Index Spool", // Rule 2
"Spill", // Rule 7
"Key Lookup", // Rule 10
"RID Lookup", // Rule 10 variant
// Key Lookup / RID Lookup (Rule 10) handled separately by ScoreKeyLookupWarning
"Scan With Predicate", // Rule 11
"Non-SARGable Predicate", // Rule 12
"Scan Cardinality Misestimate", // Rule 32
Expand Down Expand Up @@ -150,6 +149,10 @@ private static void ScoreNodeWarnings(PlanNode node, PlanStatement stmt)
{
ScoreSpillWarning(warning, node, stmt);
}
else if (warning.WarningType is "Key Lookup" or "RID Lookup") // Rule 10
{
ScoreKeyLookupWarning(warning, node, stmt);
}
else if (OperatorTimeRules.Contains(warning.WarningType))
{
ScoreByOperatorTime(warning, node, stmt);
Expand All @@ -159,7 +162,8 @@ private static void ScoreNodeWarnings(PlanNode node, PlanStatement stmt)
ScoreEstimateMismatchWarning(warning, node, stmt);
}
// Rules that stay null: Scalar UDF (Rule 6, informational reference),
// Parallel Skew (Rule 8), Data Type Mismatch (Rule 13),
// Parallel Skew (Rule 8 — will be integrated per-operator later),
// Data Type Mismatch (Rule 13),
// Lazy Spool Ineffective (Rule 14), Join OR Clause (Rule 15),
// Many-to-Many Merge Join (Rule 17), CTE Multiple References (Rule 21),
// Table Variable (Rule 22), Table-Valued Function (Rule 23),
Expand Down Expand Up @@ -282,6 +286,61 @@ private static void ScoreByOperatorTime(PlanWarning warning, PlanNode node, Plan
}
}

/// <summary>
/// Rule 10: Key Lookup / RID Lookup — benefit includes the lookup operator's time,
/// plus the parent Nested Loops join when the NL only exists to drive the lookup
/// (inner child is the lookup, outer child is a seek/scan with no subtree).
/// </summary>
private static void ScoreKeyLookupWarning(PlanWarning warning, PlanNode node, PlanStatement stmt)
{
var stmtMs = stmt.QueryTimeStats?.ElapsedTimeMs ?? 0;

if (node.HasActualStats && stmtMs > 0)
{
var operatorMs = PlanAnalyzer.GetOperatorOwnElapsedMs(node);

// Check if the parent NL join is purely a lookup driver:
// - Parent is Nested Loops
// - Has exactly 2 children
// - This node (the lookup) is the inner child (index 1)
// - The outer child (index 0) is a simple seek/scan with no children
var parent = node.Parent;
if (parent != null
&& parent.PhysicalOp == "Nested Loops"
&& parent.Children.Count == 2
&& parent.Children[1] == node
&& parent.Children[0].Children.Count == 0)
{
operatorMs += PlanAnalyzer.GetOperatorOwnElapsedMs(parent);
}

if (operatorMs > 0)
{
var benefit = (double)operatorMs / stmtMs * 100;
warning.MaxBenefitPercent = Math.Round(Math.Min(100, benefit), 1);
}
else
{
warning.MaxBenefitPercent = 0;
}
}
else if (!node.HasActualStats && stmt.StatementSubTreeCost > 0)
{
var benefit = (double)node.CostPercent;
// Same parent-NL logic for estimated plans
var parent = node.Parent;
if (parent != null
&& parent.PhysicalOp == "Nested Loops"
&& parent.Children.Count == 2
&& parent.Children[1] == node
&& parent.Children[0].Children.Count == 0)
{
benefit += parent.CostPercent;
}
warning.MaxBenefitPercent = Math.Round(Math.Min(100, benefit), 1);
}
}

/// <summary>
/// Rule 5: Row Estimate Mismatch — benefit is the harmed operator's time.
/// If the mismatch caused a spill, benefit = spilling operator time.
Expand Down
Loading