Skip to content

Commit c409e01

Browse files
committed
Rewrite comma-join / CROSS JOIN carrying a WHERE equi-predicate into an INNER JOIN at parse time, so FROM a, b WHERE a.k = b.k rides the equi-join seek/hash path instead of the O(L×R) nested loop.
1 parent aaa56ab commit c409e01

7 files changed

Lines changed: 198 additions & 1 deletion

File tree

CLAUDE.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ Per-feature deep-dives live under `docs/claude/`. Each entry below is a trigger:
137137
- **`SqlType.Promote` / `PromoteForArithmetic` / decimal precision-scale / int↔string promotion**[`arithmetic.md`](docs/claude/arithmetic.md).
138138
- **`Cast` / coercion error paths** (CAST/CONVERT narrow targets, TRY_CAST/TRY_CONVERT swallow set, PARSE/TRY_PARSE culture-aware parsing) → [`casting.md`](docs/claude/casting.md).
139139
- **`Selection`, aggregates, window functions, set ops, CASE, OFFSET/FETCH**[`query.md`](docs/claude/query.md).
140-
- **JOIN / APPLY** — INNER / LEFT / RIGHT / FULL / CROSS + CROSS/OUTER APPLY, ANSI-89 comma-FROM, EF `LeftJoin`/`RightJoin` routing (no LINQ `FullJoin`, so FULL is raw-SQL-only); `JoinDriver` equi-join hash fast path + `SqlValueKey` keying, nested-loop fallback, RIGHT/FULL materialization + derived-table-right, `JoinDiagnostics` strategy guard → [`joins.md`](docs/claude/joins.md).
140+
- **JOIN / APPLY** — INNER / LEFT / RIGHT / FULL / CROSS + CROSS/OUTER APPLY, ANSI-89 comma-FROM, EF `LeftJoin`/`RightJoin` routing (no LINQ `FullJoin`, so FULL is raw-SQL-only); `JoinDriver` equi-join hash fast path + `SqlValueKey` keying, nested-loop fallback, parse-time comma/`CROSS JOIN`+WHERE → equi-`INNER` rewrite, RIGHT/FULL materialization + derived-table-right, `JoinDiagnostics` strategy guard → [`joins.md`](docs/claude/joins.md).
141141
- **`PIVOT` / `UNPIVOT`** table operators — PIVOT desugars to grouped conditional aggregation (implicit grouping = all inner columns except FOR + aggregate-arg), UNPIVOT is a NULL-skipping unfold; both attach as a postfix FROM-source wrapper and ride the derived-table `LateralPlan` seam → [`pivot.md`](docs/claude/pivot.md).
142142
- **UPDATE / DELETE / INSERT…SELECT / SELECT…INTO / rowversion (incl. `@@DBTS` / `MIN_ACTIVE_ROWVERSION`) / identity helpers (`@@IDENTITY` / `SCOPE_IDENTITY` / `IDENT_CURRENT` / `IDENT_INCR` / `IDENT_SEED`) / `@@ROWCOUNT` / `ROWCOUNT_BIG` / OUTPUT / MERGE**[`dml.md`](docs/claude/dml.md).
143143
- **Variables, control flow (IF/WHILE/BREAK/CONTINUE/RETURN), TRY/CATCH+THROW+ERROR_*, `@@ERROR` / `@@TRANCOUNT` / `XACT_STATE`, PRINT, WAITFOR**[`control-flow.md`](docs/claude/control-flow.md).

SqlServerSimulator.Tests.Internal/JoinStrategyTests.cs

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,55 @@ public void CrossJoin_UsesNestedLoops()
8686
=> Contains("Cross:NestedLoops",
8787
CaptureStrategies("select a.id from a cross join b"));
8888

