Fix four analyzer rule gates (#577, #576, #579, #578)

#577: rule 5 reported a row-estimate mismatch on operators that never
executed. Skip ActualExecutions = 0, as rules 11, 12 and 29 already do.

#576: rule 6 dropped its scalar-UDF warning whenever the statement's
NonParallelPlanReason was one rule 3 explains, even when rule 3 was
disabled or gated out. Suppress it only when rule 3's Serial Plan finding
is actually on the statement.

#579: rules matched hints and keywords in the raw statement text, so
string literals and comments counted as code. Add MaskCommentsAndLiterals
and use it for rule 27 (OPTIMIZE FOR UNKNOWN) and for the same pattern in
rule 3 (MAXDOP 1), rule 20 (RECOMPILE), rule 28 (NOT IN), rule 37 (cursor
declaration) and the rule 26 row-goal cause.

#578: rule 30 grouped missing-index suggestions by schema and table, so
same-named tables in two databases looked like duplicates. Add the
database to the key.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G625JBNh45iTR1hpT4CxNR

Author: erikdarlingdata

src/PlanViewer.Core/Services/PlanAnalyzer.Detection.cs

@@ -93,7 +93,7 @@ private static bool HasNotInPattern(PlanNode spoolNode, PlanStatement stmt)
     {
         // Check statement text for NOT IN
         if (string.IsNullOrEmpty(stmt.StatementText) ||
-            !Regex.IsMatch(stmt.StatementText, @"\bNOT\s+IN\b", RegexOptions.IgnoreCase))
+            !Regex.IsMatch(MaskCommentsAndLiterals(stmt.StatementText), @"\bNOT\s+IN\b", RegexOptions.IgnoreCase)) // #579
             return false;
 
         // Walk up the tree checking ancestors and their children

src/PlanViewer.Core/Services/PlanAnalyzer.Helpers.cs

@@ -281,6 +281,91 @@ private static string FormatNodeRef(PlanNode node)
         return $"{node.PhysicalOp} (Node {node.NodeId})";
     }
 
+    /* #579: the rules that look for a hint or a keyword in the query text (MAXDOP 1, RECOMPILE,
+       OPTIMIZE FOR UNKNOWN, NOT IN, a cursor declaration, a row goal's cause) matched the raw
+       StatementText, so the words inside a string literal or a comment counted as code. This
+       blanks string-literal contents and whole comments with spaces, keeping every other character
+       where it was, so a match in the result is a match in the code. The scan follows
+       ParameterSubstitution's: '...' with doubled-quote escapes, -- to the end of the line, and
+       block comments, which nest in T-SQL. Delimited identifiers ("..." and [...]) are stepped
+       over unchanged, so a quote or a dash inside one does not start a string or a comment. */
+    private static string MaskCommentsAndLiterals(string? text)
+    {
+        if (string.IsNullOrEmpty(text))
+            return "";
+
+        var chars = text.ToCharArray();
+        var i = 0;
+        while (i < chars.Length)
+        {
+            var c = chars[i];
+            if (c == '\'' || c == '"' || c == '[')
+            {
+                var close = c == '[' ? ']' : c;
+                var end = i + 1;
+                while (end < chars.Length)
+                {
+                    if (chars[end] == close)
+                    {
+                        if (end + 1 < chars.Length && chars[end + 1] == close)
+                        {
+                            end += 2;
+                            continue;
+                        }
+                        break;
+                    }
+                    end++;
+                }
+                if (c == '\'')
+                {
+                    for (var k = i + 1; k < end && k < chars.Length; k++)
+                        chars[k] = ' ';
+                }
+                i = end + 1;
+                continue;
+            }
+
+            if (c == '-' && i + 1 < chars.Length && chars[i + 1] == '-')
+            {
+                while (i < chars.Length && chars[i] != '\n' && chars[i] != '\r')
+                    chars[i++] = ' ';
+                continue;
+            }
+
+            if (c == '/' && i + 1 < chars.Length && chars[i + 1] == '*')
+            {
+                var depth = 0;
+                while (i < chars.Length)
+                {
+                    if (chars[i] == '/' && i + 1 < chars.Length && chars[i + 1] == '*')
+                    {
+                        depth++;
+                        chars[i++] = ' ';
+                        chars[i++] = ' ';
+                        continue;
+                    }
+                    if (chars[i] == '*' && i + 1 < chars.Length && chars[i + 1] == '/')
+                    {
+                        depth--;
+                        chars[i++] = ' ';
+                        chars[i++] = ' ';
+                        if (depth == 0)
+                            break;
+                        continue;
+                    }
+                    if (chars[i] != '\n' && chars[i] != '\r')
+                        chars[i] = ' ';
+                    i++;
+                }
+                continue;
+            }
+
+            i++;
+        }
+
+        return new string(chars);
+    }
+
     /// <summary>
     /// Identifies the specific cause of a row goal from the statement text.
     /// Returns a specific cause when detectable, or a generic list as fallback.
