Skip to content

Join OR Clause: skip the dynamic seek for a list of parameters (#558) #559

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
erikdarlingdata merged 1 commit into from
Sep 24, 2026
Merged

Join OR Clause: skip the dynamic seek for a list of parameters (#558) #559

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions src/PlanViewer.Core/Services/PlanAnalyzer.Detection.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,54 @@ private static bool IsOrExpansionChain(PlanNode concatenationNode)
return true;
}

/// <summary>
/// True when a lookup branch under an OR expansion's Concatenation builds its seek value from
/// another input. A join OR does: in ON u.Id = p.OwnerUserId OR u.Id = p.LastEditorUserId the
/// branches produce [Posts].[OwnerUserId] and [Posts].[LastEditorUserId], once per outer row.
/// The dynamic seek for an IN list of parameters (#558) has the same operator shape, but its
/// branches produce only parameters and literals ([@p1], (62)), which no outer row changes.
/// </summary>
private static bool LookupReadsAnotherInput(PlanNode branch)
{
var values = branch.PhysicalOp == "Constant Scan"
? branch.ConstantScanValues
: branch.DefinedValues;

// Nothing to read, so nothing proves a parameter list: keep the warning.
if (string.IsNullOrEmpty(values))
return true;

return ReadsAnotherInput(values);
}

/// <summary>
/// True when a ScalarString names anything other than a parameter or a variable: a column
/// ([db].[dbo].[T].[c], or @tv.[c] as [v].[c] on a table variable) or an expression column
/// ([Expr1003]). Function names ([dbo].[fn](...)) and string literals are skipped. An
/// expression column counts too: an OR join on o.X + 1 renders its branches as [Expr1002],
/// computed on the outer input. The Constant Scan under a lookup branch is normally empty,
/// so the branch has no expression of its own to name, and a name that cannot be proved to
/// be a parameter keeps the warning, as the shape check alone did. Internal so the shapes
/// can be tested as raw strings.
/// </summary>
internal static bool ReadsAnotherInput(string scalarString)
{
foreach (Match match in BracketedNameRegex.Matches(scalarString))
{
if (!match.Groups["name"].Success || match.Groups["call"].Success)
continue; // a string literal, or the name of a function

var name = match.Groups["name"].Value;
if (name.StartsWith("[@", StringComparison.Ordinal) &&
!name.Contains("].[", StringComparison.Ordinal))
continue; // a parameter or a variable: [@p1]

return true;
}

return false;
}

/// <summary>
/// Finds Sort and Hash Match operators in the tree that consume memory.
/// </summary>
Expand Down
15 changes: 10 additions & 5 deletions src/PlanViewer.Core/Services/PlanAnalyzer.Node.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -642,16 +642,21 @@ private static void Rule15_JoinOrClause(PlanNode node, PlanStatement stmt, Analy
if (!cfg.IsRuleDisabled(15) && node.PhysicalOp == "Concatenation")
{
var constantScanBranches = node.Children
.Count(c => c.PhysicalOp == "Constant Scan" ||
.Where(c => c.PhysicalOp == "Constant Scan" ||
(c.PhysicalOp == "Compute Scalar" &&
c.Children.Any(gc => gc.PhysicalOp == "Constant Scan")));

if (constantScanBranches >= 2 && IsOrExpansionChain(node))
c.Children.Any(gc => gc.PhysicalOp == "Constant Scan")))
.ToList();

/* #558: WHERE t.A IN (@p1, @p2) builds the same operator chain, as a dynamic seek over
the parameter values, and there is no join to rewrite. Only a lookup that takes its
value from another input's row makes the OR a join OR. */
if (constantScanBranches.Count >= 2 && IsOrExpansionChain(node) &&
constantScanBranches.Any(LookupReadsAnotherInput))
{
node.Warnings.Add(new PlanWarning
{
WarningType = "Join OR Clause",
Message = $"OR in a join predicate. SQL Server rewrote the OR as {constantScanBranches} separate lookups, each evaluated independently — this multiplies the work on the inner side. Rewrite as separate queries joined with UNION ALL. For example, change \"FROM a JOIN b ON a.x = b.x OR a.y = b.y\" to \"FROM a JOIN b ON a.x = b.x UNION ALL FROM a JOIN b ON a.y = b.y\".",
Message = $"OR in a join predicate. SQL Server rewrote the OR as {constantScanBranches.Count} separate lookups, each evaluated independently — this multiplies the work on the inner side. Rewrite as separate queries joined with UNION ALL. For example, change \"FROM a JOIN b ON a.x = b.x OR a.y = b.y\" to \"FROM a JOIN b ON a.x = b.x UNION ALL FROM a JOIN b ON a.y = b.y\".",
Severity = PlanWarningSeverity.Warning
});
}
Expand Down
8 changes: 8 additions & 0 deletions src/PlanViewer.Core/Services/PlanAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,14 @@ Groups[1] match is a real operator. */
@"\b(isnull|coalesce)\s*\(",
RegexOptions.IgnoreCase | RegexOptions.Compiled);

/* A name in a ScalarString: one bracketed part or a dotted chain of them ([@p1], [Expr1003],
[db].[dbo].[T].[c]). A name followed by ( is a function call. String literals are matched
first, so a bracket inside one ('[x]') is never read as a name. Only a match with the
name group is a name. */
private static readonly Regex BracketedNameRegex = new(
@"'(?:[^']|'')*'|(?<name>\[(?:[^\]]|\]\])*\](?:\.\[(?:[^\]]|\]\])*\])*)(?<call>\s*\()?",
RegexOptions.Compiled);

public static void Analyze(ParsedPlan plan, AnalyzerConfig? config = null, ServerMetadata? serverMetadata = null) =>
AnalyzeCancellable(plan, config, serverMetadata, CancellationToken.None);

Expand Down
130 changes: 116 additions & 14 deletions tests/PlanViewer.Core.Tests/ComparisonBaseline.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,22 @@ SELECT * FROM dbo.Users WHERE DisplayName = @d
Estimated rows: 1,000 -> 1,000 (0.0% more)


##### in_list_dynamic_seek_plan.sqlplan vs in_list_dynamic_seek_plan.sqlplan
=== Plan Comparison ===
Plan A: in_list_dynamic_seek_plan.sqlplan
Plan B: in_list_dynamic_seek_plan.sqlplan

--- Statement 1 ---
(10 int, 20 int)SELECT t.Id, t.A FROM dbo.T AS t WHERE t.A IN (10, 20)

Estimated cost: 0.0033 -> 0.0033 (0.0% costlier)
Estimated rows: 2 -> 2 (0.0% more)
Runtime: 7ms -> 7ms (0.0% slower)
CPU time: 6ms -> 6ms (0.0% slower)
Memory grant: 1.0 MB -> 1.0 MB (0.0% more)
DOP: 1 -> 1


##### isnull_plan.sqlplan vs isnull_plan.sqlplan
=== Plan Comparison ===
Plan A: isnull_plan.sqlplan
Expand Down Expand Up @@ -374,6 +390,40 @@ SELECT u.Id, MaxScore = MAX(p.Score) FROM dbo.Users AS u JOIN dbo.
- HTDELETE 33ms


##### join_or_expression_plan.sqlplan vs join_or_expression_plan.sqlplan
=== Plan Comparison ===
Plan A: join_or_expression_plan.sqlplan
Plan B: join_or_expression_plan.sqlplan

--- Statement 1 ---
SELECT o.Id, t.Id FROM dbo.O AS o JOIN dbo.T AS t ON t.A = o.X + 1 OR t.A = o.Y + 1

Estimated cost: 0.0428 -> 0.0428 (0.0% costlier)
Estimated rows: 300 -> 300 (0.0% more)
Runtime: 3ms -> 3ms (0.0% slower)
CPU time: 3ms -> 3ms (0.0% slower)
Logical reads: 804 -> 804 (0.0% more)
Memory grant: 1.0 MB -> 1.0 MB (0.0% more)
DOP: 1 -> 1


##### join_or_mixed_parameter_plan.sqlplan vs join_or_mixed_parameter_plan.sqlplan
=== Plan Comparison ===
Plan A: join_or_mixed_parameter_plan.sqlplan
Plan B: join_or_mixed_parameter_plan.sqlplan

--- Statement 1 ---
SELECT o.Id, t.Id FROM dbo.O AS o JOIN dbo.T AS t ON t.A = o.X OR t.A = 5

Estimated cost: 0.0464 -> 0.0464 (0.0% costlier)
Estimated rows: 1,000 -> 1,000 (0.0% more)
Runtime: 3ms -> 3ms (0.0% slower)
CPU time: 3ms -> 3ms (0.0% slower)
Logical reads: 800 -> 800 (0.0% more)
Memory grant: 1.0 MB -> 1.0 MB (0.0% more)
DOP: 1 -> 1


##### key_lookup_plan.sqlplan vs key_lookup_plan.sqlplan
=== Plan Comparison ===
Plan A: key_lookup_plan.sqlplan
Expand Down Expand Up @@ -1426,22 +1476,40 @@ select COUNT(*) from [TimeCard].[Cards] as [t] where exists (select 1 from [Time
Logical reads: 9 -> 0 (eliminated)


##### implicit_convert_seek_plan.sqlplan vs isnull_plan.sqlplan
##### implicit_convert_seek_plan.sqlplan vs in_list_dynamic_seek_plan.sqlplan
=== Plan Comparison ===
Plan A: implicit_convert_seek_plan.sqlplan
Plan B: isnull_plan.sqlplan
Plan B: in_list_dynamic_seek_plan.sqlplan

Note: Plan A is an estimated plan. Runtime metrics only available for the actual plan.

--- Statement 1 ---
SELECT * FROM dbo.Users WHERE DisplayName = @d

Estimated cost: 5 -> 3,119.42 (9,999% costlier)
Estimated rows: 1,000 -> 1 (99.9% fewer)
Runtime: N/A -> 6.6s
CPU time: N/A -> 5.7s
Estimated cost: 5 -> 0.0033 (99.9% cheaper)
Estimated rows: 1,000 -> 2 (99.8% fewer)
Runtime: N/A -> 7ms
CPU time: N/A -> 6ms
Memory grant: 0.0 MB -> 1.0 MB (new)
DOP: 0 -> 1


##### in_list_dynamic_seek_plan.sqlplan vs isnull_plan.sqlplan
=== Plan Comparison ===
Plan A: in_list_dynamic_seek_plan.sqlplan
Plan B: isnull_plan.sqlplan

--- Statement 1 ---
(10 int, 20 int)SELECT t.Id, t.A FROM dbo.T AS t WHERE t.A IN (10, 20)

Estimated cost: 0.0033 -> 3,119.42 (9,999% costlier)
Estimated rows: 2 -> 1 (50.0% fewer)
Runtime: 7ms -> 6.6s (9,999% slower)
CPU time: 6ms -> 5.7s (9,999% slower)
Logical reads: 0 -> 4,181,158 (new)
Physical reads: 0 -> 404 (new)
Memory grant: 1.0 MB -> 0.0 MB (eliminated)
DOP: 1 -> 0
Warnings: 0 -> 4 (4 new)

Wait stats:
Expand Down Expand Up @@ -1486,21 +1554,21 @@ SELECT COUNT(*) FROM dbo.Posts AS p WHERE ISNULL(p.LastEditorDisplayName, '') =
- HTDELETE 33ms


##### join_or_clause_plan.sqlplan vs key_lookup_plan.sqlplan
##### join_or_clause_plan.sqlplan vs join_or_expression_plan.sqlplan
=== Plan Comparison ===
Plan A: join_or_clause_plan.sqlplan
Plan B: key_lookup_plan.sqlplan
Plan B: join_or_expression_plan.sqlplan

--- Statement 1 ---
SELECT u.Id, MaxScore = MAX(p.Score) FROM dbo.Users AS u JOIN dbo.Posts AS p ON u.Id = p.OwnerUserId OR u.Id = p.LastEditorUserId WHERE p.PostTypeId IN (1, 2) GROUP BY u.Id HAVING MAX(p.Score) >= 5000 ORDER BY MaxScore DESC

Estimated cost: 3,030.74 -> 0.341 (99.9% cheaper)
Estimated rows: 135 -> 1 (99.3% fewer)
Runtime: 33.0s -> 0ms (eliminated)
CPU time: 3m 48s -> 0ms (eliminated)
Logical reads: 93,307,943 -> 322 (99.9% fewer)
Estimated cost: 3,030.74 -> 0.0428 (99.9% cheaper)
Estimated rows: 135 -> 300 (122% more)
Runtime: 33.0s -> 3ms (99.9% faster)
CPU time: 3m 48s -> 3ms (99.9% faster)
Logical reads: 93,307,943 -> 804 (99.9% fewer)
Physical reads: 107,624 -> 0 (eliminated)
Memory grant: 451.8 MB -> 0.0 MB (eliminated)
Memory grant: 451.8 MB -> 1.0 MB (99.8% less)
DOP: 8 -> 1
Warnings: 8 -> 0 (8 resolved)

Expand All @@ -1516,6 +1584,40 @@ SELECT u.Id, MaxScore = MAX(p.Score) FROM dbo.Users AS u JOIN dbo.
- HTDELETE 33ms


##### join_or_expression_plan.sqlplan vs join_or_mixed_parameter_plan.sqlplan
=== Plan Comparison ===
Plan A: join_or_expression_plan.sqlplan
Plan B: join_or_mixed_parameter_plan.sqlplan

--- Statement 1 ---
SELECT o.Id, t.Id FROM dbo.O AS o JOIN dbo.T AS t ON t.A = o.X + 1 OR t.A = o.Y + 1

Estimated cost: 0.0428 -> 0.0464 (8.6% costlier)
Estimated rows: 300 -> 1,000 (233% more)
Runtime: 3ms -> 3ms (0.0% slower)
CPU time: 3ms -> 3ms (0.0% slower)
Logical reads: 804 -> 800 (0.5% fewer)
Memory grant: 1.0 MB -> 1.0 MB (0.0% more)
DOP: 1 -> 1


##### join_or_mixed_parameter_plan.sqlplan vs key_lookup_plan.sqlplan
=== Plan Comparison ===
Plan A: join_or_mixed_parameter_plan.sqlplan
Plan B: key_lookup_plan.sqlplan

--- Statement 1 ---
SELECT o.Id, t.Id FROM dbo.O AS o JOIN dbo.T AS t ON t.A = o.X OR t.A = 5

Estimated cost: 0.0464 -> 0.341 (634% costlier)
Estimated rows: 1,000 -> 1 (99.9% fewer)
Runtime: 3ms -> 0ms (eliminated)
CPU time: 3ms -> 0ms (eliminated)
Logical reads: 800 -> 322 (59.8% fewer)
Memory grant: 1.0 MB -> 0.0 MB (eliminated)
DOP: 1 -> 1


##### key_lookup_plan.sqlplan vs lazy_spool_plan.sqlplan
=== Plan Comparison ===
Plan A: key_lookup_plan.sqlplan
Expand Down
82 changes: 82 additions & 0 deletions tests/PlanViewer.Core.Tests/PlanAnalyzerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -569,6 +569,88 @@ public void Rule15_JoinOrClause_DetectsConcatenationWithConstantScans()
Assert.Contains("UNION ALL", warnings[0].Message);
}

/// <summary>
/// #558: WHERE t.A IN (@p1, @p2) on an indexed column builds the same operator chain as a join
/// OR. It is a dynamic seek: Constant Scans produce [@p1] and [@p2], Merge Interval combines the
/// ranges, and one Index Seek reads them. It ran once and returned 2 rows, and a UNION ALL rewrite
/// would not help. The fixture is the reporter's own SQL Server 2022 actual plan.
/// </summary>
[Fact]
public void Rule15_JoinOrClause_DynamicSeekForParameterInList_NotFlagged()
{
var plan = PlanTestHelper.LoadAndAnalyze("in_list_dynamic_seek_plan.sqlplan");

Assert.Empty(PlanTestHelper.WarningsOfType(plan, "Join OR Clause"));
}

/// <summary>
/// The branch values of the reporter's plan, as the parser records them: parameters and a
/// literal. The same holds for local variables ([@a]) and for functions of a parameter, such as
/// LikeRangeStart([@a]) for an OR of LIKE patterns or abs([@p2]) inside an IN list.
/// </summary>
[Theory]
[InlineData("Expr1002 = [@p2]; Expr1003 = [@p2]; Expr1001 = (62)")]
[InlineData("Expr1004 = LikeRangeStart([@a]); Expr1005 = LikeRangeEnd([@a]); Expr1006 = LikeRangeInfo([@a])")]
[InlineData("Expr1002 = abs([@p2]); Expr1003 = abs([@p2]); Expr1001 = (62)")]
[InlineData("Expr1002 = [dbo].[fn]([@p1])")]
public void Rule15_JoinOrClause_ParameterOnlyLookup_DoesNotReadAnotherInput(string values)
{
Assert.False(PlanAnalyzer.ReadsAnotherInput(values));
}

/// <summary>
/// #558's shape guard must not cost a real join OR. This one mixes a column and a parameter:
/// ON t.A = o.X OR t.A = @p. One branch produces [o].[X] and the other produces [@p], and one
/// branch that reads the outer row is enough. Captured on SQL Server 2022.
/// </summary>
[Fact]
public void Rule15_JoinOrClause_ColumnBranchNextToParameterBranch_IsFlagged()
{
var plan = PlanTestHelper.LoadAndAnalyze("join_or_mixed_parameter_plan.sqlplan");

Assert.Single(PlanTestHelper.WarningsOfType(plan, "Join OR Clause"));
}

/// <summary>
/// An OR join on expressions of the outer columns (ON t.A = o.X + 1 OR t.A = o.Y + 1) computes
/// o.X + 1 on the outer input, so its branches produce [Expr1002] and [Expr1003] and name no
/// column at all. A check that looked for column names only, which is what the issue first
/// suggested, would lose this warning. Captured on SQL Server 2022.
/// </summary>
[Fact]
public void Rule15_JoinOrClause_OrJoinOnOuterExpressions_IsFlagged()
{
var plan = PlanTestHelper.LoadAndAnalyze("join_or_expression_plan.sqlplan");

Assert.Single(PlanTestHelper.WarningsOfType(plan, "Join OR Clause"));
}

/// <summary>
/// The branch values that real OR joins produce on SQL Server 2022: the outer columns; an
/// expression of the outer columns, which renders as [Expr1002]; and a table variable's column,
/// which renders without brackets around its name. The second row puts a parameter before a
/// column, so the scan must not stop at the first name it can skip.
/// </summary>
[Theory]
[InlineData("Expr1005 = [StackOverflow2013].[dbo].[Posts].[OwnerUserId] as [p].[OwnerUserId]; Expr1004 = (62)")]
[InlineData("Expr1008 = [@p]; Expr1007 = (62); Expr1010 = [Repro].[dbo].[O].[X] as [o].[X]")]
[InlineData("Expr1010 = [Expr1002]; Expr1011 = [Expr1002]; Expr1009 = (62)")]
[InlineData("Expr1008 = @tv.[X] as [v].[X]; Expr1007 = (62)")]
public void Rule15_JoinOrClause_LookupFromAnotherInput_IsRecognized(string values)
{
Assert.True(PlanAnalyzer.ReadsAnotherInput(values));
}

/// <summary>
/// A bracket inside a string literal is text. Read as a name, it would turn an IN list of
/// strings back into a false join OR.
/// </summary>
[Fact]
public void Rule15_JoinOrClause_BracketInsideStringLiteral_IsNotAName()
{
Assert.False(PlanAnalyzer.ReadsAnotherInput("Expr1002 = N'[Posts].[OwnerUserId]'; Expr1001 = (62)"));
}

// ---------------------------------------------------------------
// Rule 16: Nested Loops High Executions
// ---------------------------------------------------------------
Expand Down
Loading
Loading