89+
/// <summary>
90+
/// ANSI-89 comma join with an equi-predicate in WHERE: the parser rewrites
91+
/// `FROM a, b WHERE a.id = b.a_id` into `a INNER JOIN b ON a.id = b.a_id`,
92+
/// so it hashes instead of nested-looping the cross product.
93+
/// </summary>
94+
[TestMethod]
95+
public void CommaJoin_WithEquiPredicate_TakesHashPath()
96+
=> Contains("Inner:HashMatch(keys=1,residual=0)",
97+
CaptureStrategies("select a.id from a, b where a.id = b.a_id"));
98+
99+
/// <summary>
100+
/// Explicit CROSS JOIN carrying an equi-predicate in WHERE is the same shape
101+
/// post-parse (JoinKind.Cross, null ON) and rewrites identically.
102+
/// </summary>
103+
[TestMethod]
104+
public void ExplicitCrossJoin_WithEquiPredicate_TakesHashPath()
105+
=> Contains("Inner:HashMatch(keys=1,residual=0)",
106+
CaptureStrategies("select a.id from a cross join b where a.id = b.a_id"));
107+
108+
/// <summary>
109+
/// A non-equi WHERE term alongside the equi-key isn't pulled into the
110+
/// synthesized ON (only equi-keys are) — it stays a post-join WHERE filter,
111+
/// so the join's residual count is 0, not 1.
112+
/// </summary>
113+
[TestMethod]
114+
public void CommaJoin_NonEquiTermStaysInWhere_NotPulledToOn()
115+
=> Contains("Inner:HashMatch(keys=1,residual=0)",
116+
CaptureStrategies("select a.id from a, b where a.id = b.a_id and b.id > 10"));
117+
118+
/// <summary>
119+
/// No equi-key connects the two comma sources, so there's nothing to pull
120+
/// into an ON — the join stays a Cross nested loop (the cross product is
121+
/// genuinely required).
122+
/// </summary>
123+
[TestMethod]
124+
public void CommaJoin_NoEquiPredicate_StaysNestedLoops()
125+
=> Contains("Cross:NestedLoops",
126+
CaptureStrategies("select a.id from a, b where a.id <> b.a_id"));
127+
128+
/// <summary>
129+
/// Comma join + WHERE filter on the small outer: after the Cross→Inner
130+
/// rewrite the leftmost is seeked to one row and the indexed inner is seeked
131+
/// per outer row — same acceleration the explicit-JOIN form already got.
132+
/// </summary>
133+
[TestMethod]
134+
public void CommaJoin_SmallOuter_IndexedInner_SeeksPerOuter()
135+
=> Contains("Inner:NestedLoopIndexSeek(keys=1)",
136+
CaptureStrategies(IndexedSetup, "select p.label, c.amt from p, c where c.pid = p.id and p.id = 5"));
137+
89138
/// <summary>
90139
/// WHERE p.id = 5 pushes down to seek the leftmost to one row, then the
91140
/// inner (indexed on pid) is seeked per outer row instead of hash-built.

SqlServerSimulator.Tests/JoinTests.cs

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -491,6 +491,72 @@ insert c values (1, 'c1'), (3, 'c3'), (5, 'c5')
491491
CollectionAssert.AreEquivalent(new[] { ("a1", "b1", "c1") }, rows);
492492
}
493493

