Plan analysis: mark which warnings are SQL Server's and key duplicate index suggestions on the database (#4520, #4523)

Darling/Darling.Tests/PlanSync4520Tests.cs

@@ -0,0 +1,87 @@
+/*
+ * Copyright (c) 2026 Erik Darling, Darling Data LLC
+ *
+ * This file is part of the SQL Server Performance Monitor.
+ *
+ * Licensed under the MIT License. See LICENSE file in the project root for full license information.
+ */
+
+using System.Linq;
+using PerformanceMonitor.PlanAnalysis;
+using Xunit;
+
+namespace Darling.Tests;
+
+/// <summary>
+/// #4520 (part of #4511, ported from erikdarlingdata/PerformanceStudio@60eb0bf): a reader could not tell a
+/// warning SQL Server itself wrote into the plan apart from one the analyzer inferred from plan shape, because
+/// both arrive as a <see cref="PlanWarning"/> carrying nothing but type, severity and message.
+///
+/// <para><see cref="PlanWarning.Source"/> defaults to <see cref="PlanWarningSource.Analyzer"/>. Everything
+/// <c>ShowPlanParser.ParseWarningsFromElement</c> returns is stamped <see cref="PlanWarningSource.SqlServer"/>
+/// in one place, so a warning lifted straight out of the plan's own <c>&lt;Warnings&gt;</c> element is marked as
+/// the engine's record, not an inference the analyzer could get wrong for a given plan.</para>
+///
+/// <para>The repro XML is synthetic (<c>dbo.t</c>, <c>[db]</c>): one Table Scan carries both a
+/// <c>PlanAffectingConvert</c> warning (the engine's own record) and a non-SARGable residual predicate that Rule
+/// 12 flags as "Non-SARGable Predicate" (the analyzer's inference).</para>
+/// </summary>
+public sealed class PlanSync4520Tests
+{
+    private const string ReproXml = """
+        <ShowPlanXML xmlns="http://schemas.microsoft.com/sqlserver/2004/07/showplan" Version="1.564" Build="16.0.4215.2"><BatchSequence><Batch><Statements>
+        <StmtSimple StatementText="SELECT c FROM dbo.t WHERE CONVERT_IMPLICIT(varchar, c, 0) = @p" StatementId="1" StatementCompId="1" StatementType="SELECT" RetrievedFromCache="true" QueryHash="0x1111111111111111" QueryPlanHash="0x2222222222222222">
+          <QueryPlan CachedPlanSize="16" CompileTime="1" CompileCPU="1" CompileMemory="104">
+            <RelOp NodeId="0" PhysicalOp="Table Scan" LogicalOp="Table Scan" EstimateRows="1" EstimateIO="0.003" EstimateCPU="0.0001" AvgRowSize="9" EstimatedTotalSubtreeCost="0.0032" TableCardinality="100" Parallel="0" EstimateRebinds="0" EstimateRewinds="0" EstimatedExecutionMode="Row">
+              <OutputList/>
+              <TableScan Ordered="0" ForcedIndex="0" ForceScan="0" NoExpandHint="0" Storage="RowStore">
+                <DefinedValues/>
+                <Predicate><ScalarOperator ScalarString="CONVERT_IMPLICIT(varchar(30),[db].[dbo].[t].[c],0)=@p"/></Predicate>
+                <Object Database="[db]" Schema="[dbo]" Table="[t]" IndexKind="Heap" Storage="RowStore"/>
+              </TableScan>
+              <Warnings>
+                <PlanAffectingConvert ConvertIssue="Cardinality Estimate" Expression="CONVERT_IMPLICIT(varchar(30),[db].[dbo].[t].[c],0)"/>
+              </Warnings>
+            </RelOp>
+          </QueryPlan>
+        </StmtSimple>
+        </Statements></Batch></BatchSequence></ShowPlanXML>
+        """;
+
+    private static ParsedPlan ParseAndAnalyze()
+    {
+        var plan = ShowPlanParser.Parse(ReproXml);
+        PlanAnalyzer.Analyze(plan);
+        return plan;
+    }
+
+    [Fact]
+    public void EngineWarning_IsStampedSqlServer()
+    {
+        var plan = ParseAndAnalyze();
+        var scanNode = plan.Batches.Single().Statements.Single().RootNode!.Children.Single();
+        var convertWarning = scanNode.Warnings.Single(w => w.WarningType == "Implicit Conversion");
+
+        Assert.Equal(PlanWarningSource.SqlServer, convertWarning.Source);
+    }
+
+    [Fact]
+    public void AnalyzerWarning_KeepsTheDefaultSource()
+    {
+        var plan = ParseAndAnalyze();
+        var scanNode = plan.Batches.Single().Statements.Single().RootNode!.Children.Single();
+        var sargableWarning = scanNode.Warnings.Single(w => w.WarningType == "Non-SARGable Predicate");
+
+        Assert.Equal(PlanWarningSource.Analyzer, sargableWarning.Source);
+    }
+
+    [Fact]
+    public void McpFormatter_CarriesSourceOnEachWarning()
+    {
+        // Goes through the product's own MCP formatting path, not the parser directly.
+        var json = McpPlanAnalysisFormatter.BuildAnalysisResult(ReproXml, serverName: null, source: "test", identifier: null);
+
+        Assert.Contains("\"source\":\"SqlServer\"", json);
+        Assert.Contains("\"source\":\"Analyzer\"", json);
+    }
+}

