Skip to content

Commit bb85522

Browse files
authored
fix join expression order bug and add validation (#291)
1 parent 291b59a commit bb85522

File tree

3 files changed

+372
-9
lines changed

3 files changed

+372
-9
lines changed

.changeset/pink-ducks-fix.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@tanstack/db": patch
3+
---
4+
5+
fixed an issue with joins where a specific order of references in the `eq()` expression was required, and added additional validation

packages/db/src/query/compiler/joins.ts

Lines changed: 97 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,12 @@ import {
77
import { compileExpression } from "./evaluators.js"
88
import { compileQuery } from "./index.js"
99
import type { IStreamBuilder, JoinType } from "@electric-sql/d2mini"
10-
import type { CollectionRef, JoinClause, QueryRef } from "../ir.js"
10+
import type {
11+
BasicExpression,
12+
CollectionRef,
13+
JoinClause,
14+
QueryRef,
15+
} from "../ir.js"
1116
import type {
1217
KeyedStream,
1318
NamespacedAndKeyedStream,
@@ -75,18 +80,26 @@ function processJoin(
7580
? `full`
7681
: (joinClause.type as JoinType)
7782

83+
// Analyze which table each expression refers to and swap if necessary
84+
const { mainExpr, joinedExpr } = analyzeJoinExpressions(
85+
joinClause.left,
86+
joinClause.right,
87+
mainTableAlias,
88+
joinedTableAlias
89+
)
90+
7891
// Pre-compile the join expressions
79-
const compiledLeftExpr = compileExpression(joinClause.left)
80-
const compiledRightExpr = compileExpression(joinClause.right)
92+
const compiledMainExpr = compileExpression(mainExpr)
93+
const compiledJoinedExpr = compileExpression(joinedExpr)
8194

8295
// Prepare the main pipeline for joining
8396
const mainPipeline = pipeline.pipe(
8497
map(([currentKey, namespacedRow]) => {
85-
// Extract the join key from the left side of the join condition
86-
const leftKey = compiledLeftExpr(namespacedRow)
98+
// Extract the join key from the main table expression
99+
const mainKey = compiledMainExpr(namespacedRow)
87100

88101
// Return [joinKey, [originalKey, namespacedRow]]
89-
return [leftKey, [currentKey, namespacedRow]] as [
102+
return [mainKey, [currentKey, namespacedRow]] as [
90103
unknown,
91104
[string, typeof namespacedRow],
92105
]
@@ -99,11 +112,11 @@ function processJoin(
99112
// Wrap the row in a namespaced structure
100113
const namespacedRow: NamespacedRow = { [joinedTableAlias]: row }
101114

102-
// Extract the join key from the right side of the join condition
103-
const rightKey = compiledRightExpr(namespacedRow)
115+
// Extract the join key from the joined table expression
116+
const joinedKey = compiledJoinedExpr(namespacedRow)
104117

105118
// Return [joinKey, [originalKey, namespacedRow]]
106-
return [rightKey, [currentKey, namespacedRow]] as [
119+
return [joinedKey, [currentKey, namespacedRow]] as [
107120
unknown,
108121
[string, typeof namespacedRow],
109122
]
@@ -121,6 +134,81 @@ function processJoin(
121134
)
122135
}
123136

137+
/**
138+
* Analyzes join expressions to determine which refers to which table
139+
* and returns them in the correct order (main table expression first, joined table expression second)
140+
*/
141+
function analyzeJoinExpressions(
142+
left: BasicExpression,
143+
right: BasicExpression,
144+
mainTableAlias: string,
145+
joinedTableAlias: string
146+
): { mainExpr: BasicExpression; joinedExpr: BasicExpression } {
147+
const leftTableAlias = getTableAliasFromExpression(left)
148+
const rightTableAlias = getTableAliasFromExpression(right)
149+
150+
// If left expression refers to main table and right refers to joined table, keep as is
151+
if (
152+
leftTableAlias === mainTableAlias &&
153+
rightTableAlias === joinedTableAlias
154+
) {
155+
return { mainExpr: left, joinedExpr: right }
156+
}
157+
158+
// If left expression refers to joined table and right refers to main table, swap them
159+
if (
160+
leftTableAlias === joinedTableAlias &&
161+
rightTableAlias === mainTableAlias
162+
) {
163+
return { mainExpr: right, joinedExpr: left }
164+
}
165+
166+
// If both expressions refer to the same alias, this is an invalid join
167+
if (leftTableAlias === rightTableAlias) {
168+
throw new Error(
169+
`Invalid join condition: both expressions refer to the same table "${leftTableAlias}"`
170+
)
171+
}
172+
173+
// If one expression doesn't refer to either table, this is an invalid join
174+
if (!leftTableAlias || !rightTableAlias) {
175+
throw new Error(
176+
`Invalid join condition: expressions must reference table aliases "${mainTableAlias}" and "${joinedTableAlias}"`
177+
)
178+
}
179+
180+
// If expressions refer to tables not involved in this join, this is an invalid join
181+
throw new Error(
182+
`Invalid join condition: expressions reference tables "${leftTableAlias}" and "${rightTableAlias}" but join is between "${mainTableAlias}" and "${joinedTableAlias}"`
183+
)
184+
}
185+
186+
/**
187+
* Extracts the table alias from a join expression
188+
*/
189+
function getTableAliasFromExpression(expr: BasicExpression): string | null {
190+
switch (expr.type) {
191+
case `ref`:
192+
// PropRef path has the table alias as the first element
193+
return expr.path[0] || null
194+
case `func`: {
195+
// For function expressions, we need to check if all arguments refer to the same table
196+
const tableAliases = new Set<string>()
197+
for (const arg of expr.args) {
198+
const alias = getTableAliasFromExpression(arg)
199+
if (alias) {
200+
tableAliases.add(alias)
201+
}
202+
}
203+
// If all arguments refer to the same table, return that table alias
204+
return tableAliases.size === 1 ? Array.from(tableAliases)[0]! : null
205+
}
206+
default:
207+
// Values (type='val') don't reference any table
208+
return null
209+
}
210+
}
211+
124212
/**
125213
* Processes the join source (collection or sub-query)
126214
*/

0 commit comments

Comments
 (0)