Plan analyzer improvements

src/PlanViewer.App/Controls/PlanViewerControl.axaml.cs

@@ -331,6 +331,8 @@ private Border CreateNodeVisual(PlanNode node, int totalWarningCount = -1)
         if (totalWarningCount > 0)
         {
             var allWarnings = new List<PlanWarning>();
+            if (_currentStatement != null)
+                allWarnings.AddRange(_currentStatement.PlanWarnings);
             CollectWarnings(node, allWarnings);
             ToolTip.SetTip(border, BuildNodeTooltipContent(node, allWarnings));
         }
@@ -2165,9 +2167,22 @@ private void ShowParameters(PlanStatement statement)
         // Annotations
         if (allCompiledNull && parameters.Count > 0)
         {
-            AddParameterAnnotation(
-                "OPTION(RECOMPILE) — parameter values embedded as literals, not sniffed",
-                "#FFB347");
+            var hasOptimizeForUnknown = statement.StatementText
+                .Contains("OPTIMIZE", StringComparison.OrdinalIgnoreCase)
+                && Regex.IsMatch(statement.StatementText, @"OPTIMIZE\s+FOR\s+UNKNOWN", RegexOptions.IgnoreCase);
+
+            if (hasOptimizeForUnknown)
+            {
+                AddParameterAnnotation(
+                    "OPTIMIZE FOR UNKNOWN — optimizer used average density estimates instead of sniffed values",
+                    "#6BB5FF");
+            }
+            else
+            {
+                AddParameterAnnotation(
+                    "OPTION(RECOMPILE) — parameter values embedded as literals, not sniffed",
+                    "#FFB347");
+            }
         }
 
         var unresolved = FindUnresolvedVariables(statement.StatementText, parameters);

src/PlanViewer.Core/Services/PlanAnalyzer.cs

@@ -388,6 +388,55 @@
                 }
             }
         }
+
+        // Rule 22 (statement-level): Table variable warnings
+        // Walk the tree to find table variable references, then emit statement-level warnings
+        if (!cfg.IsRuleDisabled(22) && stmt.RootNode != null)
+        {
+            var hasTableVar = false;
+            var isModification = stmt.StatementType is "INSERT" or "UPDATE" or "DELETE" or "MERGE";
+            var modifiesTableVar = false;
+            CheckForTableVariables(stmt.RootNode, isModification, ref hasTableVar, ref modifiesTableVar);
+
+            if (hasTableVar)
+            {
+                stmt.PlanWarnings.Add(new PlanWarning
+                {
+                    WarningType = "Table Variable",
+                    Message = "Table variable detected. Table variables lack column-level statistics, which causes bad row estimates, join choices, and memory grant decisions. Replace with a #temp table.",
+                    Severity = PlanWarningSeverity.Warning
+                });
+            }
+
+            if (modifiesTableVar)
+            {
+                stmt.PlanWarnings.Add(new PlanWarning
+                {
+                    WarningType = "Table Variable",
+                    Message = "This query modifies a table variable, which forces the entire plan to run single-threaded. SQL Server cannot use parallelism for modifications to table variables. Replace with a #temp table to allow parallel execution.",
+                    Severity = PlanWarningSeverity.Critical
+                });
+            }
+        }
+    }
+
+    private static void CheckForTableVariables(PlanNode node, bool isModification,
+        ref bool hasTableVar, ref bool modifiesTableVar)
+    {
+        if (!string.IsNullOrEmpty(node.ObjectName) && node.ObjectName.StartsWith("@"))
+        {
+            hasTableVar = true;
+            // The modification target is typically an Insert/Update/Delete operator on a table variable
+            if (isModification && (node.PhysicalOp.Contains("Insert", StringComparison.OrdinalIgnoreCase)
+                || node.PhysicalOp.Contains("Update", StringComparison.OrdinalIgnoreCase)
+                || node.PhysicalOp.Contains("Delete", StringComparison.OrdinalIgnoreCase)
+                || node.PhysicalOp.Contains("Merge", StringComparison.OrdinalIgnoreCase)))
+            {
+                modifiesTableVar = true;
+            }
+        }
+        foreach (var child in node.Children)
+            CheckForTableVariables(child, isModification, ref hasTableVar, ref modifiesTableVar);
     }
 
     private static void AnalyzeNodeTree(PlanNode node, PlanStatement stmt, AnalyzerConfig cfg)
@@ -572,11 +621,24 @@
                 var skewThreshold = node.PerThreadStats.Count == 2 ? 0.75 : 0.50;
                 if (skewRatio >= skewThreshold)
                 {
+                    var message = $"Thread {maxThread.ThreadId} processed {skewRatio:P0} of rows ({maxThread.ActualRows:N0}/{totalRows:N0}). Work is heavily skewed to one thread, so parallelism isn't helping much.";
+                    var severity = PlanWarningSeverity.Warning;
+
+                    // Batch mode sorts produce all output on a single thread by design
+                    // unless their parent is a batch mode Window Aggregate
+                    if (node.PhysicalOp == "Sort"
+                        && (node.ActualExecutionMode ?? node.ExecutionMode) == "Batch"
+                        && node.Parent?.PhysicalOp != "Window Aggregate")
+                    {
+                        message += " Batch mode sorts produce all output rows on a single thread by design, unless feeding a batch mode Window Aggregate.";
+                        severity = PlanWarningSeverity.Info;
+                    }
+
                     node.Warnings.Add(new PlanWarning
                     {
                         WarningType = "Parallel Skew",
-                        Message = $"Thread {maxThread.ThreadId} processed {skewRatio:P0} of rows ({maxThread.ActualRows:N0}/{totalRows:N0}). Work is heavily skewed to one thread, so parallelism isn't helping much.",
-                        Severity = PlanWarningSeverity.Warning
+                        Message = message,
+                        Severity = severity
                     });
                 }
             }