Darling/Darling.Tests/PlanSync4523Tests.cs

@@ -0,0 +1,65 @@
+/*
+ * Copyright (c) 2026 Erik Darling, Darling Data LLC
+ *
+ * This file is part of the SQL Server Performance Monitor.
+ *
+ * Licensed under the MIT License. See LICENSE file in the project root for full license information.
+ */
+
+using System.Linq;
+using PerformanceMonitor.PlanAnalysis;
+using Xunit;
+
+namespace Darling.Tests;
+
+/// <summary>
+/// #4523 (part of #4511, ported from erikdarlingdata/PerformanceStudio@cc18844): Rule 30's duplicate-index-
+/// suggestion check grouped missing index suggestions by <c>$"{Schema}.{Table}"</c>, so a plan touching a
+/// same-named table (<c>dbo.t</c>) in two different databases had its suggestions merged and reported as
+/// duplicates of each other, even though they target unrelated tables. <see cref="MissingIndex"/> already
+/// carries <c>Database</c>; the fix adds it to the grouping key.
+///
+/// <para>The repro XML is synthetic (<c>dbo.t</c>, <c>[db1]</c>/<c>[db2]</c>): one statement with two missing
+/// index groups for <c>[db1].[dbo].[t]</c> and <c>[db2].[dbo].[t]</c>.</para>
+/// </summary>
+public sealed class PlanSync4523Tests
+{
+    private static string ReproXml(string db1, string db2) => $"""
+        <ShowPlanXML xmlns="http://schemas.microsoft.com/sqlserver/2004/07/showplan" Version="1.564" Build="16.0.4215.2"><BatchSequence><Batch><Statements>
+        <StmtSimple StatementText="SELECT c FROM {db1}.dbo.t JOIN {db2}.dbo.t AS t2 ON t2.id = c" StatementId="1" StatementCompId="1" StatementType="SELECT" RetrievedFromCache="true" QueryHash="0x1111111111111111" QueryPlanHash="0x2222222222222222">
+          <QueryPlan CachedPlanSize="16" CompileTime="1" CompileCPU="1" CompileMemory="104">
+            <MissingIndexes>
+              <MissingIndexGroup Impact="50.0"><MissingIndex Database="[{db1}]" Schema="[dbo]" Table="[t]"><ColumnGroup Usage="EQUALITY"><Column Name="[id]" ColumnId="1"/></ColumnGroup></MissingIndex></MissingIndexGroup>
+              <MissingIndexGroup Impact="60.0"><MissingIndex Database="[{db2}]" Schema="[dbo]" Table="[t]"><ColumnGroup Usage="EQUALITY"><Column Name="[id]" ColumnId="1"/></ColumnGroup></MissingIndex></MissingIndexGroup>
+            </MissingIndexes>
+            <RelOp NodeId="0" PhysicalOp="Table Scan" LogicalOp="Table Scan" EstimateRows="1" EstimateIO="0.003" EstimateCPU="0.0001" AvgRowSize="9" EstimatedTotalSubtreeCost="0.0032" TableCardinality="100" Parallel="0" EstimateRebinds="0" EstimateRewinds="0" EstimatedExecutionMode="Row"><OutputList/><TableScan Ordered="0" ForcedIndex="0" ForceScan="0" NoExpandHint="0" Storage="RowStore"><DefinedValues/><Object Database="[{db1}]" Schema="[dbo]" Table="[t]" IndexKind="Heap" Storage="RowStore"/></TableScan></RelOp>
+          </QueryPlan>
+        </StmtSimple>
+        </Statements></Batch></BatchSequence></ShowPlanXML>
+        """;
+
+    private static ParsedPlan ParseAndAnalyze(string db1, string db2)
+    {
+        var plan = ShowPlanParser.Parse(ReproXml(db1, db2));
+        PlanAnalyzer.Analyze(plan);
+        return plan;
+    }
+
+    [Fact]
+    public void SameTableName_DifferentDatabases_AreNotReportedAsDuplicates()
+    {
+        var plan = ParseAndAnalyze("db1", "db2");
+        var stmt = plan.Batches.Single().Statements.Single();
+
+        Assert.DoesNotContain(stmt.PlanWarnings, w => w.WarningType == "Duplicate Index Suggestions");
+    }
+
+    [Fact]
+    public void SameTableName_SameDatabase_AreStillReportedAsDuplicates()
+    {
+        var plan = ParseAndAnalyze("db1", "db1");
+        var stmt = plan.Batches.Single().Statements.Single();
+
+        Assert.Contains(stmt.PlanWarnings, w => w.WarningType == "Duplicate Index Suggestions");
+    }
+}

