diff --git a/src/PlanViewer.App/Controls/PlanViewerControl.Interaction.cs b/src/PlanViewer.App/Controls/PlanViewerControl.Interaction.cs index a67609e..7dd13b2 100644 --- a/src/PlanViewer.App/Controls/PlanViewerControl.Interaction.cs +++ b/src/PlanViewer.App/Controls/PlanViewerControl.Interaction.cs @@ -46,6 +46,49 @@ private void SelectNode(Border border, PlanNode node) UpdateMinimapSelection(node); } + /// + /// Selects the operator with and scrolls it into view, so a warning + /// can take you to where it came from (#440). Returns false when the plan has no such operator, + /// which is what keeps a stale or wrong origin from silently scrolling somewhere arbitrary. + /// + private bool TryNavigateToNode(int nodeId) + { + foreach (var child in PlanCanvas.Children) + { + if (child is not Border border || + !_nodeBorderMap.TryGetValue(border, out var node) || + node.NodeId != nodeId) + { + continue; + } + + SelectNode(border, node); + ScrollNodeIntoView(node); + return true; + } + + return false; + } + + /// + /// Centres the operator in the viewport. Node coordinates are unscaled layout positions, so they + /// are multiplied by the zoom level to get canvas pixels; the offset is then clamped, because + /// asking a ScrollViewer for a negative offset on a plan smaller than the viewport just leaves + /// it where it was and looks like the navigation did nothing. + /// + private void ScrollNodeIntoView(PlanNode node) + { + var targetX = node.X * _zoomLevel - (PlanScrollViewer.Bounds.Width / 2); + var targetY = node.Y * _zoomLevel - (PlanScrollViewer.Bounds.Height / 2); + + var maxX = Math.Max(0, PlanScrollViewer.Extent.Width - PlanScrollViewer.Viewport.Width); + var maxY = Math.Max(0, PlanScrollViewer.Extent.Height - PlanScrollViewer.Viewport.Height); + + PlanScrollViewer.Offset = new Vector( + Math.Clamp(targetX, 0, maxX), + Math.Clamp(targetY, 0, maxY)); + } + private ContextMenu BuildNodeContextMenu(PlanNode node) { var menu = new ContextMenu(); diff --git a/src/PlanViewer.App/Controls/PlanViewerControl.Properties.cs b/src/PlanViewer.App/Controls/PlanViewerControl.Properties.cs index dbd8369..500514c 100644 --- a/src/PlanViewer.App/Controls/PlanViewerControl.Properties.cs +++ b/src/PlanViewer.App/Controls/PlanViewerControl.Properties.cs @@ -822,13 +822,15 @@ private void ShowPropertiesPanel(PlanNode node) var planWarnHeader = w.MaxBenefitPercent.HasValue ? $"\u26A0 {w.WarningType}{sourceTag}{legacyTag} \u2014 up to {FormatBenefitPercent(w.MaxBenefitPercent.Value)}% benefit" : $"\u26A0 {w.WarningType}{sourceTag}{legacyTag}"; - warnPanel.Children.Add(new TextBlock + var planWarnHeaderBlock = new TextBlock { Text = planWarnHeader, FontWeight = FontWeight.SemiBold, FontSize = 11, Foreground = new SolidColorBrush(Color.Parse(warnColor)) - }); + }; + AttachOriginNavigation(planWarnHeaderBlock, planWarnHeader, w.OriginNodeIds); + warnPanel.Children.Add(planWarnHeaderBlock); warnPanel.Children.Add(new TextBlock { Text = w.Message, @@ -875,6 +877,78 @@ private void ShowPropertiesPanel(PlanNode node) PropertiesContent.Children.Add(planWarningsExpander); } + /* === Operator Warnings (#440) === + Every warning hanging off an operator anywhere in this statement, gathered in one + place and each one a link to its operator. + + This section is the point of #440. The reporter's case was "the plan is huge and the + warning origin is murky", and the operator warnings are exactly the ones with a + murky origin - but until now the only way to see one was to already have clicked the + operator it was on, which is no help when you do not know which operator to click. + Nothing is removed from the per-operator panel; this is an index into it. */ + var operatorWarnings = WarningIndex.CollectOperatorWarnings(s.RootNode); + if (operatorWarnings.Count > 0) + { + var operatorWarningsPanel = new StackPanel(); + foreach (var (originNode, w) in operatorWarnings + .OrderByDescending(x => x.Warning.MaxBenefitPercent ?? -1) + .ThenByDescending(x => x.Warning.Severity) + .ThenBy(x => x.Warning.WarningType)) + { + var opWarnColor = w.Severity == PlanWarningSeverity.Critical ? "#E57373" + : w.Severity == PlanWarningSeverity.Warning ? "#FFB347" : "#6BB5FF"; + var opWarnPanel = new StackPanel { Margin = new Thickness(10, 2, 10, 2) }; + var opBenefit = w.MaxBenefitPercent.HasValue + ? $" \u2014 up to {FormatBenefitPercent(w.MaxBenefitPercent.Value)}% benefit" + : ""; + var opHeaderText = + $"\u26A0 {w.WarningType}{WarningSourceTag(w)}{(w.IsLegacy ? " [legacy]" : "")}{opBenefit}"; + var opHeader = new TextBlock + { + Text = opHeaderText, + FontWeight = FontWeight.SemiBold, + FontSize = 11, + Foreground = new SolidColorBrush(Color.Parse(opWarnColor)) + }; + AttachOriginNavigation(opHeader, opHeaderText, w.OriginNodeIds); + opWarnPanel.Children.Add(opHeader); + opWarnPanel.Children.Add(new TextBlock + { + Text = OperatorOriginLabel(originNode), + FontSize = 11, + Foreground = SectionHeaderBrush, + Margin = new Thickness(16, 0, 0, 0) + }); + operatorWarningsPanel.Children.Add(opWarnPanel); + } + + var operatorWarningsExpander = new Expander + { + /* Collapsed by default, unlike Plan Warnings. On a large plan this is the + longest section in the panel, and expanding it by default would push the + statement's own details off screen - the opposite of the problem #440 is + about. */ + IsExpanded = false, + Header = new TextBlock + { + Text = $"Operator Warnings ({operatorWarnings.Count})", + FontWeight = FontWeight.SemiBold, + FontSize = 11, + Foreground = SectionHeaderBrush + }, + Content = operatorWarningsPanel, + Margin = new Thickness(0, 2, 0, 0), + Padding = new Thickness(0), + Foreground = SectionHeaderBrush, + Background = new SolidColorBrush(Color.FromArgb(0x18, 0x4F, 0xA3, 0xFF)), + BorderBrush = PropSeparatorBrush, + BorderThickness = new Thickness(0, 0, 0, 1), + HorizontalAlignment = HorizontalAlignment.Stretch, + HorizontalContentAlignment = HorizontalAlignment.Stretch + }; + PropertiesContent.Children.Add(operatorWarningsExpander); + } + // === Missing Indexes === if (s.MissingIndexes.Count > 0) { diff --git a/src/PlanViewer.App/Controls/PlanViewerControl.Rendering.cs b/src/PlanViewer.App/Controls/PlanViewerControl.Rendering.cs index ae4ef22..ca337fe 100644 --- a/src/PlanViewer.App/Controls/PlanViewerControl.Rendering.cs +++ b/src/PlanViewer.App/Controls/PlanViewerControl.Rendering.cs @@ -516,6 +516,46 @@ private static string FormatBytes(double bytes) private static string FormatBenefitPercent(double pct) => pct >= 100 ? $"{pct:N0}" : $"{pct:N1}"; + /// + /// How an operator is named in the aggregated warning index (#440) — enough to recognise it + /// before clicking, matching what the operator's own panel puts in its header. + /// + private static string OperatorOriginLabel(PlanNode node) => + string.IsNullOrEmpty(node.FullObjectName) + ? $"Node {node.NodeId} \u00B7 {node.PhysicalOp}" + : $"Node {node.NodeId} \u00B7 {node.PhysicalOp} on {node.FullObjectName}"; + + /// + /// Turns a warning header into a link to the operator it came from (#440). + /// + /// Only when the warning actually knows. Findings with no operator origin — "High Compile + /// CPU" happened before a row was read — are left as plain text rather than given a link that + /// goes somewhere arbitrary, because a reader would believe it. + /// + /// Where a warning came from several operators, the first is the navigation target and the + /// rest are named in the tooltip, so the count is visible rather than silently dropped. + /// + private void AttachOriginNavigation(TextBlock header, string headerText, List originNodeIds) + { + if (originNodeIds.Count == 0) + return; + + header.Text = headerText + " \u2192"; + header.Cursor = new Cursor(StandardCursorType.Hand); + ToolTip.SetTip(header, originNodeIds.Count == 1 + ? $"Go to operator (Node {originNodeIds[0]})" + : $"Go to Node {originNodeIds[0]} — also from {string.Join(", ", originNodeIds.Skip(1).Select(id => "Node " + id))}"); + + header.PointerPressed += (_, e) => + { + if (!e.GetCurrentPoint(header).Properties.IsLeftButtonPressed) + return; + + if (TryNavigateToNode(originNodeIds[0])) + e.Handled = true; + }; + } + /// /// #436: marks the warnings SQL Server itself wrote into the plan, so they are not read as one of /// our inferences. Only the engine's are tagged — they are the minority, and a badge on every diff --git a/src/PlanViewer.Core/Models/PlanModels.cs b/src/PlanViewer.Core/Models/PlanModels.cs index 20ecd09..d6341d0 100644 --- a/src/PlanViewer.Core/Models/PlanModels.cs +++ b/src/PlanViewer.Core/Models/PlanModels.cs @@ -391,6 +391,21 @@ public class PlanWarning /// public PlanWarningSource Source { get; set; } = PlanWarningSource.PerformanceStudio; + /// + /// The operators this finding actually came from, so a reader can be taken to them (#440). + /// + /// A LIST rather than a single id, because the three honest answers are genuinely + /// different. A key lookup came from exactly one operator. A table variable warning came from + /// every operator that touched one, which on a big plan is several. And some findings have no + /// operator at all — "High Compile CPU" happened before a single row was read, and + /// "UDF Execution" is reported by SQL Server at the statement level only. Those keep this + /// empty, and the UI offers no navigation rather than picking somewhere arbitrary. + /// + /// Sending a reader to the wrong operator is worse than sending them nowhere, because + /// they would believe it. + /// + public List OriginNodeIds { get; set; } = []; + /// /// Maximum percentage of elapsed time that could be saved by addressing this finding. /// null = not quantifiable, 0 = calculated as negligible. diff --git a/src/PlanViewer.Core/Output/AnalysisResult.cs b/src/PlanViewer.Core/Output/AnalysisResult.cs index 267f4fc..c9a3e70 100644 --- a/src/PlanViewer.Core/Output/AnalysisResult.cs +++ b/src/PlanViewer.Core/Output/AnalysisResult.cs @@ -256,6 +256,15 @@ public class WarningResult /// [JsonPropertyName("source")] public string Source { get; set; } = ""; + + /// + /// Node ids of the operators this finding came from, empty when it has no operator origin — + /// "High Compile CPU" happened before any operator ran (#440). Distinct from + /// , which says where the warning is ATTACHED in this output; these say + /// where it came FROM, and for a statement-level warning there is no NodeId at all. + /// + [JsonPropertyName("origin_node_ids")] + public List OriginNodeIds { get; set; } = []; } public class MissingIndexResult diff --git a/src/PlanViewer.Core/Output/ResultMapper.cs b/src/PlanViewer.Core/Output/ResultMapper.cs index 97996af..83ac5fd 100644 --- a/src/PlanViewer.Core/Output/ResultMapper.cs +++ b/src/PlanViewer.Core/Output/ResultMapper.cs @@ -196,7 +196,8 @@ private static StatementResult MapStatement( MaxBenefitPercent = w.MaxBenefitPercent, ActionableFix = w.ActionableFix, IsLegacy = w.IsLegacy, - Source = w.Source.ToString() + Source = w.Source.ToString(), + OriginNodeIds = w.OriginNodeIds }); } @@ -306,7 +307,8 @@ private static OperatorResult MapNode(PlanNode node, CancellationToken cancellat MaxBenefitPercent = w.MaxBenefitPercent, ActionableFix = w.ActionableFix, IsLegacy = w.IsLegacy, - Source = w.Source.ToString() + Source = w.Source.ToString(), + OriginNodeIds = w.OriginNodeIds }); } diff --git a/src/PlanViewer.Core/Services/PlanAnalyzer.Detection.cs b/src/PlanViewer.Core/Services/PlanAnalyzer.Detection.cs index 29086f6..ce63fa3 100644 --- a/src/PlanViewer.Core/Services/PlanAnalyzer.Detection.cs +++ b/src/PlanViewer.Core/Services/PlanAnalyzer.Detection.cs @@ -21,12 +21,18 @@ private static bool HasBatchModeNode(PlanNode node) return false; } + /* #440: collects the operators it found, because this walk already knows exactly which ones + touched a table variable and used to throw that away. Two lists rather than one, since the + two warnings this feeds are about different operators: every operator referencing a table + variable, versus only the ones modifying it (which is what forces the plan serial). */ private static void CheckForTableVariables(PlanNode node, bool isModification, - ref bool hasTableVar, ref bool modifiesTableVar) + ref bool hasTableVar, ref bool modifiesTableVar, + List? referencingNodeIds = null, List? modifyingNodeIds = null) { if (!string.IsNullOrEmpty(node.ObjectName) && node.ObjectName.StartsWith("@")) { hasTableVar = true; + referencingNodeIds?.Add(node.NodeId); // 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) @@ -34,10 +40,12 @@ private static void CheckForTableVariables(PlanNode node, bool isModification, || node.PhysicalOp.Contains("Merge", StringComparison.OrdinalIgnoreCase))) { modifiesTableVar = true; + modifyingNodeIds?.Add(node.NodeId); } } foreach (var child in node.Children) - CheckForTableVariables(child, isModification, ref hasTableVar, ref modifiesTableVar); + CheckForTableVariables(child, isModification, ref hasTableVar, ref modifiesTableVar, + referencingNodeIds, modifyingNodeIds); } /// diff --git a/src/PlanViewer.Core/Services/PlanAnalyzer.Node.cs b/src/PlanViewer.Core/Services/PlanAnalyzer.Node.cs index 2a702bf..8dcbbc2 100644 --- a/src/PlanViewer.Core/Services/PlanAnalyzer.Node.cs +++ b/src/PlanViewer.Core/Services/PlanAnalyzer.Node.cs @@ -48,6 +48,20 @@ private static void AnalyzeNode(PlanNode node, PlanStatement stmt, AnalyzerConfi Rule28_RowCountSpool(node, stmt, cfg); Rule29_ImplicitConversionSeek(node, stmt, cfg); Rule35_ExpensiveOperator(node, stmt, cfg); + + /* #440: an operator warning's origin is the operator it is hanging off, so it is stamped + here rather than at each of the 26 places above that add one. Same reasoning as the + provenance stamp in ShowPlanParser: a rule you have to remember at every construction + site is a rule that eventually gets forgotten, and the one time it is forgotten the UI + quietly offers no link on a warning that has a perfectly good one. + + Only fills what a rule left empty, so a rule that knows better - one pointing at the + operator that CAUSED the problem rather than the one reporting it - keeps its own answer. */ + foreach (var warning in node.Warnings) + { + if (warning.OriginNodeIds.Count == 0) + warning.OriginNodeIds.Add(node.NodeId); + } } private static void Rule01_FilterOperator(PlanNode node, PlanStatement stmt, AnalyzerConfig cfg) diff --git a/src/PlanViewer.Core/Services/PlanAnalyzer.Statement.cs b/src/PlanViewer.Core/Services/PlanAnalyzer.Statement.cs index 901e5ee..bce1d0c 100644 --- a/src/PlanViewer.Core/Services/PlanAnalyzer.Statement.cs +++ b/src/PlanViewer.Core/Services/PlanAnalyzer.Statement.cs @@ -488,7 +488,10 @@ private static void Rule22Stmt_TableVariable(PlanStatement stmt, AnalyzerConfig 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); + var referencingNodeIds = new List(); + var modifyingNodeIds = new List(); + CheckForTableVariables(stmt.RootNode, isModification, ref hasTableVar, ref modifiesTableVar, + referencingNodeIds, modifyingNodeIds); if (hasTableVar && !modifiesTableVar) { @@ -496,7 +499,8 @@ private static void Rule22Stmt_TableVariable(PlanStatement stmt, AnalyzerConfig { 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 + Severity = PlanWarningSeverity.Warning, + OriginNodeIds = referencingNodeIds }); } @@ -506,7 +510,8 @@ private static void Rule22Stmt_TableVariable(PlanStatement stmt, AnalyzerConfig { 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 + Severity = PlanWarningSeverity.Critical, + OriginNodeIds = modifyingNodeIds }); } } diff --git a/src/PlanViewer.Core/Services/WarningIndex.cs b/src/PlanViewer.Core/Services/WarningIndex.cs new file mode 100644 index 0000000..5d4f8d6 --- /dev/null +++ b/src/PlanViewer.Core/Services/WarningIndex.cs @@ -0,0 +1,41 @@ +using System.Collections.Generic; +using PlanViewer.Core.Models; + +namespace PlanViewer.Core.Services; + +/// +/// Gathers the warnings scattered across an operator tree into one list (#440). +/// +/// Lives in Core rather than next to the panel that renders it for two reasons: it is a plan +/// tree walk over Core's own models with nothing UI about it, and putting it here means it can be +/// tested without standing up Avalonia. +/// +public static class WarningIndex +{ + /// + /// Every warning hanging off an operator beneath , paired with the + /// operator carrying it, in no particular order — callers sort for themselves, because the + /// statement panel wants them by benefit while other callers may not. + /// + public static List<(PlanNode Node, PlanWarning Warning)> CollectOperatorWarnings(PlanNode? root) + { + var collected = new List<(PlanNode, PlanWarning)>(); + if (root == null) + return collected; + + /* Explicit stack rather than recursion: a deep plan is exactly the case this feature exists + for, and #430 was a crash caused by assuming operator trees are shallow. */ + var pending = new Stack(); + pending.Push(root); + while (pending.Count > 0) + { + var node = pending.Pop(); + foreach (var warning in node.Warnings) + collected.Add((node, warning)); + foreach (var child in node.Children) + pending.Push(child); + } + + return collected; + } +} diff --git a/tests/PlanViewer.Core.Tests/HistoricalCliContractTests.cs b/tests/PlanViewer.Core.Tests/HistoricalCliContractTests.cs index 5be64e4..d0d809a 100644 --- a/tests/PlanViewer.Core.Tests/HistoricalCliContractTests.cs +++ b/tests/PlanViewer.Core.Tests/HistoricalCliContractTests.cs @@ -6,13 +6,15 @@ namespace PlanViewer.Core.Tests; public sealed class HistoricalCliContractTests { - /* Rolled for #436, which adds "source" to every warning in the JSON output so a consumer can tell - SQL Server's own warnings from Performance Studio's inferences. The change is additive — nothing - was removed or renamed, so a consumer reading fields by name is unaffected — but anything - diffing or hashing whole output sees different bytes, which is exactly what this constant is - here to make somebody decide on rather than discover. */ + /* Rolled twice now, both times additively and both times deliberately: #436 added "source" to + every warning so a consumer can tell SQL Server's own warnings from Performance Studio's + inferences, and #440 added "origin_node_ids" so a consumer can get from a finding to the + operator that produced it. Nothing has been removed or renamed either time, so a consumer + reading fields by name is unaffected — but anything diffing or hashing whole output sees + different bytes, which is exactly what this constant exists to make somebody decide on rather + than discover. Verified for #440 that the only new key on a warning is origin_node_ids. */ private const string ExpectedCompactOutputSha256 = - "06975e4e513eef23669c86bc2cfeec916f442461d605060bbd996bd44df13405"; + "779a85036b74dc7cc7d26d4bad3d7e2b327e4ff89a5ed79b00b78cfae3717cab"; [Fact] public async Task AnalyzeCompact_PreservesHistoricalOutputBytes() diff --git a/tests/PlanViewer.Core.Tests/WarningOriginTests.cs b/tests/PlanViewer.Core.Tests/WarningOriginTests.cs new file mode 100644 index 0000000..a0f1651 --- /dev/null +++ b/tests/PlanViewer.Core.Tests/WarningOriginTests.cs @@ -0,0 +1,144 @@ +using System.Linq; +using PlanViewer.Core.Models; +using PlanViewer.Core.Output; + +namespace PlanViewer.Core.Tests; + +/// +/// #440: a warning did not record which operator it came from, so on a large plan there was no way +/// to get from a finding to the thing that produced it. +/// +/// The interesting half of this feature is knowing when to say nothing. A warning pointed at the +/// wrong operator is worse than a warning pointed nowhere, because the reader would believe it — so +/// these pin both directions: that origins appear where a rule genuinely knows, and that they stay +/// empty where no operator is responsible. +/// +public class WarningOriginTests +{ + /// + /// Operator warnings are stamped in one place, at the end of AnalyzeNode, rather than at the 26 + /// separate sites that add one. This asserts the property that arrangement buys: across every + /// committed plan, no operator warning is ever left without an origin. + /// + [Fact] + public void EveryOperatorWarningKnowsItsOperator() + { + var plansDir = System.IO.Path.Combine(System.AppContext.BaseDirectory, "Plans"); + var orphans = new System.Collections.Generic.List(); + + foreach (var file in System.IO.Directory.GetFiles(plansDir, "*.sqlplan")) + { + var plan = PlanTestHelper.LoadAndAnalyze(System.IO.Path.GetFileName(file)); + foreach (var stmt in plan.Batches.SelectMany(b => b.Statements)) + { + if (stmt.RootNode == null) continue; + foreach (var (node, warning) in Walk(stmt.RootNode)) + { + if (warning.OriginNodeIds.Count == 0) + orphans.Add($"{System.IO.Path.GetFileName(file)}:{warning.WarningType}"); + else if (!warning.OriginNodeIds.Contains(node.NodeId)) + orphans.Add($"{System.IO.Path.GetFileName(file)}:{warning.WarningType} points away from its own node"); + } + } + } + + Assert.True(orphans.Count == 0, "Operator warnings without a usable origin: " + string.Join(", ", orphans)); + } + + /// + /// The statement-level table variable warning is the case that motivated carrying origins on + /// statement warnings at all: the rule already walked the tree and knew exactly which operators + /// touched a table variable, and threw that away before emitting. + /// + [Fact] + public void TheTableVariableWarningPointsAtTheOperatorsThatTouchOne() + { + var plan = PlanTestHelper.LoadAndAnalyze("table_variable_plan.sqlplan"); + var statementWarnings = plan.Batches + .SelectMany(b => b.Statements) + .SelectMany(s => s.PlanWarnings) + .Where(w => w.WarningType == "Table Variable") + .ToList(); + + Assert.NotEmpty(statementWarnings); + Assert.All(statementWarnings, w => Assert.NotEmpty(w.OriginNodeIds)); + } + + /// + /// The other direction, and the one worth protecting. "High Compile CPU" is measured before a + /// single row is read, so no operator is responsible for it; SQL Server reports "UDF Execution" + /// at the statement level only. Both must stay empty so the UI offers no link rather than a + /// misleading one. + /// + [Theory] + [InlineData("convert_implicit_plan.sqlplan", "High Compile CPU")] + [InlineData("udf_plan.sqlplan", "UDF Execution")] + public void WarningsWithNoResponsibleOperatorClaimNone(string planFile, string warningType) + { + var plan = PlanTestHelper.LoadAndAnalyze(planFile); + var matching = plan.Batches + .SelectMany(b => b.Statements) + .SelectMany(s => s.PlanWarnings) + .Where(w => w.WarningType == warningType) + .ToList(); + + Assert.NotEmpty(matching); + Assert.All(matching, w => Assert.Empty(w.OriginNodeIds)); + } + + /// The JSON consumers get origins as a field, not by re-deriving them from the tree. + [Fact] + public void TheJsonOutputCarriesOrigins() + { + var plan = PlanTestHelper.LoadAndAnalyze("table_variable_plan.sqlplan"); + var result = ResultMapper.Map(plan, "table_variable_plan.sqlplan"); + + var tableVariable = result.Statements + .SelectMany(s => s.Warnings) + .Single(w => w.Type == "Table Variable"); + + Assert.NotEmpty(tableVariable.OriginNodeIds); + } + + /// + /// The index the statement panel is built from. The reporter's case is a plan too big to hunt + /// through by hand, so "found all of them" is the property that matters — this compares the + /// index against an independent recursive walk rather than against a hand-written expected + /// count, so it cannot drift with the fixtures. + /// + [Fact] + public void TheOperatorWarningIndexFindsEveryWarningInTheTree() + { + var plansDir = System.IO.Path.Combine(System.AppContext.BaseDirectory, "Plans"); + var mismatches = new System.Collections.Generic.List(); + + foreach (var file in System.IO.Directory.GetFiles(plansDir, "*.sqlplan")) + { + var plan = PlanTestHelper.LoadAndAnalyze(System.IO.Path.GetFileName(file)); + foreach (var stmt in plan.Batches.SelectMany(b => b.Statements)) + { + if (stmt.RootNode == null) continue; + var indexed = PlanViewer.Core.Services.WarningIndex.CollectOperatorWarnings(stmt.RootNode).Count; + var walked = Walk(stmt.RootNode).Count(); + if (indexed != walked) + mismatches.Add($"{System.IO.Path.GetFileName(file)}: indexed {indexed} vs walked {walked}"); + } + } + + Assert.True(mismatches.Count == 0, string.Join(", ", mismatches)); + } + + /// A statement with no operator tree indexes to nothing rather than throwing. + [Fact] + public void TheOperatorWarningIndexHandlesAMissingTree() + { + Assert.Empty(PlanViewer.Core.Services.WarningIndex.CollectOperatorWarnings(null)); + } + + private static System.Collections.Generic.IEnumerable<(PlanNode Node, PlanWarning Warning)> Walk(PlanNode node) + { + foreach (var w in node.Warnings) yield return (node, w); + foreach (var child in node.Children) + foreach (var pair in Walk(child)) yield return pair; + } +}