From f03ea276f30cf6579619320c7aa7336cfbf1b1e3 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Mon, 30 Mar 2026 17:18:46 -0400 Subject: [PATCH] Fix Rule 2 false positive: eager table spools misidentified as eager index spools The condition checked PhysicalOp.Contains("Spool") which matched both "Index Spool" and "Table Spool". Now checks for "Index" specifically. Added negative test with an eager table spool plan. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/PlanViewer.Core/Services/PlanAnalyzer.cs | 2 +- .../PlanAnalyzerTests.cs | 11 + .../Plans/eager_table_spool_plan.sqlplan | 1283 +++++++++++++++++ 3 files changed, 1295 insertions(+), 1 deletion(-) create mode 100644 tests/PlanViewer.Core.Tests/Plans/eager_table_spool_plan.sqlplan diff --git a/src/PlanViewer.Core/Services/PlanAnalyzer.cs b/src/PlanViewer.Core/Services/PlanAnalyzer.cs index 0d577b3..f4c829d 100644 --- a/src/PlanViewer.Core/Services/PlanAnalyzer.cs +++ b/src/PlanViewer.Core/Services/PlanAnalyzer.cs @@ -460,7 +460,7 @@ private static void AnalyzeNode(PlanNode node, PlanStatement stmt, AnalyzerConfi // Rule 2: Eager Index Spools — optimizer building temporary indexes on the fly if (!cfg.IsRuleDisabled(2) && node.LogicalOp == "Eager Spool" && - node.PhysicalOp.Contains("Spool", StringComparison.OrdinalIgnoreCase)) + node.PhysicalOp.Contains("Index", StringComparison.OrdinalIgnoreCase)) { var message = "SQL Server is building a temporary index in TempDB at runtime because no suitable permanent index exists. This is expensive — it builds the index from scratch on every execution. Create a permanent index on the underlying table to eliminate this operator entirely."; if (!string.IsNullOrEmpty(node.SuggestedIndex)) diff --git a/tests/PlanViewer.Core.Tests/PlanAnalyzerTests.cs b/tests/PlanViewer.Core.Tests/PlanAnalyzerTests.cs index e8815ab..28ad002 100644 --- a/tests/PlanViewer.Core.Tests/PlanAnalyzerTests.cs +++ b/tests/PlanViewer.Core.Tests/PlanAnalyzerTests.cs @@ -33,6 +33,17 @@ public void Rule02_EagerIndexSpool_DetectedInLazySpoolPlan() Assert.Contains(warnings, w => w.Message.Contains("temporary index in TempDB")); } + [Fact] + public void Rule02_EagerIndexSpool_NotFiredForEagerTableSpool() + { + // Plan with Eager Table Spool (PhysicalOp="Table Spool", LogicalOp="Eager Spool") + // should NOT trigger the Eager Index Spool warning + var plan = PlanTestHelper.LoadAndAnalyze("eager_table_spool_plan.sqlplan"); + var warnings = PlanTestHelper.WarningsOfType(plan, "Eager Index Spool"); + + Assert.Empty(warnings); + } + // --------------------------------------------------------------- // Rule 3: Serial Plan // --------------------------------------------------------------- diff --git a/tests/PlanViewer.Core.Tests/Plans/eager_table_spool_plan.sqlplan b/tests/PlanViewer.Core.Tests/Plans/eager_table_spool_plan.sqlplan new file mode 100644 index 0000000..40c0a52 --- /dev/null +++ b/tests/PlanViewer.Core.Tests/Plans/eager_table_spool_plan.sqlplan @@ -0,0 +1,1283 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file