494+
// A NULL join key never matches under `=` (NULL = x is UNKNOWN), so the
495+
// comma-join's Cross→Inner rewrite must drop those rows — exactly as the
496+
// pre-rewrite cross-product-then-WHERE did. b row (13, NULL) and a rows
497+
// with no matching b are absent.
498+
[TestMethod]
499+
public void CommaFrom_NullJoinKey_ExcludedAfterRewrite()
500+
{
501+
using var connection = SeededAB();
502+
_ = connection.CreateCommand("insert b values (13, null, 400)").ExecuteNonQuery();
503+
var rows = ReadIntPairs(connection.CreateCommand("select a.id, b.val from a, b where a.id = b.a_id"));
504+
CollectionAssert.AreEquivalent(new[] { (1, 100), (1, 200), (2, 300) }, rows);
505+
}
506+
507+
// The comma source's equi-predicate references the NULL-extended side of a
508+
// preceding LEFT JOIN. A null-extended b.id makes `b.id = c.id` UNKNOWN, so
509+
// those rows drop — identical whether the predicate filters post-cross
510+
// (pre-rewrite) or anchors the synthesized INNER JOIN ON (post-rewrite),
511+
// because the predicate stays in WHERE as the residual.
512+
[TestMethod]
513+
public void CommaFrom_AfterLeftJoin_NullExtendedKeyDropsRow()
514+
{
515+
var simulation = new Simulation();
516+
_ = simulation.ExecuteNonQuery("""
517+
create table a (id int, av varchar(10));
518+
create table b (id int, bv varchar(10));
519+
create table c (id int, cv varchar(10));
520+
insert a values (1, 'a1'), (2, 'a2');
521+
insert b values (1, 'b1');
522+
insert c values (1, 'c1'), (2, 'c2')
523+
""");
524+
525+
using var connection = simulation.CreateOpenConnection();
526+
using var reader = connection.CreateCommand(
527+
"select a.av, b.bv, c.cv from a left join b on a.id = b.id, c where b.id = c.id").ExecuteReader();
528+
var rows = new List<(string, string, string)>();
529+
while (reader.Read())
530+
rows.Add((reader.GetString(0), reader.GetString(1), reader.GetString(2)));
531+
// a2's b is null-extended → b.id = c.id is UNKNOWN → only a1's row survives.
532+
CollectionAssert.AreEquivalent(new[] { ("a1", "b1", "c1") }, rows);
533+
}
534+
535+
// A comma source feeding a later LEFT JOIN must still null-extend: the
536+
// rewrite only flips the Cross (a,b) level to INNER, leaving the explicit
537+
// LEFT JOIN to c untouched, so an a/b pair with no c match keeps its row.
538+
[TestMethod]
539+
public void CommaFrom_FeedingLeftJoin_StillNullExtends()
540+
{
541+
var simulation = new Simulation();
542+
_ = simulation.ExecuteNonQuery("""
543+
create table a (id int, av varchar(10));
544+
create table b (id int, j int, bv varchar(10));
545+
create table c (k int, cv varchar(10));
546+
insert a values (1, 'a1'), (2, 'a2');
547+
insert b values (1, 9, 'b1'), (2, 8, 'b2');
548+
insert c values (9, 'c9')
549+
""");
550+
551+
using var connection = simulation.CreateOpenConnection();
552+
using var reader = connection.CreateCommand(
553+
"select a.av, b.bv, c.cv from a, b left join c on b.j = c.k where a.id = b.id order by a.av").ExecuteReader();
554+
var rows = new List<(string, string, string?)>();
555+
while (reader.Read())
556+
rows.Add((reader.GetString(0), reader.GetString(1), reader.IsDBNull(2) ? null : reader.GetString(2)));
557+
CollectionAssert.AreEqual(new[] { ("a1", "b1", (string?)"c9"), ("a2", "b2", null) }, rows);
558+
}
559+
494560
[TestMethod]
495561
public void CommaFrom_TrailingComma_RaisesSyntaxError()
496562
=> _ = new Simulation().AssertSqlError("""

SqlServerSimulator/Parser/BooleanExpression.cs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -592,6 +592,15 @@ private static BetweenExpression ParseBetween(Expression left, ParserContext con
592592
/// </summary>
593593
internal virtual void CollectConjuncts(List<BooleanExpression> sink) => sink.Add(this);
594594

595+
/// <summary>
596+
/// Combines two predicates with a three-valued <c>AND</c>. Used to
597+
/// synthesize an <c>ON</c> predicate from the WHERE conjuncts when a
598+
/// comma / <c>CROSS JOIN</c> is rewritten into an equi-join — the result
599+
/// re-splits cleanly through <see cref="CollectConjuncts"/>, so the join
600+
/// planner recovers the individual key equalities.
601+
/// </summary>
602+
internal static BooleanExpression And(BooleanExpression left, BooleanExpression right) => new AndExpression(left, right);
603+
595604
/// <summary>
596605
/// Flattens a top-level <c>OR</c> chain into its individual disjunct terms,
597606
/// appending each to <paramref name="sink"/>. A non-<c>OR</c> predicate

SqlServerSimulator/Parser/Selection.Execution.Joins.cs

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -454,6 +454,66 @@ private static int SourceSide(Reference reference, FromSource[] sources, int lev
454454

455455
private static readonly JoinSpec[] NoJoins = [];
456456

457+
/// <summary>
458+
/// Rewrites a comma-join / explicit <c>CROSS JOIN</c> whose WHERE carries an
459+
/// equi-join predicate into an <c>INNER JOIN</c> with that predicate as its
460+
/// <c>ON</c>, so the equi-join hash / per-outer-seek machinery (which only
461+
/// fires for INNER / LEFT / RIGHT / FULL — a <see cref="JoinKind.Cross"/>
462+
/// always falls to the O(L×R) nested loop) accelerates it.
463+
/// <c>FROM a, b WHERE a.k = b.k</c> is the textbook equivalent of
464+
/// <c>a INNER JOIN b ON a.k = b.k</c>.
465+
/// <para>
466+
/// Run once at parse time (the join shape is value-independent), so the
467+
/// rewritten array is captured in the cached plan. Every pulled conjunct
468+
/// <b>stays</b> in <paramref name="excluders"/> as a residual filter — never
469+
/// removed — so flipping a Cross level to Inner can only drop rows the WHERE
470+
/// would drop anyway: the post-WHERE result is provably unchanged whatever
471+
/// outer joins sit elsewhere in the chain. Only Cross levels with no existing
472+
/// <c>ON</c> and a re-enumerable (non-lateral) right side are touched; a level
473+
/// that already carries an <c>ON</c> is left alone. Returns the same array
474+
/// when nothing rewrites — the common single-source / explicit-join case.
475+
/// </para>
476+
/// </summary>
477+
internal static JoinSpec[] RewriteCommaJoinsToEquiJoins(
478+
FromSource[] sources, JoinSpec[] joins, List<BooleanExpression> excluders)
479+
{
480+
if (sources.Length < 2 || excluders.Count == 0)
481+
return joins;
482+
483+
List<BooleanExpression>? conjuncts = null;
484+
JoinSpec[]? rewritten = null;
485+
for (var level = 1; level < sources.Length; level++)
486+
{
487+
var join = joins[level - 1];
488+
// Only a Cross join with no ON is a candidate; a lateral / derived
489+
// right side has no re-enumerable Rows to hash or seek, so leave it
490+
// on the nested-loop path.
491+
if (join.Kind != JoinKind.Cross || join.OnPredicate is not null || sources[level].LateralPlan is not null)
492+
continue;
493+
494+
if (conjuncts is null)
495+
{
496+
conjuncts = [];
497+
foreach (var excluder in excluders)
498+
excluder.CollectConjuncts(conjuncts);
499+
}
500+
501+
BooleanExpression? on = null;
502+
foreach (var conjunct in conjuncts)
503+
{
504+
if (TryExtractEquiKey(conjunct, sources, level, out _))
505+
on = on is null ? conjunct : BooleanExpression.And(on, conjunct);
506+
}
507+
508+
if (on is null)
509+
continue;
510+
rewritten ??= (JoinSpec[])joins.Clone();
511+
rewritten[level - 1] = new JoinSpec(JoinKind.Inner, on);
512+
}
513+
514+
return rewritten ?? joins;
515+
}
516+
457517
/// <summary>
458518
/// Outer-row cap for choosing the per-outer index-seek strategy. A small
459519
/// outer set joined to an indexed inner wins big from seeking the inner per

SqlServerSimulator/Parser/Selection.Execution.cs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,13 @@ private static Selection BuildSqlProjection(
112112
{
113113
if (windows.Count > 0 && (aggregates.Count > 0 || fromClause.GroupingSets.Count > 0 || fromClause.Having is not null))
114114
throw new NotSupportedException("Combining window functions with GROUP BY / HAVING / aggregates in the same SELECT isn't modeled. EF Core 10 doesn't emit this shape.");
115+
116+
// Convert a comma-join / CROSS JOIN carrying an equi-join predicate in
117+
// WHERE into an INNER JOIN, so it rides the equi-join seek / hash path
118+
// instead of the O(L×R) nested loop. Value-independent, so it's done
119+
// once here and the rewritten array is captured in the cached plan.
120+
joins = RewriteCommaJoinsToEquiJoins(sources, joins, fromClause.Excluders);
121+
115122
var orderBy = fromClause.OrderBy;
116123
var outputSchema = new SqlType[expressions.Count];
117124
var outputColumnNames = new string[expressions.Count];

docs/claude/joins.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,12 @@ INNER / bare `JOIN` (= INNER) / LEFT [OUTER] / RIGHT [OUTER] / FULL [OUTER] / CR
88

99
**Quirk — back-reference across a comma silently succeeds** (e.g. `FROM a, b JOIN c ON c.id = a.id`): real SQL Server binds `b JOIN c ON …` as its own scope and raises Msg 4104 because `a` isn't visible there; the simulator doesn't do parse-time scope-checking on ON predicates (column refs resolve at runtime through `ResolveAcrossTuple` across all FROM sources in the tuple), so the query runs and returns the Cartesian-filtered rowset. The common shapes — basic `FROM a, b WHERE …`, multi-comma chains, comma + derived table, explicit JOIN followed by comma — all match real SQL Server byte-for-byte; only this rare back-reference-across-comma case diverges, toward "more permissive" rather than wrong rowset.
1010

11+
### Cross→Inner equi-join rewrite
12+
13+
A bare `JoinKind.Cross` (from either a comma or an explicit `CROSS JOIN`) carries no ON predicate, so `ApplyJoin` never computes an equi-plan for it — it would always fall to the O(L×R) nested loop, even when WHERE carries a clean `a.k = b.k`. `RewriteCommaJoinsToEquiJoins` (run once at parse time, inside `BuildSqlProjection`, so the rewritten `joins[]` is captured in the cached plan) closes that gap: for each Cross level with no ON and a re-enumerable (non-lateral) right side, it scans the WHERE conjuncts for equi-keys connecting a prior source to that level (the same `TryExtractEquiKey` classifier the explicit-JOIN equi-plan uses), synthesizes them into an ON via `BooleanExpression.And`, and flips the level to `JoinKind.Inner`. It then rides `EquiJoinSeekOrHash` / `HashEquiJoin` exactly like an explicitly-written `INNER JOIN … ON`.
14+
15+
`FROM a, b WHERE a.k = b.k``a INNER JOIN b ON a.k = b.k` — the textbook inner-join ON/WHERE identity. **Correctness is anchored by the residual invariant**: every pulled conjunct *stays* in the WHERE excluders (never removed), so flipping Cross→Inner can only drop rows the WHERE would drop anyway. The post-WHERE result is therefore provably unchanged regardless of outer joins elsewhere in the chain — converting a `Cross` level mixed with a preceding LEFT JOIN's null-extended side, or feeding a later LEFT JOIN, leaves the surrounding outer-join semantics intact (the rewrite only ever touches Cross levels, never one that already has an ON). Only equi-keys are pulled; a non-equi WHERE term (`b.id > 10`) stays a post-join filter, so the synthesized ON's residual count is 0. A derived-table right side after a comma keeps its `LateralPlan`, so it's skipped (can't be seeked/hashed anyway) and stays on the nested loop.
16+
1117
## JoinDriver
1218

1319
`JoinDriver` is a fold over `joins[]`: the leftmost rowset is wrapped with each join's operator in turn to produce the final enumerator. `ApplyJoin` picks the operator per join level — and is the single point where the strategy (hash vs nested loop) is decided.

0 commit comments

Comments
 (0)