@@ -603,7 +665,7 @@
             {
                 WarningType = "Key Lookup",
                 Message = $"Key Lookup — SQL Server found rows via a nonclustered index but had to go back to the clustered index for additional columns. Alter the nonclustered index to add the predicate column as a key column or as an INCLUDE column.\nPredicate: {Truncate(node.Predicate, 200)}",
-                Severity = PlanWarningSeverity.Warning
+                Severity = PlanWarningSeverity.Critical
             });
         }
 
@@ -829,35 +891,39 @@
             });
         }
 
-        // 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 (!cfg.IsRuleDisabled(24) && node.PhysicalOp == "Nested Loops" && node.Children.Count >= 2)
+        // Rule 24: Top above a scan
+        // Detects Top or Top N Sort operators feeding from a scan. This often means the
+        // query is scanning the entire table/index and sorting just to return a few rows,
+        // when an appropriate index could satisfy the request directly.
+        if (!cfg.IsRuleDisabled(24))
         {
-            var inner = node.Children[1];
+            var isTop = node.PhysicalOp == "Top";
+            var isTopNSort = node.LogicalOp == "Top N Sort";
 
-            // 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)
+            if ((isTop || isTopNSort) && node.Children.Count > 0)
             {
                 // 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)
+                var scanCandidate = node.Children[0];
+                while ((scanCandidate.PhysicalOp == "Compute Scalar" || scanCandidate.PhysicalOp == "Parallelism")
+                    && scanCandidate.Children.Count > 0)
                     scanCandidate = scanCandidate.Children[0];
 
                 if (IsScanOperator(scanCandidate))
                 {
+                    var topLabel = isTopNSort ? "Top N Sort" : "Top";
+                    var onInner = node.Parent?.PhysicalOp == "Nested Loops" && node.Parent.Children.Count >= 2
+                        && node.Parent.Children[1] == node;
+                    var innerNote = onInner
+                        ? $" This is on the inner side of Nested Loops (Node {node.Parent!.NodeId}), so the scan repeats for every outer row."
+                        : "";
                     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
+                    node.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} Check that you have appropriate indexes to convert the scan into a seek.",
-                        Severity = PlanWarningSeverity.Warning
+                        Message = $"{topLabel} reads from {scanCandidate.PhysicalOp} (Node {scanCandidate.NodeId}).{innerNote}{predInfo} An index on the ORDER BY columns could eliminate the scan and sort entirely.",
+                        Severity = onInner ? PlanWarningSeverity.Critical : PlanWarningSeverity.Warning
                     });
                 }
             }
@@ -883,7 +949,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"))
         {
             var rewinds = node.HasActualStats ? (double)node.ActualRewinds : node.EstimateRewinds;
             if (rewinds > 10000 && HasNotInPattern(node, stmt))
@@ -1097,8 +1163,8 @@
         while (parent != null && parent.PhysicalOp == "Compute Scalar")
             parent = parent.Parent;
 
-        // Expect TopN Sort
-        if (parent == null || parent.LogicalOp != "TopN Sort")
+        // Expect TopN Sort (XML says "TopN Sort", parser normalizes to "Top N Sort")
+        if (parent == null || parent.LogicalOp != "Top N Sort")
             return false;
 
         // Walk up to Merge Interval

src/PlanViewer.Core/Services/ShowPlanParser.cs

@@ -652,6 +652,10 @@ private static PlanNode ParseRelOp(XElement relOpEl)
         var physicalOpEl = GetOperatorElement(relOpEl);
         if (physicalOpEl != null)
         {
+            // Top N Sort — XML element is <TopSort> but PhysicalOp is "Sort"
+            if (physicalOpEl.Name.LocalName == "TopSort")
+                node.LogicalOp = "Top N Sort";
+
             // Object reference (table/index name) — scoped to stop at child RelOps
             var objEl = ScopedDescendants(physicalOpEl, Ns + "Object").FirstOrDefault();
             if (objEl != null)

tests/PlanViewer.Core.Tests/PlanAnalyzerTests.cs

@@ -443,8 +443,13 @@ public void Rule22_TableVariable_DetectsAtSignInObjectName()
         var plan = PlanTestHelper.LoadAndAnalyze("table_variable_plan.sqlplan");
         var warnings = PlanTestHelper.WarningsOfType(plan, "Table Variable");
 
-        Assert.Single(warnings);
-        Assert.Contains("lack column-level statistics", warnings[0].Message);
+        // Node-level + statement-level warnings
+        Assert.True(warnings.Count >= 2);
+        Assert.Contains(warnings, w => w.Message.Contains("lack column-level statistics"));
+        // Statement-level stats warning
+        var stmtWarnings = PlanTestHelper.FirstStatement(plan).PlanWarnings
+            .Where(w => w.WarningType == "Table Variable").ToList();
+        Assert.Contains(stmtWarnings, w => w.Message.Contains("lack column-level statistics"));
     }
 
     // ---------------------------------------------------------------