From 4e39244d158ac99875f3c719a7d6a7b729e7012d Mon Sep 17 00:00:00 2001
From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com>
Date: Mon, 28 Sep 2026 21:35:29 -0400
Subject: [PATCH 1/3] Rule 9: an Excessive Memory Grant now fires for a grant
that used none of it (#4686)
A grant of 1 GB or more with MaxUsedMemory=0 is the largest possible waste, but the check
required MaxUsedMemory above 0 and skipped it. MemoryGrantInfo.HasMaxUsedMemory now records
whether the plan XML carried the attribute, so a plan with no MaxUsedMemory (estimated, or
no runtime grant info) still does not read as used-nothing. Follows PerformanceStudio#608.
---
.../memory_grant_wait_plan.sqlplan | 572 ++++++++++++++++++
Darling/Darling.Tests/PlanSync4531Tests.cs | 3 +-
.../PlanSync4535StatementRulesTests.cs | 2 +-
.../PlanSync4686ExcessiveMemoryGrantTests.cs | 172 ++++++
.../PlanAnalyzer.cs | 18 +-
PerformanceMonitor.PlanAnalysis/PlanModels.cs | 7 +
.../ShowPlanParser.cs | 2 +
7 files changed, 767 insertions(+), 9 deletions(-)
create mode 100644 Darling/Darling.Tests/Fixtures/OriginPlans/memory_grant_wait_plan.sqlplan
create mode 100644 Darling/Darling.Tests/PlanSync4686ExcessiveMemoryGrantTests.cs
diff --git a/Darling/Darling.Tests/Fixtures/OriginPlans/memory_grant_wait_plan.sqlplan b/Darling/Darling.Tests/Fixtures/OriginPlans/memory_grant_wait_plan.sqlplan
new file mode 100644
index 000000000..b27fd82de
--- /dev/null
+++ b/Darling/Darling.Tests/Fixtures/OriginPlans/memory_grant_wait_plan.sqlplan
@@ -0,0 +1,572 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
\ No newline at end of file
diff --git a/Darling/Darling.Tests/PlanSync4531Tests.cs b/Darling/Darling.Tests/PlanSync4531Tests.cs
index a82e62d90..08a59dfbe 100644
--- a/Darling/Darling.Tests/PlanSync4531Tests.cs
+++ b/Darling/Darling.Tests/PlanSync4531Tests.cs
@@ -34,7 +34,8 @@ private static ParsedPlan Analyze(PlanStatement stmt)
MemoryGrant = new MemoryGrantInfo
{
GrantedMemoryKB = 2_097_152, // 2 GB
- MaxUsedMemoryKB = 1024 // 1 MB used — well past the 10x/1GB thresholds
+ MaxUsedMemoryKB = 1024, // 1 MB used — well past the 10x/1GB thresholds
+ HasMaxUsedMemory = true // an actual plan reports MaxUsedMemory
}
};
diff --git a/Darling/Darling.Tests/PlanSync4535StatementRulesTests.cs b/Darling/Darling.Tests/PlanSync4535StatementRulesTests.cs
index dda9bc9c5..1e7c0dbb4 100644
--- a/Darling/Darling.Tests/PlanSync4535StatementRulesTests.cs
+++ b/Darling/Darling.Tests/PlanSync4535StatementRulesTests.cs
@@ -45,7 +45,7 @@ private static AnalyzerConfig Disabling(int ruleNumber) =>
private static PlanStatement Rule9_MemoryGrant() => new()
{
- MemoryGrant = new MemoryGrantInfo { GrantedMemoryKB = 2097152, MaxUsedMemoryKB = 10240 }
+ MemoryGrant = new MemoryGrantInfo { GrantedMemoryKB = 2097152, MaxUsedMemoryKB = 10240, HasMaxUsedMemory = true }
};
private static PlanStatement Rule18_CompileMemoryExceeded() => new()
diff --git a/Darling/Darling.Tests/PlanSync4686ExcessiveMemoryGrantTests.cs b/Darling/Darling.Tests/PlanSync4686ExcessiveMemoryGrantTests.cs
new file mode 100644
index 000000000..ba20cfc3b
--- /dev/null
+++ b/Darling/Darling.Tests/PlanSync4686ExcessiveMemoryGrantTests.cs
@@ -0,0 +1,172 @@
+/*
+ * 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;
+using System.IO;
+using System.Linq;
+using PerformanceMonitor.PlanAnalysis;
+using Xunit;
+
+namespace Darling.Tests;
+
+///
+/// #4686 (erikdarlingdata/PerformanceStudio#608) - Rule 9's excessive-grant check. A grant of 1 GB or
+/// more that used none of it is the worst waste there is, and the check used to skip it because it only
+/// looked at plans where MaxUsedMemory was above 0. The other half of the fix is just as important: a
+/// plan that has no MaxUsedMemory at all (an estimated plan, or one with no runtime grant info) must not
+/// read as "used nothing".
+///
+/// memory_grant_wait_plan.sqlplan (copied verbatim from PerformanceStudio's public repo) has a
+/// 10,851,312 KB grant, so it clears the 1 GB floor. Each test edits the MemoryGrantInfo attributes in
+/// the XML text, so the parser and the analyzer are both in the path.
+///
+public sealed class PlanSync4686ExcessiveMemoryGrantTests
+{
+ private const string Fixture = "memory_grant_wait_plan.sqlplan";
+ private const string Granted = "GrantedMemory=\"10851312\"";
+ private const string MaxUsed = "MaxUsedMemory=\"10232840\"";
+
+ private static ParsedPlan Analyze(Func editXml)
+ {
+ var path = Path.Combine(AppContext.BaseDirectory, "Fixtures", "OriginPlans", Fixture);
+ var xml = File.ReadAllText(path);
+ Assert.Contains(Granted, xml);
+ Assert.Contains(MaxUsed, xml);
+
+ var plan = ShowPlanParser.Parse(editXml(xml));
+ PlanAnalyzer.Analyze(plan);
+ return plan;
+ }
+
+ private static PlanWarning[] Excessive(ParsedPlan plan) =>
+ PlanStatements.EnumerateAll(plan)
+ .SelectMany(s => s.PlanWarnings)
+ .Where(w => w.WarningType == "Excessive Memory Grant")
+ .ToArray();
+
+ private static PlanStatement FirstStatement(ParsedPlan plan) =>
+ PlanStatements.EnumerateAll(plan).First(s => s.MemoryGrant != null);
+
+ [Fact]
+ public void GrantThatUsedNothing_Fires()
+ {
+ var plan = Analyze(xml => xml.Replace(MaxUsed, "MaxUsedMemory=\"0\""));
+
+ var warning = Assert.Single(Excessive(plan));
+ Assert.Equal(9, warning.RuleNumber);
+ Assert.Equal(PlanWarningSeverity.Warning, warning.Severity);
+ Assert.Contains("used none of it", warning.Message);
+ }
+
+ [Fact]
+ public void GrantThatUsedNothing_MessageHasNoRatio()
+ {
+ var plan = Analyze(xml => xml.Replace(MaxUsed, "MaxUsedMemory=\"0\""));
+
+ var message = Assert.Single(Excessive(plan)).Message;
+ Assert.DoesNotContain("overestimate", message);
+ Assert.DoesNotContain("only used", message);
+ Assert.DoesNotContain("Infinity", message);
+ Assert.DoesNotContain("NaN", message);
+ }
+
+ [Fact]
+ public void MaxUsedMemoryMissingFromXml_DoesNotFire()
+ {
+ // No MaxUsedMemory attribute, as in an estimated plan or a plan with no runtime grant info.
+ // The parser reports 0 for it, and 0 there means "not reported", not "used nothing".
+ var plan = Analyze(xml => xml.Replace(" " + MaxUsed, ""));
+
+ Assert.Empty(Excessive(plan));
+ }
+
+ [Fact]
+ public void MaxUsedMemoryMissingFromXml_ParserSaysItWasNotReported()
+ {
+ var absent = Analyze(xml => xml.Replace(" " + MaxUsed, ""));
+ var zero = Analyze(xml => xml.Replace(MaxUsed, "MaxUsedMemory=\"0\""));
+ var present = Analyze(xml => xml);
+
+ Assert.False(FirstStatement(absent).MemoryGrant!.HasMaxUsedMemory);
+ Assert.True(FirstStatement(zero).MemoryGrant!.HasMaxUsedMemory);
+ Assert.True(FirstStatement(present).MemoryGrant!.HasMaxUsedMemory);
+ }
+
+ [Fact]
+ public void GrantThatUsedNothing_JustUnderOneGb_DoesNotFire()
+ {
+ var plan = Analyze(xml => xml
+ .Replace(Granted, "GrantedMemory=\"1048575\"")
+ .Replace(MaxUsed, "MaxUsedMemory=\"0\""));
+
+ Assert.Empty(Excessive(plan));
+ }
+
+ [Fact]
+ public void GrantThatUsedNothing_AtOneGbExactly_Fires()
+ {
+ var plan = Analyze(xml => xml
+ .Replace(Granted, "GrantedMemory=\"1048576\"")
+ .Replace(MaxUsed, "MaxUsedMemory=\"0\""));
+
+ Assert.Single(Excessive(plan));
+ }
+
+ [Fact]
+ public void GrantThatUsedALittle_StillReportsTheRatio()
+ {
+ // 10,851,312 KB granted, 1,000 KB used: the old ratio message, unchanged.
+ var plan = Analyze(xml => xml.Replace(MaxUsed, "MaxUsedMemory=\"1000\""));
+
+ var message = Assert.Single(Excessive(plan)).Message;
+ Assert.Contains("only used", message);
+ Assert.Contains("x overestimate", message);
+ Assert.DoesNotContain("used none of it", message);
+ }
+
+ [Fact]
+ public void GrantThatUsedMostOfIt_DoesNotFire()
+ {
+ // The fixture as shipped: 10,851,312 KB granted, 10,232,840 KB used.
+ var plan = Analyze(xml => xml);
+
+ Assert.Empty(Excessive(plan));
+ }
+
+ [Fact]
+ public void GrantUnderTenTimesTheUse_DoesNotFire()
+ {
+ // 5.4x, under the 10x floor.
+ var plan = Analyze(xml => xml.Replace(MaxUsed, "MaxUsedMemory=\"2000000\""));
+
+ Assert.Empty(Excessive(plan));
+ }
+
+ [Fact]
+ public void GrantThatUsedNothing_OnAnAdaptiveJoinThatRanNestedLoops_KeepsTheAdaptiveJoinNote()
+ {
+ // The #4531 note is appended to both messages, so a hand-built statement (no XML) with a used-none
+ // grant and an adaptive join that ran Nested Loops carries it after the used-none sentence.
+ var stmt = new PlanStatement
+ {
+ RootNode = new PlanNode
+ {
+ PhysicalOp = "Adaptive Join",
+ LogicalOp = "Adaptive Join",
+ IsAdaptive = true,
+ ActualJoinType = "Nested Loops"
+ },
+ MemoryGrant = new MemoryGrantInfo { GrantedMemoryKB = 2_097_152, MaxUsedMemoryKB = 0, HasMaxUsedMemory = true }
+ };
+ PlanAnalyzer.Analyze(new ParsedPlan { Batches = [new PlanBatch { Statements = [stmt] }] });
+
+ var message = Assert.Single(stmt.PlanWarnings, w => w.WarningType == "Excessive Memory Grant").Message;
+ Assert.Contains("used none of it", message);
+ Assert.Contains("adaptive join", message);
+ }
+}
diff --git a/PerformanceMonitor.PlanAnalysis/PlanAnalyzer.cs b/PerformanceMonitor.PlanAnalysis/PlanAnalyzer.cs
index 2cc952967..36533f795 100644
--- a/PerformanceMonitor.PlanAnalysis/PlanAnalyzer.cs
+++ b/PerformanceMonitor.PlanAnalysis/PlanAnalyzer.cs
@@ -365,15 +365,19 @@ private static void AnalyzeStatement(PlanStatement stmt, AnalyzerConfig cfg, Ser
{
var grant = stmt.MemoryGrant;
- // Excessive grant — granted far more than actually used
- if (grant.GrantedMemoryKB > 0 && grant.MaxUsedMemoryKB > 0)
- {
- var wasteRatio = (double)grant.GrantedMemoryKB / grant.MaxUsedMemoryKB;
- if (wasteRatio >= 10 && grant.GrantedMemoryKB >= 1048576)
+ // Excessive grant — granted far more than actually used. A grant that used nothing is the
+ // worst case, so MaxUsedMemory="0" counts, but only when the plan reported it: a plan with
+ // no MaxUsedMemory attribute (estimated, or no runtime grant info) says nothing about use (#4686).
+ if (grant.GrantedMemoryKB >= 1048576 && grant.HasMaxUsedMemory)
+ {
+ var usedNothing = grant.MaxUsedMemoryKB <= 0;
+ var wasteRatio = usedNothing ? 0 : (double)grant.GrantedMemoryKB / grant.MaxUsedMemoryKB;
+ if (usedNothing || wasteRatio >= 10)
{
var grantMB = grant.GrantedMemoryKB / 1024.0;
- var usedMB = grant.MaxUsedMemoryKB / 1024.0;
- var message = $"Granted {grantMB:N0} MB but only used {usedMB:N0} MB ({wasteRatio:F0}x overestimate). The unused memory is reserved and unavailable to other queries.";
+ var message = usedNothing
+ ? $"Granted {grantMB:N0} MB but the query used none of it. The unused memory is reserved and unavailable to other queries."
+ : $"Granted {grantMB:N0} MB but only used {grant.MaxUsedMemoryKB / 1024.0:N0} MB ({wasteRatio:F0}x overestimate). The unused memory is reserved and unavailable to other queries.";
// Note adaptive joins that chose Nested Loops at runtime — the grant
// was sized for a hash join that never happened.
diff --git a/PerformanceMonitor.PlanAnalysis/PlanModels.cs b/PerformanceMonitor.PlanAnalysis/PlanModels.cs
index b793e1cf1..d12d8659b 100644
--- a/PerformanceMonitor.PlanAnalysis/PlanModels.cs
+++ b/PerformanceMonitor.PlanAnalysis/PlanModels.cs
@@ -500,6 +500,13 @@ public class MemoryGrantInfo
public long RequestedMemoryKB { get; set; }
public long GrantedMemoryKB { get; set; }
public long MaxUsedMemoryKB { get; set; }
+ ///
+ /// True when the plan XML carried a MaxUsedMemory attribute. An actual plan does, and there 0
+ /// means the query used none of its grant. An estimated plan, or a plan with no runtime grant
+ /// info, does not, and there is 0 only because nothing was
+ /// reported. A rule that acts on "used nothing" must check this first.
+ ///
+ public bool HasMaxUsedMemory { get; set; }
public long GrantWaitTimeMs { get; set; }
public long LastRequestedMemoryKB { get; set; }
public string? IsMemoryGrantFeedbackAdjusted { get; set; }
diff --git a/PerformanceMonitor.PlanAnalysis/ShowPlanParser.cs b/PerformanceMonitor.PlanAnalysis/ShowPlanParser.cs
index cc5d63053..3ea1c52b6 100644
--- a/PerformanceMonitor.PlanAnalysis/ShowPlanParser.cs
+++ b/PerformanceMonitor.PlanAnalysis/ShowPlanParser.cs
@@ -648,6 +648,8 @@ private static void ParseQueryPlanElements(PlanStatement stmt, XElement stmtEl,
RequestedMemoryKB = ParseLong(memEl.Attribute("RequestedMemory")?.Value),
GrantedMemoryKB = ParseLong(memEl.Attribute("GrantedMemory")?.Value),
MaxUsedMemoryKB = ParseLong(memEl.Attribute("MaxUsedMemory")?.Value),
+ HasMaxUsedMemory = long.TryParse(memEl.Attribute("MaxUsedMemory")?.Value,
+ System.Globalization.NumberStyles.Integer, System.Globalization.CultureInfo.InvariantCulture, out _),
GrantWaitTimeMs = ParseLong(memEl.Attribute("GrantWaitTime")?.Value),
LastRequestedMemoryKB = ParseLong(memEl.Attribute("LastRequestedMemory")?.Value),
IsMemoryGrantFeedbackAdjusted = memEl.Attribute("IsMemoryGrantFeedbackAdjusted")?.Value
From 7ff1f560bd919f10e3bf807c51c36cff1be2a2f1 Mon Sep 17 00:00:00 2001
From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com>
Date: Mon, 28 Sep 2026 21:37:34 -0400
Subject: [PATCH 2/3] Rule 35: an exchange is no longer named the expensive
operator (#4690)
An exchange spends most of its elapsed time waiting on the operators that feed it and drain it,
so Rule 35 could name a Gather, Distribute or Repartition Streams while the operator next to it
did the work. The rule now skips exchanges; the rule number, warning type, 20% share and 1,000 ms
floor are unchanged. Follows PerformanceStudio#608.
---
.../OriginPlans/serially-parallel.sqlplan | Bin 0 -> 127312 bytes
.../Fixtures/OriginPlans/spill_plan.sqlplan | 572 ++++++++++++++++++
...nSync4690ExpensiveOperatorExchangeTests.cs | 122 ++++
.../PlanAnalyzer.cs | 14 +
4 files changed, 708 insertions(+)
create mode 100644 Darling/Darling.Tests/Fixtures/OriginPlans/serially-parallel.sqlplan
create mode 100644 Darling/Darling.Tests/Fixtures/OriginPlans/spill_plan.sqlplan
create mode 100644 Darling/Darling.Tests/PlanSync4690ExpensiveOperatorExchangeTests.cs
diff --git a/Darling/Darling.Tests/Fixtures/OriginPlans/serially-parallel.sqlplan b/Darling/Darling.Tests/Fixtures/OriginPlans/serially-parallel.sqlplan
new file mode 100644
index 0000000000000000000000000000000000000000..8a1f9d32dee2795d136eef0801c9dca23a45257f
GIT binary patch
literal 127312
zcmeHQZBraalJ3tJasPqvMQlhHYXk@cc-h^+2yCtOW`WM$a5xko5W)t88DuQ~_`7?a
z%1KpqRdr9#Oux-gLBI?%)m2%US(#b+QuTlTyOsSj`y;!^&a;K=_v|FQ&IZ|8_945>
z{+@l2EoY0_5?&YZ&1Lo$pC9Af%k0#A`!T!8-eq5+)g!d|XZDZmKePYHwzB=~JzCyo
zFVX8ETK||m%bubS8Rrrse1-P|^u;mWqnDfP3ZFf|&#n1c#NUS)`vc%P#iuKPVhQ6s
z81}UfSl$Ds3-nLu{GQKh>G?}D*8!m8EZ$-~j`crLEUf_ttLFQ!vyXW0d^kJehB#cr
zw=3pn5r6YAhgcXm^BdsV3-tIFxcC69eE`mH@ENIb0gMc?-!R(1KqVAZe68Z!rEJ}_
z5sF+5De}dTRtuQVR`xaKcVkA}M=!qtexde>)5>otwC1P!L%I`FH+cHO^l$^H!gkUV
z+g{*zA2YiE_0RFdLZpWsGs~;&$!B~Lci~9880#i`h1R!tM~prMp4YPtgL(32?5R<`
zz5rx|^C_SsZqH56d)aGz^3?RXgBf2BSH&T|KSTRZnLxvt5g)Rio&mOF(?^Vh9)VJE
z823SsqwD~qoZyG};7nz8a(*kA;rfvGLcAQ{nLmc@_W{>a{O+K)zhwV~&mU)h8@}7d
zJEi11!zDM_8^9~{T)=o`Pw116vu*Ufo9%%TFRzGW&g&=
z9Ph7YCcgnMI|gf(29JmHxsNtg=2JY@GtBQfX7;TattOb$iZ@@Lz
zpeLmV*X9kRr^xZDl-PF@;o1Qxvkh)o
zfRu^h|6ds82}UV<@-X@&DMgB~=S7V17C$Fu^_~EVZ3i1%^PKepcwy1Vl^7&DhJ*JF
ztO`}ET{+KIU2)21l$g(ftJ@)j&j9(4!1PrqFUR209;?vest}{>#s7}BQO=o6mY1&wzwahxx5977s9$E6%j)|D8vOtk!WHO79qvkqFF=*=jrDbn
z7T-cX4Iro4Y6-e}&uIA_(02ot@HYP6G_ME#t*y8&07;gwd;G5V+Nd4U#(#!eL2*@m_vM-DBIeXO-}GmA^~K>JQ-vV?JH
z&+p-P6}-BO-xb(vkMQ{(K3O(1d}QETMVqe;Y&&=*#E;N+4=wmjJ3pMC$caPfGg3wD
zuuEV`tSj2h+{K7a9hmhf78;*@iDxJ;+5QE3_yzCZ;=Nj*MV6mDcR1A8T<2%mpXf_1
zN|%qc5BB`gw4@zMy-S<)E#7Z~=V)h>sjp}R6u9x|Wbu8AcX!hse
zgD0>vREq8!n!9{LIVf$oE+6BE`u>}ec0ZuqE3}bzu^wOHJ6dAf;E)$+dl2+S>*YDP
zm)5C;?hPd16Y$nkK(X(9PRhA3b2iV=j^pg09c^3MLXh%P?4l3$}8t>MRb<_Z4p
z;+X@y+cTrS3ZQ&Yuv;P}wj9ZzYnxno0u3xXgBRErD#x6g^+s9`z&)JLhFPW0(Eh?`
z?@z$_HRw;Cjyw9b7LzFbUO
zes}_|cn63n)kW%1P7qr92F?tIkFg>wFA0^_j0`8d?sMc(`jkkQ@TpBavx!+M-;EyS
zAt*z=!Er|Qo#$!ooA6L_$DqkSVR7;U=*hjk%i$t(ZJR1RK0`l$eC9hJhr|N8U1le3
ztwrFsUIrfc1T`U78!_?}{d0Ds=PK==ptaktoy{IF#2HcJz0H2jw(9g3B-=xS(Is#uArE__lsyFm1IHc`jY=Da+(}<$
z{Mir^&g#_6?*#gyOoqE$k(a6NY&eHi0|jBBTo*|_2?h3nN7@?$!?7~!a+xC1NS~wr
zAtm18)k+oLmLkMI4dpdC?Zwa&O+OU(!u0+wgBm{@oO0)+zF&D#ybSW~!DkA~ch>Wu
zPs@y4$fV^P>~y!GsrO+k4e*+`EjP?3ln$GqI_>7YAw1&OI|Jo7Pivi#LLWctSGCV*
z&50F8FZxT+{u)mSHxmweiZ?(rTDjs^w`=+}o;)@qh=wIzxZC^Eync!I8<0=r+pqAu
zg1?j`^gfIBAH@N)OS_lZcc71@HSMkAM(DF<@JZf~Ri*N_j5faLl;
zq&4}8l45>oa|NqG)?U3Ug}BV?yfPZBfCl0EcnJJ69#hTAn%(tLF5A{-RJJif$Q{dk
zub*<-t;{g(Hs)UTmD6r_hH1wgd6jjy1fuh
zzV0BpvJWp1eM7V##Cybf+1BFGcm|u0_8I*G%g`8N-)pP9t@jZ6y$WtbpJ*TMm=;3D
zUFaizg}ejtB+(N^PxdRo{U+PSj_)b{6W`VoynBvMcQLO61B3Ri`&OL?V+8Iz^D|&n
zc^x)nH5yl$$37sVk8K~2GM@ApI||04U*R+Bg>xbDA?6Oi{XP#dvnzB?)odxnOdRid
z&Jh3MK42C5pL53?Ygct2N#Dac=zWD=x!*knmAQ{|{fG4N%cz6gUfp^JX&rMf!`+K;W2`+n
zUiM9~Zwbi}_GR=K$okc}=0|0_+Y0TDfhFI&p?MvILgpEVgcwlYqo0}b6!_2qa$s;5_bU26VD{vCvtMHO%pI=mR_$p1bK*OW&4t|Y0e#Xd
zDA_3jS$TF!6>0s#c#X)XH;@jpM<4y1$RDn>b3AzgYLlMt4Boi2=Kfkd)%rbgLP}l%
zqq2{vwk|nR9ixq-AE-Zxk8!cG1uW42petGZFW113^-@&DZoV%o)ySKJp&!HbUoT)>
z>r*GbW#ut5;|6evN7=3k^>x@>H2T-#&+VG^OkWv&L6YghmZFp05yI7~TAX}c-ZH+f
z^Uu(qs1lQ_QlOxPKpVjyLu8w_6MT%)1j20(YNF*9C`Df?vCnvoMu@6&L4A^)yLZNc
zmtl!)7HETcmhRh-_~Q3H1w9y#AiwxBU3l838u_e>Om}q#`8_sb=-(b!x)a7~93wMBtuFQLh?7cx+e2P*Poo8j`kX`?i1p`6Q?Z>yhKe?xA6*b?
zrb%^y(txkNhM$BEjB@8Sp-TtRQHc&N*2DL8&_VJbiq?bJ5{V8W?%hx7po7ThB09)A
zPIPdw9=@-G4x#~y=#cb7i1qN3I_Mxiw<0>E^$=n`{G<*#gu6*sPYacjTncgTeo_Y=
zLQBr2Ls}0mzKh=1L5Fg0()4y~*ng2PsY=U9eMw>8)9xD>S*UkMDEZ;)nGSsn)jb2`
zmo1a!$6P8_xEv$uMPC)G42o(=i@2=3xACCTG1~V4=*Th~@psbecx~b<^uE$}#Cj}7
z2>OM7Nx3S%|sW;mwtQeduAMx`F_mJW{$hqVt#wV*5_EG
zco^c5#(g6}Zp`}hJ3HB=Xz02?+sf
zP$r31>OA$;@7wCa>e0)XzuI5#vAsb&ub^l@Em*y4`#2XH`G{_4|K{Zv*B@)WiXz2+
zuEVJI`u(=4FevMpa2z2=!Oh#yr;DJGXU2s|p3=VKklFR>R)W}C9t7^XerGbtI*r9M>R+eAmua2yj1kcgl>6>F9&crgyC%w4
zgAYeqr+G;r+}q_U6*=M3i4@IS&(mvDk#=d7rd4Wl&XP7|rcu?Ly;wA~{&jjcz2>g;
zOIB~pemFAz9$MjjwtBnQrs9or`KbwQDzYGH)1*z4HkC>TiEUKZrm
zmr6T&A$;8si-Ee*FKJUBW3yxRcCSrk*XZ(7(x!|m@0!SRQMF52r7p+FdK=fPH9M1KjbV;u5Q1z&CH8^Tclt7Uf3?CWBhh?pZOuNPulw7i64aN%4EvkAWN3NyossG
z*&pItnlww#f3Ci>KX|75BsqJ^_glFfv+>RZ(LVgwZn9rpC}h=5a;k^hi~HWsajP?b
zPRH@bn8Y58<|U1E7tmM$W-r?IcY-FlIIU8Uuu^)%!8l>0!J
zSf-4n%iYXy=>uJ2nGu#QchhUs2fD;!;g&9UGlty6`#SV~EZSx1
zlGaOATDa}%pi5{4r8@II$T6{mT@S2Fm+9@~u>T@Exk}4P?c}iUSkA>X^k~kXsVlMQ
ztV6wz&GoRH!OL=r6D#ex%X)9#G`V}0seP4V-DpMzGc
z`{yYRyu;=LewaNH!}Yk8z`O-=+Lb9h>&AEL@h
zPs;wF4sQzgv~}!dL;Sh?GQA`T`!DixRB1U|UJjX2n;wqlV>#+e*@BPSdX=qDnLfoU
zlP`M#Z}y+Y=j{8be=r%f?{SCYB|OoZk52FOOZ+F<+&9P{e{c5G*26tnKD0utubRio
z9C$`C^10)6M1kx`ZhR>mi&_3WGx!kaEgixiFL%lRgp+gm+1u<_;PQ8TdJP!QfvH=>
zM|exu!{II7tT^GFguF$LIQ|J3JVLvljjTH`{qk-x`Oe#CDDkv>Adw?rCt|AHb1yoeyJK!lii}98SiK9^Muef
zXqye_J-zAFe`Xe=YK#!B`UkM9Xhy|)^SnqTo~_0CG+=pXMWWP*E_uKt|>c`5DRns5wDh4#lgSREqUd5ggf
zERlP#LFBtDyz;Yq$R1)$<~sW;Y@#aG)S_cM@x5y+@&6t4#U8Z|OzgH3SXvACbb!A<
zWwf8l+ujE}A*>ge&n0?(-$Q8c!FsrlpE|XV%MrpA+?CVHKQopkZ^K^5_RM`1Jl{$x
z@16tC7oY(xQpI7&3r)f~nuD8J(=NWYj0a;9T51?qn2(*ei8v_ZaFAmAI2+y*Sr9?|9*Et356ug+}X6k^24K
ztruykHhEjH`M`Jwx}jm9XjmE1@oy0u=
zeb&vcWS45>>!cfx;H{q4lbv*0w_z!EXQ{Nwd+pDX
zBi3_E<|wWz8>!A1sz@xxPDh<+#VRuLt{-i&u`$L57txA2ArkTN+s<=LjJuI4wPsS6
zF=;H!%?hZm){OL8#BaTEFy(R02xuz0q2mZI(+5>{Yvn9>8QRVXm_H1*ZHrP~Hy`yC
z4tMLms@!QxsqMl>k1TIG-Nes2-mRLG>zeV+`Z;uzl6Bp7>T+%Dy+1Z3!?-tDl%L`@
zeg0mUPg<0}SEc1_EMakO8
zPz#4ulx!#1>3-j|0SeF>lGR;TtxpsC?XarR<)`WG;jsTA|4fyZv*n+W8MRqo8FI)2
z)DY5VcYvHl<|nu1Kb+nA%Mgd+`)#(q&L5Tk&qu~*6E&DVwnk&hjcU>2pcJ}7p42k*
zC4ZaDDzLGQc6q2;&E{Q=OOxswh_*9+!+KP5K)hO$GP6o#ni`+I^d~mGhPf>Qs$~-c=FBQ~01>DE`&
z)Sg;tRtLROEPJ--m14=WMXz*b?QGF2^y1d*YYctkMZU%=Et~Z<&Vv3*vCP?8Un!P2
zTl5O0kSh{0*WLhp0Em$Md9>$e&q|lws`3rKEg2
z4ei4Ii!@u6mb0tb=t=K(FF$Tv>z(0lo}JmG_f{#ap_n_Qh4
zYRvX@93d*cHLUKFb#?w_uf@=`ixq`&bXA(Gp~>N0WI-sj{%iRy8Rd7NJ%cUxBkn3{
z?@qI;3U`{dweRpT$^W+^)~#B+?tV!yM{8U28@OX)b{zS`wW_<}X`Nd+cXSJz;Jf>~
zC=^%z>u9;qr)pKMEb^l-tGcQh^s!m=KXrHicz{rKd#6PizPh$26t@0hi4e7OUxDJF
z=(P-zwym!EJZ$1!o1eQ5$}$%Z;0e7lGBwOilsNL_9yQx{9ey)<&$_Uhy39`W*Im-q
zK1S;JpHi%QA1P1!QyOg_MFe1MjQT~{0_B>w9b
zBCfP37EFxw*6h?IKil^i@Fsx)qH(Nud5gci#W-y9gNf+AHzx(23^SX%_dMt<)kj9i
z7*XV%+1>dY%KaeaUNX6}z6<-xwA<>YiCfB(TcAIbkX9?BHaXt8#;CQqci-LdhOpFobW_DkeI
zyq+~2!QS&A`?R7KDhZjxax;{!-$3H9PMLC(v))8*!G@W6d_I!-a*H_=1Bb|jDS9$q
z=SF*!x9a`@P7}`&GiYLUZIU}a`QEW;Bp*AIWMHtqjZhx|19C&|4&Gg3X7q0ppUfiq
zG{govF^w-#zqchgt++?AVFgsuwe-u~3RD^%|5mbo`U>~WR2yc3p|EWIkM2B&?`Qz$;-KWZaw$BYiP
zU^a*BSJ?9FY#Sb%@3J58`U){dKCuTGDIS_*XFRrc{V;C3ee`>h{c6T8Zo};B%Sao>
zp=9sM?~k)@ja44=4Ke$SupJ^l+-CUSnoMqf-u&q{E;VBBi?hJmj*gB8U3We#7tXY|
zaIw+Ud&;?fWE1!^EQ74;^~1gRQT7XD3?(QtVZTQU|Wal|r7HNZR^b-Og2mRHs@I_x}ZK^-+ydRu3LuZd=bBEsH=>ha%bZUYiRk%
zv=wiK-z)c9eT*-JcMJKD7exqsFL>FlL^eVo`vB^PFuP2uSAFiJ9&-09{!Uk`HLWD)
zp2m(7&WCc2n)w{BmO
zt7AQdl3v$HvZ0viTO{ktRzQf;#QD`Yx=Jg_;$ijv0kQxu;8pG_QdfVb`yOJSuTq{n
zA_wFA^dq0W7>agc#)`4|$U|s5dxO}cWa&QVJn<1DhoiE~w-JXqGgq7|&)63{QiSyy%yv7Luo%
zu;!Ban{9E3$l+o*o&%QKmg`?EUcSTVya}jXt5I;Wi(M6S?YZ-1^{TDRbx&Y_3hmt0
zi8J(hD0Hr_?*d;|?z)iBxA+df*ZAeR^u%J9b$C@RrM_q}#j0>5tKIuVZ{Z-riPDPNZHCPW?jqjt<;^JnRPA}w9bb9aNY{`b7yN@Ie`8ZR_*W%
z#xty`GqbANmx~IIzK*t9U3oz4`ncr|r>%@yh1!3mt=2F65~4B=?uPP&%E}<6B&`)a
zTeFKbPpEut-7bEFT4Kw!Z?m|ghn8fU7F;b|_&@9adHw(~AlB9iG}r>P-vEEbKUCK42_z5eL0{wCt*5PPKG7%_uqLRvX)#iR
z*#B*P>3-|5QE!yfw>>^{by(l5gV9#OZ0rKUb=0M`Lh0HMD@EiyeHC&0y8Ttq&s9mu
zHR`NvD`&g0gZo8GLX(&G?eS7SFhMA~uwOW?^{8srQ?uCTnDh}feaa5Up&cw|A|0Fa
zM0p27Qx<6(^P=}fF~{n{-{GZ{e5HMO2mZu&+-)!8hdRqYb;F*jq9?I0q2(G*Oo_MG
zlU1}ePit8zIE969<@C)!s?&IuB;((Z{1Xdih)?wKc;NwRks%
zd(yf+-y}RoMXJvyJx67nv3)96J9~XO0PU41)k|8c%_T?x4Oe8-$;~!f7jvl6fe+prx!?1
z57E~n^zQ3f=D+3d@6$5^c#3--4p%>i&rN!=0mop}Cd5zvYy)i*
z#pW<_;LbBudb>F~lfSRSCmQ;{-E}74PP10R{zEP=^1_8JCtVLA6!~=!em@QWhrK0^
zpPb9}Nr|0zd8R@9ECcSUKH$#0kH{OKwsG&kc!BsEv-25+8}~-Pbk2cNzRGKHML2U?
zn~zTQ_fD~wxE1n^lMDRDgv|8d7;X&D?+(yd8ih74~%yfAsU-SHG{+ACuZbX^-I
z7H&LGdsH1H<@h1$c7H_${O%)P~3-?v_RI2Z~mQ94lwF=si(j2vYJIEci8!NCJf~M`@ZfY^htAX0k
zdM}_OFO9~0jqEde28#4xQOmz)_n>9z`=)>QerpzT)%we}t@d)7wj)DSruM~pn2()9
zd?wk#g))E7Zom|o$pZ9CMRjH)(5j(SA^lP_$h?+3n0ZdT3cSw@x^o
z^@VEusG0FN7Co~~)km`bHtHgqng{z;ZvO~h#mO)mP`n-W(4-02>3vkH_PusfqMUwZ
zA>q$gYtmL-M;+xlJVKI>;6HP0&VI^gOvEF-{*!L3TA~qt3!cvEj4X2+e(MHXtKPKf-s?nqbW_>;5;TMJVS1CJV+>DVfa=Yu`@KlpYeH3O4
zsr^`#8`A3Qb7^=Eq^lppnl6zu#TA|56YQE#qxTjo8D
zu@=+VIIOSa>6U!;ai4rLnl&@tYaO~Eq=v2pDS5246&d-(k{
zydL(JID9*;-74s|p0#!COYe{uO0gg%>*gIb9mqP8%jH89N
zLw&Pi|4r4Pv2RMqx@ykYUlTQEb-~Gg{5qa9R4iOf=ntM2E2O8jKqD}Wx-t7mZu$E;FtF&
zQWl*@XRa*;GZ$!ciD!H(ovmzS(c&-vInN{c%|Li*yeQI6MtBb%xcv
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
\ No newline at end of file
diff --git a/Darling/Darling.Tests/PlanSync4690ExpensiveOperatorExchangeTests.cs b/Darling/Darling.Tests/PlanSync4690ExpensiveOperatorExchangeTests.cs
new file mode 100644
index 000000000..adbe15482
--- /dev/null
+++ b/Darling/Darling.Tests/PlanSync4690ExpensiveOperatorExchangeTests.cs
@@ -0,0 +1,122 @@
+/*
+ * 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;
+using System.Collections.Generic;
+using System.IO;
+using System.Linq;
+using PerformanceMonitor.PlanAnalysis;
+using Xunit;
+
+namespace Darling.Tests;
+
+///
+/// #4690 (erikdarlingdata/PerformanceStudio#608) - Rule 35 (Expensive Operator) used to rank exchanges
+/// by their self-time. An exchange's elapsed time is mostly spent waiting on the operators that feed it
+/// and drain it, so it could be named the expensive operator when the operator next to it was the real
+/// problem. serially-parallel.sqlplan shows it plainly: the Sort took 17,111 ms and the
+/// Repartition Streams below it, which feeds the Sort, was given the same 17,111 ms as if it had done
+/// the same work. The three fixtures are copied verbatim from PerformanceStudio's public repo.
+///
+public sealed class PlanSync4690ExpensiveOperatorExchangeTests
+{
+ private static ParsedPlan LoadAndAnalyze(string fileName)
+ {
+ var xml = File.ReadAllText(Path.Combine(AppContext.BaseDirectory, "Fixtures", "OriginPlans", fileName));
+ var plan = ShowPlanParser.Parse(xml);
+ PlanAnalyzer.Analyze(plan);
+ BenefitScorer.Score(plan);
+ return plan;
+ }
+
+ private static IEnumerable Nodes(PlanNode node)
+ {
+ yield return node;
+ foreach (var child in node.Children)
+ foreach (var descendant in Nodes(child))
+ yield return descendant;
+ }
+
+ private static List AllNodes(ParsedPlan plan) =>
+ PlanStatements.EnumerateAll(plan)
+ .Where(s => s.RootNode != null)
+ .SelectMany(s => Nodes(s.RootNode!))
+ .ToList();
+
+ private static List ExchangesIn(ParsedPlan plan) =>
+ AllNodes(plan).Where(n => n.PhysicalOp == "Parallelism").ToList();
+
+ [Theory]
+ [InlineData("serially-parallel.sqlplan")]
+ [InlineData("memory_grant_wait_plan.sqlplan")]
+ [InlineData("spill_plan.sqlplan")]
+ public void Exchange_IsNeverNamedTheExpensiveOperator(string fixture)
+ {
+ var plan = LoadAndAnalyze(fixture);
+
+ var exchanges = ExchangesIn(plan);
+ Assert.NotEmpty(exchanges); // so the assertion below can't pass on a plan with no exchange
+ Assert.All(exchanges, e =>
+ Assert.DoesNotContain(e.Warnings, w => w.WarningType == "Expensive Operator"));
+ }
+
+ [Fact]
+ public void SeriallyParallelPlan_StillNamesTheSort()
+ {
+ // The Sort is the operator that did the work, and it keeps its Rule 35 warning. Only the
+ // exchange that mirrored its time drops out.
+ var plan = LoadAndAnalyze("serially-parallel.sqlplan");
+
+ var warning = Assert.Single(AllNodes(plan).SelectMany(n => n.Warnings), w => w.WarningType == "Expensive Operator");
+ Assert.StartsWith("Sort took", warning.Message);
+ }
+
+ // ---- built in code: the same timing on an exchange and on an ordinary operator ---------------
+
+ private static PlanNode Analyze(string physicalOp, string logicalOp)
+ {
+ var node = new PlanNode
+ {
+ NodeId = 1,
+ PhysicalOp = physicalOp,
+ LogicalOp = logicalOp,
+ HasActualStats = true,
+ ActualExecutions = 1,
+ ActualRows = 100,
+ EstimateRows = 100,
+ ActualElapsedMs = 6000
+ };
+ var stmt = new PlanStatement
+ {
+ RootNode = node,
+ QueryTimeStats = new QueryTimeInfo { ElapsedTimeMs = 10000, CpuTimeMs = 10000 }
+ };
+ PlanAnalyzer.Analyze(new ParsedPlan { Batches = [new PlanBatch { Statements = [stmt] }] });
+ return node;
+ }
+
+ [Fact]
+ public void ControlOperator_With60PercentOfTheStatement_IsFlagged()
+ {
+ var node = Analyze("Sort", "Sort");
+
+ var warning = Assert.Single(node.Warnings, w => w.WarningType == "Expensive Operator");
+ Assert.Equal(35, warning.RuleNumber);
+ }
+
+ [Theory]
+ [InlineData("Repartition Streams")]
+ [InlineData("Gather Streams")]
+ [InlineData("Distribute Streams")]
+ public void Exchange_With60PercentOfTheStatement_IsNotFlagged(string logicalOp)
+ {
+ var node = Analyze("Parallelism", logicalOp);
+
+ Assert.DoesNotContain(node.Warnings, w => w.WarningType == "Expensive Operator");
+ }
+}
diff --git a/PerformanceMonitor.PlanAnalysis/PlanAnalyzer.cs b/PerformanceMonitor.PlanAnalysis/PlanAnalyzer.cs
index 36533f795..d8f129541 100644
--- a/PerformanceMonitor.PlanAnalysis/PlanAnalyzer.cs
+++ b/PerformanceMonitor.PlanAnalysis/PlanAnalyzer.cs
@@ -1563,7 +1563,12 @@ _ when nonSargableReason.StartsWith("Function call", StringComparison.OrdinalIgn
// one or two operators always take most of the time just because there's almost
// nothing else to divide it among, so the share points at nothing. The benefit % is
// just the self-time share.
+ // Exchanges (Parallelism) are skipped: their self-time is mostly time spent waiting on the
+ // operators that feed them and drain them, not work of their own. On a live plan an exchange
+ // feeding a spilling sort showed 21 s of elapsed time on 2.4 s of CPU per thread, and was
+ // named as the expensive operator while the sort beside it was the real problem (#4690).
if (!cfg.IsRuleDisabled(35) && node.HasActualStats && node.Warnings.Count == 0
+ && !IsExchangeOperator(node)
&& stmt.QueryTimeStats != null && stmt.QueryTimeStats.ElapsedTimeMs >= 1000)
{
var selfMs = GetOperatorOwnElapsedMs(node);
@@ -1594,6 +1599,15 @@ _ when nonSargableReason.StartsWith("Function call", StringComparison.OrdinalIgn
}
}
+ ///
+ /// True for parallelism exchange operators (Gather/Distribute/Repartition Streams), whose
+ /// timings reflect time spent waiting on the operators around them rather than the operator's
+ /// own work.
+ ///
+ private static bool IsExchangeOperator(PlanNode node) =>
+ node.PhysicalOp == "Parallelism"
+ || node.LogicalOp is "Gather Streams" or "Distribute Streams" or "Repartition Streams";
+
///
/// Detects the NOT IN with nullable column pattern: statement has NOT IN,
/// and a nearby Nested Loops Anti Semi Join has an IS NULL residual predicate.
From a422f04ed9c38bc20c97d19db0d998258dc92b44 Mon Sep 17 00:00:00 2001
From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com>
Date: Mon, 28 Sep 2026 21:40:56 -0400
Subject: [PATCH 3/3] Rule 33: an Estimated Plan CE Guess names the predicate
that produced it, and follows the plan's CE version (#4687)
DetectCeGuess called 30% an equality guess and 10% and 1% inequality guesses, none of which
those predicates produce, never checked for the equality guess, and ignored the CE model
version. It now takes the statement's CardinalityEstimationModelVersion, labels each measured
guess for what produces it, detects the equality guess (rows^0.5 from CE 120, rows^0.75 under
CE 70, 1% tolerance), and reports a band only when the plan's estimator has it. The 100,000-row
minimum is the named constant CeGuessMinTableRows. Follows PerformanceStudio#608.
---
.../PlanSync4687CeGuessDetectionTests.cs | 249 ++++++++++++++++++
.../PlanAnalyzer.cs | 76 +++++-
2 files changed, 313 insertions(+), 12 deletions(-)
create mode 100644 Darling/Darling.Tests/PlanSync4687CeGuessDetectionTests.cs
diff --git a/Darling/Darling.Tests/PlanSync4687CeGuessDetectionTests.cs b/Darling/Darling.Tests/PlanSync4687CeGuessDetectionTests.cs
new file mode 100644
index 000000000..37bb00daa
--- /dev/null
+++ b/Darling/Darling.Tests/PlanSync4687CeGuessDetectionTests.cs
@@ -0,0 +1,249 @@
+/*
+ * 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.Collections.Generic;
+using System.Linq;
+using PerformanceMonitor.PlanAnalysis;
+using Xunit;
+
+namespace Darling.Tests;
+
+///
+/// #4687 (erikdarlingdata/PerformanceStudio#608) - Rule 33 names the default guess an estimate matches.
+/// The labels used to be wrong: 30% was called the equality guess (it is the inequality guess), and 10%
+/// and 1% were called inequality guesses that no predicate produces. The numbers below were measured on
+/// SQL Server 2025 against a 100,000-row heap with no statistics, using estimated plans
+/// (SET SHOWPLAN_XML), CE 70 through FORCE_LEGACY_CARDINALITY_ESTIMATION and CE 120 to 170 through the
+/// compatibility level.
+///
+/// Each row is what the optimizer wrote for EstimateRows on the scan, so every test feeds the rule a
+/// value the engine produced, not one worked out by hand. The code-built plans follow the pattern in
+/// PlanSync4535NodeRulesATests.
+///
+public sealed class PlanSync4687CeGuessDetectionTests
+{
+ private const double HundredThousand = 100_000;
+
+ ///
+ /// Runs the whole analyzer over one estimated table scan and returns the text of its
+ /// "Estimated Plan CE Guess" warning, or null when the rule said nothing.
+ ///
+ private static string? Label(int ceModelVersion, double estimateRows, double tableCardinality = HundredThousand)
+ {
+ var node = new PlanNode
+ {
+ NodeId = 0,
+ PhysicalOp = "Table Scan",
+ LogicalOp = "Table Scan",
+ EstimateRows = estimateRows,
+ TableCardinality = tableCardinality,
+ EstimatedTotalSubtreeCost = 1.0,
+ Predicate = "[t].[a]>(5)"
+ };
+ var stmt = new PlanStatement
+ {
+ RootNode = node,
+ StatementSubTreeCost = 1.0,
+ CardinalityEstimationModelVersion = ceModelVersion
+ };
+ var plan = new ParsedPlan { Batches = [new PlanBatch { Statements = [stmt] }] };
+ PlanAnalyzer.Analyze(plan);
+
+ return node.Warnings.FirstOrDefault(w => w.WarningType == "Estimated Plan CE Guess")?.Message;
+ }
+
+ // ---- what each measured guess is called ----------------------------------------------------
+
+ [Theory]
+ // a > 5, a < 5, a >= 5: 30% in every estimator
+ [InlineData(170, 30000, "the 30% guess for an inequality such as > or <")]
+ [InlineData(70, 30000, "the 30% guess for an inequality such as > or <")]
+ [InlineData(0, 30000, "the 30% guess for an inequality such as > or <")]
+ // a BETWEEN 5 AND 10, a >= 5 AND a <= 10, and LIKE (CE 120+): 9%
+ [InlineData(170, 9000, "the 9% guess for BETWEEN or a two-sided range on a column with known values, or for LIKE (9.0%)")]
+ [InlineData(120, 9000, "the 9% guess for BETWEEN or a two-sided range on a column with known values, or for LIKE (9.0%)")]
+ // a > 5 AND b > 5 and BETWEEN on variables under CE 70: 30% * 30% = 9%
+ [InlineData(70, 9000, "the 9% guess for BETWEEN, a two-sided range, or two inequalities on different columns (9.0%)")]
+ // no estimator named: only what both estimators agree on
+ [InlineData(0, 9000, "the 9% guess for BETWEEN or a two-sided range (9.0%)")]
+ // a > 5 AND b > 5, and BETWEEN on variables, under CE 120+: 30% * sqrt(30%) = 16.43%
+ [InlineData(170, 16431.7, "the 16.4% guess for two inequalities on different columns, or for a BETWEEN or range on variables or on an expression")]
+ [InlineData(120, 16431.7, "the 16.4% guess for two inequalities on different columns")]
+ [InlineData(0, 16431.7, "the 16.4% guess for two inequalities on different columns")]
+ // a = b, and from CE 130 an equality on an expression such as ABS(a) = 5: 10%
+ [InlineData(170, 10000, "the 10% guess for comparing one column with another, or for an equality on an expression such as a function of a column")]
+ [InlineData(130, 10000, "the 10% guess for comparing one column with another, or for an equality on an expression")]
+ [InlineData(0, 10000, "the 10% guess for comparing one column with another, or for an equality on an expression")]
+ [InlineData(120, 10000, "the 10% guess for comparing one column with another (10.0%)")]
+ [InlineData(70, 10000, "the 10% guess for comparing one column with another (10.0%)")]
+ // a = b AND c = d under CE 70: 10% * 10% = 1%
+ [InlineData(70, 1000, "the 1% guess that CE 70 gets from multiplying two 10% guesses")]
+ [InlineData(0, 1000, "the 1% guess that CE 70 gets from multiplying two 10% guesses")]
+ // a = 5 and a IS NULL with no statistics: CE 120+ rows^0.5 (316.228), CE 70 rows^0.75 (5623.41)
+ [InlineData(170, 316.228, "the square root of the row count (0.3%)")]
+ [InlineData(120, 316.228, "the square root of the row count (0.3%)")]
+ [InlineData(0, 316.228, "the square root of the row count (0.3%)")]
+ [InlineData(70, 5623.41, "the row count to the power 0.75 (5.6%)")]
+ [InlineData(0, 5623.41, "the row count to the power 0.75 (5.6%)")]
+ public void MeasuredGuess_IsNamedForWhatProducedIt(int ceModelVersion, double estimateRows, string expected)
+ {
+ var label = Label(ceModelVersion, estimateRows);
+
+ Assert.NotNull(label);
+ Assert.Contains(expected, label);
+ }
+
+ [Fact]
+ public void ThirtyPercent_IsTheInequalityGuess_NotTheEqualityGuess()
+ {
+ var label = Label(170, 30000);
+
+ Assert.NotNull(label);
+ Assert.DoesNotContain("30% equality", label);
+ Assert.Contains("inequality", label);
+ }
+
+ [Fact]
+ public void EqualityGuess_MessageSaysItIsAnEqualityGuess()
+ {
+ var label = Label(170, 316.228);
+
+ Assert.NotNull(label);
+ Assert.Contains("equality guess", label);
+ }
+
+ // ---- estimates that are not a guess in that estimator ----------------------------------------
+
+ [Theory]
+ [InlineData(70, 16431.7)] // CE 70 has no 16.43% guess: it gets 9% for the same predicate
+ [InlineData(170, 1000)] // CE 120+ has no 1% guess: a = b AND c = d is 3.16% there
+ [InlineData(120, 1000)]
+ [InlineData(170, 5623.41)] // rows^0.75 is CE 70's equality guess only
+ [InlineData(70, 316.228)] // the square root is CE 120+'s equality guess only
+ [InlineData(170, 25000)] // ordinary estimates
+ [InlineData(170, 50000)]
+ [InlineData(70, 12345)]
+ [InlineData(170, 322.6)] // 2% above the equality guess, outside its 1% tolerance
+ [InlineData(170, 309.9)] // 2% below
+ public void NotAGuess_GetsNoLabel(int ceModelVersion, double estimateRows)
+ {
+ Assert.Null(Label(ceModelVersion, estimateRows));
+ }
+
+ [Fact]
+ public void EqualityGuess_ToleratesHalfAPercent()
+ {
+ Assert.NotNull(Label(170, 316.228 * 1.005));
+ Assert.NotNull(Label(170, 316.228 * 0.995));
+ }
+
+ // ---- the equality guess follows the table's row count ----------------------------------------
+
+ [Theory]
+ [InlineData(400_000, 170, 632.456)] // measured: sqrt(400,000)
+ [InlineData(400_000, 70, 15905.4)] // measured: 400,000^0.75
+ [InlineData(1_000_000, 170, 1000)]
+ [InlineData(1_000_000, 70, 31622.8)]
+ // 100,000,000^0.75 is 1,000,000, exactly 1% of the table. It is still the equality guess.
+ [InlineData(100_000_000, 70, 1_000_000)]
+ public void EqualityGuess_ScalesWithTheTable(double tableRows, int ceModelVersion, double estimateRows)
+ {
+ var label = Label(ceModelVersion, estimateRows, tableRows);
+
+ Assert.NotNull(label);
+ Assert.Contains("equality guess", label);
+ Assert.DoesNotContain("1% guess", label);
+ }
+
+ [Fact]
+ public void EqualityGuess_IsNotLookedForBelowTheTableSizeFloor()
+ {
+ // The rule only looks at tables of 100,000 rows or more. The same estimate on a table just
+ // under the floor gets no label: there the equality guess could sit on a fixed guess.
+ Assert.Null(Label(170, System.Math.Sqrt(99_999), 99_999));
+ }
+
+ // ---- the estimator version reaches the rule from the plan XML --------------------------------
+
+ private const string PlanTemplate = """
+
+ """;
+
+ private static IEnumerable Nodes(PlanNode node)
+ {
+ yield return node;
+ foreach (var child in node.Children)
+ foreach (var descendant in Nodes(child))
+ yield return descendant;
+ }
+
+ ///
+ /// The shape of a real estimated plan for one of the probe queries, with the estimate, the
+ /// estimator version and the predicate filled in from what SQL Server 2025 produced.
+ ///
+ private static string? LabelFromXml(string sql, string predicate, int ce, string estimateRows)
+ {
+ var xml = PlanTemplate
+ .Replace("{SQL}", sql)
+ .Replace("{PREDICATE}", predicate)
+ .Replace("{CE}", ce.ToString())
+ .Replace("{ROWS}", estimateRows);
+
+ var plan = ShowPlanParser.Parse(xml);
+ PlanAnalyzer.Analyze(plan);
+
+ return PlanStatements.EnumerateAll(plan)
+ .Where(s => s.RootNode != null)
+ .SelectMany(s => Nodes(s.RootNode!))
+ .SelectMany(n => n.Warnings)
+ .FirstOrDefault(w => w.WarningType == "Estimated Plan CE Guess")?.Message;
+ }
+
+ private const string TwoInequalitiesSql = "a > 5 AND b > 5";
+ private const string TwoInequalitiesPredicate = "[t].[a]>(5) AND [t].[b]>(5)";
+
+ [Fact]
+ public void TwoInequalitiesOnDifferentColumns_Ce170_IsSixteenPointFourPercent()
+ {
+ // WHERE a > 5 AND b > 5 under CE 170: EstimateRows="16431.7"
+ var label = LabelFromXml(TwoInequalitiesSql, TwoInequalitiesPredicate, 170, "16431.7");
+
+ Assert.NotNull(label);
+ Assert.Contains("the 16.4% guess for two inequalities on different columns", label);
+ }
+
+ [Fact]
+ public void TwoInequalitiesOnDifferentColumns_Ce70_IsNinePercent()
+ {
+ // The same query under CE 70: EstimateRows="9000", and the label says why.
+ var label = LabelFromXml(TwoInequalitiesSql, TwoInequalitiesPredicate, 70, "9000");
+
+ Assert.NotNull(label);
+ Assert.Contains("the 9% guess for BETWEEN, a two-sided range, or two inequalities on different columns", label);
+ }
+
+ [Fact]
+ public void Equality_Ce170_IsTheSquareRootOfTheRowCount()
+ {
+ // WHERE a = 5 under CE 170: EstimateRows="316.228"
+ var label = LabelFromXml("a = 5", "[t].[a]=(5)", 170, "316.228");
+
+ Assert.NotNull(label);
+ Assert.Contains("the square root of the row count", label);
+ }
+
+ [Fact]
+ public void Equality_Ce70_IsTheRowCountToTheThreeQuarters()
+ {
+ // WHERE a = 5 under CE 70: EstimateRows="5623.41"
+ var label = LabelFromXml("a = 5", "[t].[a]=(5)", 70, "5623.41");
+
+ Assert.NotNull(label);
+ Assert.Contains("the row count to the power 0.75", label);
+ }
+}
diff --git a/PerformanceMonitor.PlanAnalysis/PlanAnalyzer.cs b/PerformanceMonitor.PlanAnalysis/PlanAnalyzer.cs
index d8f129541..258d941d8 100644
--- a/PerformanceMonitor.PlanAnalysis/PlanAnalyzer.cs
+++ b/PerformanceMonitor.PlanAnalysis/PlanAnalyzer.cs
@@ -1221,17 +1221,20 @@ _ when nonSargableReason.StartsWith("Function call", StringComparison.OrdinalIgn
}
// Rule 33: Estimated plan CE guess detection — scans with telltale default selectivity
- // When the optimizer uses a local variable or can't sniff, it falls back to density-based
- // guesses: 30% (equality), 10% (inequality), 9% (LIKE/between), ~16.43% (sqrt(30%)),
- // 1% (multi-inequality). On large tables, these guesses can hide the need for an index.
+ // When the optimizer has no statistics to use (a local variable it can't sniff, a column with
+ // no statistics, an expression), it falls back on fixed guesses: 30% for an inequality, 9%
+ // for BETWEEN or LIKE, ~16.4% for two inequalities, 10% for comparing two columns, and an
+ // equality guess that grows with the table. DetectCeGuess has the measured details and
+ // which estimator (CE 70 or 120+) each one belongs to. On large tables, these guesses can
+ // hide the need for an index (#4687).
if (!cfg.IsRuleDisabled(33) && !node.HasActualStats && IsRowstoreScan(node)
- && node.TableCardinality >= 100_000 && node.EstimateRows > 0
+ && node.TableCardinality >= CeGuessMinTableRows && node.EstimateRows > 0
&& !string.IsNullOrEmpty(node.Predicate))
{
var impact = BuildScanImpactDetails(node, stmt);
if (impact.CostPct >= 50)
{
- var guessDesc = DetectCeGuess(node.EstimateRows, node.TableCardinality);
+ var guessDesc = DetectCeGuess(node.EstimateRows, node.TableCardinality, stmt.CardinalityEstimationModelVersion);
if (guessDesc != null)
{
node.Warnings.Add(new PlanWarning
@@ -3009,23 +3012,72 @@ private static string StripBrackets(string bracketPart) =>
return null;
}
+ // Rule 33 only looks at tables with at least this many rows, and DetectCeGuess relies on that:
+ // the equality guess is a power of the row count, so on a small table it lands on the fixed
+ // guesses below, and from this size up it stays clear of them (0.3% and 5.6% at this size,
+ // falling as the table grows).
+ private const double CeGuessMinTableRows = 100_000;
+
///
/// Detects well-known CE default selectivity guesses by comparing EstimateRows to TableCardinality.
/// Returns a description of the guess pattern, or null if no known pattern matches.
+ ///
+ /// Where each guess comes from was measured on SQL Server 2025: a 100,000-row heap with no
+ /// statistics, estimated plans through SET SHOWPLAN_XML, CE 70 through
+ /// FORCE_LEGACY_CARDINALITY_ESTIMATION and CE 120 to 170 through the compatibility level. The
+ /// versions from 120 to 170 agree except where a row says otherwise.
+ /// equality, a = 5 or a IS NULL CE 120+: rows^0.5 CE 70: rows^0.75
+ /// inequality, a > 5 30%, every CE
+ /// BETWEEN or a two-sided range CE 70: 9% CE 120+: 9% on a column with known values
+ /// LIKE CE 120+: 9% CE 70: not a fixed guess
+ /// two inequalities, two columns CE 120+: 16.43% CE 70: 9%
+ /// range on variables, or on an expression, ABS(a) BETWEEN 5 AND 10 CE 120+: 16.43% CE 70: 9%
+ /// one column compared with another 10%, every CE
+ /// equality on an expression, ABS(a) = 5 CE 130+: 10% CE 120: rows^0.5 CE 70: rows^0.75
+ /// two 10% guesses, a = b AND c = d CE 120+: 3.16% CE 70: 1%
+ /// The 16.43% is 30% times the square root of 30%, how CE 120+ combines two 30% guesses.
///
- private static string? DetectCeGuess(double estimateRows, double tableCardinality)
+ /// The scan's estimated row count.
+ /// The table's row count.
+ ///
+ /// The statement's CardinalityEstimationModelVersion: 70 is the legacy estimator, 120 and later
+ /// the current one, and 0 means the plan did not say, so both stay possible.
+ ///
+ private static string? DetectCeGuess(double estimateRows, double tableCardinality, int ceModelVersion = 0)
{
if (tableCardinality <= 0) return null;
var selectivity = estimateRows / tableCardinality;
+ var pct = $"{selectivity * 100:N1}%";
+ var legacy = ceModelVersion == 70;
+ var current = ceModelVersion >= 120;
+
+ // Equality is not a fixed share of the table, so it is checked on its own, to 1%. The two
+ // estimators use different powers of the row count, so a plan that names its estimator only
+ // gets the one that estimator uses.
+ static bool Near(double rows, double guess) => Math.Abs(rows - guess) <= guess * 0.01;
+
+ if (!legacy && Near(estimateRows, Math.Sqrt(tableCardinality)))
+ return $"matches the equality guess (an equality or IS NULL with no statistics to use), the square root of the row count ({pct})";
+ if (!current && Near(estimateRows, Math.Pow(tableCardinality, 0.75)))
+ return $"matches the equality guess (an equality or IS NULL with no statistics to use), the row count to the power 0.75 ({pct})";
- // Known CE guess selectivities with a 2% tolerance band
+ // The fixed guesses, with a 2% tolerance band
return selectivity switch
{
- >= 0.29 and <= 0.31 => $"matches the 30% equality guess ({selectivity * 100:N1}%)",
- >= 0.098 and <= 0.102 => $"matches the 10% inequality guess ({selectivity * 100:N1}%)",
- >= 0.088 and <= 0.092 => $"matches the 9% LIKE/BETWEEN guess ({selectivity * 100:N1}%)",
- >= 0.155 and <= 0.175 => $"matches the ~16.4% compound predicate guess ({selectivity * 100:N1}%)",
- >= 0.009 and <= 0.011 => $"matches the 1% multi-inequality guess ({selectivity * 100:N1}%)",
+ >= 0.29 and <= 0.31 =>
+ $"matches the 30% guess for an inequality such as > or < ({pct})",
+ >= 0.088 and <= 0.092 =>
+ current ? $"matches the 9% guess for BETWEEN or a two-sided range on a column with known values, or for LIKE ({pct})"
+ : legacy ? $"matches the 9% guess for BETWEEN, a two-sided range, or two inequalities on different columns ({pct})"
+ : $"matches the 9% guess for BETWEEN or a two-sided range ({pct})",
+ >= 0.098 and <= 0.102 =>
+ ceModelVersion is 0 or >= 130
+ ? $"matches the 10% guess for comparing one column with another, or for an equality on an expression such as a function of a column ({pct})"
+ : $"matches the 10% guess for comparing one column with another ({pct})",
+ >= 0.155 and <= 0.175 when !legacy =>
+ $"matches the 16.4% guess for two inequalities on different columns, or for a BETWEEN or range on variables or on an expression ({pct})",
+ >= 0.009 and <= 0.011 when !current =>
+ $"matches the 1% guess that CE 70 gets from multiplying two 10% guesses, as when two predicates each compare one column with another ({pct})",
_ => null
};
}