@@ -290,7 +375,7 @@ private static string IdentifyRowGoalCause(string stmtText)
         if (string.IsNullOrEmpty(stmtText))
             return "TOP, EXISTS, IN, or FAST hint";
 
-        var text = stmtText.ToUpperInvariant();
+        var text = MaskCommentsAndLiterals(stmtText).ToUpperInvariant();
         var causes = new List<string>(4);
 
         if (Regex.IsMatch(text, @"\bTOP\b"))

src/PlanViewer.Core/Services/PlanAnalyzer.Node.cs

@@ -151,7 +151,11 @@ private static void Rule05_RowEstimateMismatch(PlanNode node, PlanStatement stmt
         // - A parent join may have chosen the wrong strategy
         // - Root nodes with no parent to harm are skipped
         // - Nodes whose only parents are Parallelism/Top/Sort (no spill) are skipped
+        /* #577: an operator that never executed returned zero rows because it never ran, so its
+           zero is no evidence that the estimate was wrong. Rules 11, 12 and 29 skip such
+           operators the same way. */
         if (!cfg.IsRuleDisabled(5) && node.HasActualStats && node.EstimateRows > 0
+            && node.ActualExecutions > 0
             && !node.Lookup) // Key lookups are point lookups (1 row per execution) — per-execution estimate is misleading
         {
             if (node.ActualRows == 0)
@@ -204,10 +208,15 @@ private static void Rule06_ScalarUdfReference(PlanNode node, PlanStatement stmt,
         // Rule 6: Scalar UDF references (works on estimated plans too)
         // Suppress when Serial Plan warning is already firing for a UDF-related reason —
         // the Serial Plan warning already explains the issue, this would be redundant.
-        var serialPlanCoversUdf = stmt.NonParallelPlanReason is
-            "TSQLUserDefinedFunctionsNotParallelizable"
-            or "CLRUserDefinedFunctionRequiresDataAccess"
-            or "CouldNotGenerateValidParallelPlan";
+        /* #576: "already firing" has to be checked, not assumed from the reason. Rule 3 can be
+           disabled, and it skips statements that cost under 1, TRIVIAL plans and 0 ms runs, so
+           the UDF warning used to vanish with nothing in its place. Statement rules run before
+           node rules, so rule 3's finding is on the statement by now if it fired. */
+        var serialPlanCoversUdf =
+            (stmt.NonParallelPlanReason is "TSQLUserDefinedFunctionsNotParallelizable"
+                or "CLRUserDefinedFunctionRequiresDataAccess"
+                or "CouldNotGenerateValidParallelPlan")
+            && stmt.PlanWarnings.Any(w => w.WarningType == "Serial Plan");
         if (!cfg.IsRuleDisabled(6) && !serialPlanCoversUdf)
             foreach (var udf in node.ScalarUdfs)
             {

src/PlanViewer.Core/Services/PlanAnalyzer.Statement.cs

@@ -135,7 +135,7 @@ private static void Rule03_SerialPlan(PlanStatement stmt, AnalyzerConfig cfg, Se
             // SQL Server truncates StatementText at ~4,000 characters in plan XML.
             if (stmt.NonParallelPlanReason == "MaxDOPSetToOne")
             {
-                var text = stmt.StatementText ?? "";
+                var text = MaskCommentsAndLiterals(stmt.StatementText); // #579
                 var hasMaxdop1InText = Regex.IsMatch(text, @"MAXDOP\s+1\b", RegexOptions.IgnoreCase);
                 var isTruncated = stmt.IsTextTruncated;
 
@@ -306,7 +306,8 @@ private static void Rule20_LocalVariablesNoRecompile(PlanStatement stmt, Analyze
 
             if (unsnifffedParams.Count > 0)
             {
-                var hasRecompile = stmt.StatementText?.Contains("RECOMPILE", StringComparison.OrdinalIgnoreCase) == true;
+                var hasRecompile = MaskCommentsAndLiterals(stmt.StatementText) // #579
+                    .Contains("RECOMPILE", StringComparison.OrdinalIgnoreCase);
                 if (!hasRecompile)
                 {
                     var names = string.Join(", ", unsnifffedParams.Select(p => p.Name));
@@ -330,7 +331,7 @@ private static void Rule27_OptimizeForUnknown(PlanStatement stmt, AnalyzerConfig
     {
         // Rule 27: OPTIMIZE FOR UNKNOWN in statement text
         if (!cfg.IsRuleDisabled(27) && !string.IsNullOrEmpty(stmt.StatementText) &&
-            Regex.IsMatch(stmt.StatementText, @"OPTIMIZE\s+FOR\s+UNKNOWN", RegexOptions.IgnoreCase))
+            Regex.IsMatch(MaskCommentsAndLiterals(stmt.StatementText), @"OPTIMIZE\s+FOR\s+UNKNOWN", RegexOptions.IgnoreCase)) // #579
         {
             stmt.PlanWarnings.Add(new PlanWarning
             {
@@ -377,7 +378,7 @@ private static void Rule37_CursorWithoutLocal(PlanStatement stmt, AnalyzerConfig
             // the SELECT. Capturing tokens *before* CURSOR never sees LOCAL and would
             // fire on every cursor, including ones already declared LOCAL.
             var cursorDeclMatch = Regex.Match(
-                stmt.StatementText,
+                MaskCommentsAndLiterals(stmt.StatementText), // #579
                 @"\bDECLARE\s+\w+\s+(?:INSENSITIVE\s+|SCROLL\s+)*CURSOR\b(.*?)\bFOR\b",
                 RegexOptions.IgnoreCase | RegexOptions.Singleline);
             if (cursorDeclMatch.Success)
@@ -451,16 +452,18 @@ private static void Rule30_MissingIndexQuality(PlanStatement stmt, AnalyzerConfi
         if (!cfg.IsRuleDisabled(30))
         {
             // Detect duplicate suggestions for the same table
+            /* #578: the database is part of the key. dbo.T in database A and dbo.T in database B
+               are two tables, and their indexes cannot be consolidated into one. */
             var tableSuggestionCount = stmt.MissingIndexes
-                .GroupBy(mi => $"{mi.Schema}.{mi.Table}", StringComparer.OrdinalIgnoreCase)
+                .GroupBy(mi => $"{mi.Database}.{mi.Schema}.{mi.Table}", StringComparer.OrdinalIgnoreCase)
                 .Where(g => g.Count() > 1)
                 .ToDictionary(g => g.Key, g => g.Count(), StringComparer.OrdinalIgnoreCase);
 
             foreach (var mi in stmt.MissingIndexes)
             {
                 var keyCount = mi.EqualityColumns.Count + mi.InequalityColumns.Count;
                 var includeCount = mi.IncludeColumns.Count;
-                var tableKey = $"{mi.Schema}.{mi.Table}";
+                var tableKey = $"{mi.Database}.{mi.Schema}.{mi.Table}";
 
                 // Low-impact suggestion (< 25% improvement)
                 if (mi.Impact < 25)