PerformanceMonitor.PlanAnalysis/McpPlanAnalysisFormatter.cs

@@ -154,7 +154,8 @@ RankedByActualElapsed. truncated is OBSERVED against the whole population (every
                     {
                         severity = w.Severity.ToString(),
                         type = w.WarningType,
-                        message = w.Message
+                        message = w.Message,
+                        source = w.Source.ToString()
                     }),
                     warning_count = allWarnings.Count,
                     critical_count = allWarnings.Count(w => w.Severity == PlanWarningSeverity.Critical),

PerformanceMonitor.PlanAnalysis/PlanAnalyzer.cs

@@ -336,15 +336,15 @@ private static void AnalyzeStatement(PlanStatement stmt)
         {
             // Detect duplicate suggestions for the same table
             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)

PerformanceMonitor.PlanAnalysis/PlanModels.cs

@@ -394,6 +394,17 @@ public class PlanWarning
     public PlanWarningSeverity Severity { get; set; }
     public SpillDetail? SpillDetails { get; set; }
 
+    /// <summary>
+    /// Who says so — SQL Server itself, or the analyzer. Defaults to <see cref="PlanWarningSource.Analyzer"/>
+    /// because the analyzer builds the large majority of warnings. Everything the parser lifts out of
+    /// the plan's own &lt;Warnings&gt; element is stamped <see cref="PlanWarningSource.SqlServer"/> in one
+    /// place, at the single return of ShowPlanParser.ParseWarningsFromElement, so a new engine warning
+    /// cannot be added and forgotten. A warning the engine wrote into the plan is a record of what
+    /// happened when the query ran; an analyzer rule is an inference from plan shape, and an inference
+    /// can be wrong about a particular plan in a way the engine's own record cannot be.
+    /// </summary>
+    public PlanWarningSource Source { get; set; } = PlanWarningSource.Analyzer;
+
     /// <summary>
     /// Maximum percentage of elapsed time that could be saved by addressing this finding.
     /// null = not quantifiable, 0 = calculated as negligible.
@@ -408,6 +419,16 @@ public class PlanWarning
 
 public enum PlanWarningSeverity { Info, Warning, Critical }
 
+/// <summary>Where a <see cref="PlanWarning"/> came from. See <see cref="PlanWarning.Source"/>.</summary>
+public enum PlanWarningSource
+{
+    /// <summary>An inference of the analyzer's, from the shape of the plan.</summary>
+    Analyzer,
+
+    /// <summary>Read out of the plan's own &lt;Warnings&gt; element — the engine's record, not the analyzer's.</summary>
+    SqlServer
+}
+
 public class MemoryGrantInfo
 {
     public long SerialRequiredMemoryKB { get; set; }

PerformanceMonitor.PlanAnalysis/ShowPlanParser.cs

@@ -1764,6 +1764,12 @@ private static List<PlanWarning> ParseWarningsFromElement(XElement warningsEl)
             });
         }
 
+        /* Stamped here rather than on each construction above, so that everything read out of the
+           plan's own <Warnings> element is marked as the engine's, including whatever gets added to
+           this method next. This is the only place parser warnings are built. */
+        foreach (var warning in result)
+            warning.Source = PlanWarningSource.SqlServer;
+
         return result;
     }
 

PerformanceMonitor.Ui/PlanViewerControl.Properties.cs

@@ -818,7 +818,7 @@ private void ShowPropertiesPanel(PlanNode node)
                     var warnPanel = new StackPanel { Margin = new Thickness(10, 2, 10, 2) };
                     warnPanel.Children.Add(new TextBlock
                     {
-                        Text = $"\u26A0 {w.WarningType}",
+                        Text = $"\u26A0 {w.WarningType}{WarningSourceTag(w)}",
                         FontWeight = FontWeights.SemiBold,
                         FontSize = 11,
                         Foreground = new SolidColorBrush((Color)ColorConverter.ConvertFromString(warnColor))
@@ -859,7 +859,7 @@ private void ShowPropertiesPanel(PlanNode node)
                 var warnPanel = new StackPanel { Margin = new Thickness(10, 2, 10, 2) };
                 warnPanel.Children.Add(new TextBlock
                 {
-                    Text = $"\u26A0 {w.WarningType}",
+                    Text = $"\u26A0 {w.WarningType}{WarningSourceTag(w)}",
                     FontWeight = FontWeights.SemiBold,
                     FontSize = 11,
                     Foreground = new SolidColorBrush((Color)ColorConverter.ConvertFromString(warnColor))
@@ -1117,6 +1117,14 @@ private void ShowWaitStats(List<WaitStatInfo> waits, bool isActualPlan)
 
     // Wait category + color come from the shared ChartPalette (PerformanceStudio's ~21-category
     // taxonomy + palette), with a light-theme override for the near-invisible pale colors (D3).
+    /// <summary>
+    /// #4520: marks the warnings SQL Server itself wrote into the plan, so they are not read as one of
+    /// the analyzer's inferences. Only the engine's are tagged — they are the minority, and a badge on
+    /// every warning would carry no information.
+    /// </summary>
+    private static string WarningSourceTag(PlanWarning warning) =>
+        warning.Source == PlanWarningSource.SqlServer ? " [SQL Server]" : "";
+
     private static string GetWaitCategory(string waitType) => ChartPalette.WaitCategory(waitType);
 
     private static string GetWaitCategoryColor(string category)