Skip to content

Commit 248e2c6

Browse files
authored
Don't push down where clauses that only touch the namespace of a source and not a prop (#600)
* add test * the fix * changeset
1 parent ce7e2b2 commit 248e2c6

File tree

3 files changed

+80
-7
lines changed

3 files changed

+80
-7
lines changed

.changeset/curvy-mangos-yell.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+
Prevent pushing down of where clauses that only touch the namespace of a source, rather than a prop on that namespace. This ensures that the semantics of the query are maintained for things such as `isUndefined(namespace)` after a join.

packages/db/src/query/optimizer.ts

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,8 @@ export interface AnalyzedWhereClause {
142142
expression: BasicExpression<boolean>
143143
/** Set of table/source aliases that this WHERE clause touches */
144144
touchedSources: Set<string>
145+
/** Whether this clause contains namespace-only references that prevent pushdown */
146+
hasNamespaceOnlyRef: boolean
145147
}
146148

147149
/**
@@ -486,19 +488,31 @@ function splitAndClausesRecursive(
486488
* This determines whether a clause can be pushed down to a specific table
487489
* or must remain in the main query (for multi-source clauses like join conditions).
488490
*
491+
* Special handling for namespace-only references in outer joins:
492+
* WHERE clauses that reference only a table namespace (e.g., isUndefined(special), eq(special, value))
493+
* rather than specific properties (e.g., isUndefined(special.id), eq(special.id, value)) are treated as
494+
* multi-source to prevent incorrect predicate pushdown that would change join semantics.
495+
*
489496
* @param clause - The WHERE expression to analyze
490497
* @returns Analysis result with the expression and touched source aliases
491498
*
492499
* @example
493500
* ```typescript
494-
* // eq(users.department_id, 1) -> touches ['users']
495-
* // eq(users.id, posts.user_id) -> touches ['users', 'posts']
501+
* // eq(users.department_id, 1) -> touches ['users'], hasNamespaceOnlyRef: false
502+
* // eq(users.id, posts.user_id) -> touches ['users', 'posts'], hasNamespaceOnlyRef: false
503+
* // isUndefined(special) -> touches ['special'], hasNamespaceOnlyRef: true (prevents pushdown)
504+
* // eq(special, someValue) -> touches ['special'], hasNamespaceOnlyRef: true (prevents pushdown)
505+
* // isUndefined(special.id) -> touches ['special'], hasNamespaceOnlyRef: false (allows pushdown)
506+
* // eq(special.id, 5) -> touches ['special'], hasNamespaceOnlyRef: false (allows pushdown)
496507
* ```
497508
*/
498509
function analyzeWhereClause(
499510
clause: BasicExpression<boolean>
500511
): AnalyzedWhereClause {
512+
// Track which table aliases this WHERE clause touches
501513
const touchedSources = new Set<string>()
514+
// Track whether this clause contains namespace-only references that prevent pushdown
515+
let hasNamespaceOnlyRef = false
502516

503517
/**
504518
* Recursively collect all table aliases referenced in an expression
@@ -511,6 +525,13 @@ function analyzeWhereClause(
511525
const firstElement = expr.path[0]
512526
if (firstElement) {
513527
touchedSources.add(firstElement)
528+
529+
// If the path has only one element (just the namespace),
530+
// this is a namespace-only reference that should not be pushed down
531+
// This applies to ANY function, not just existence-checking functions
532+
if (expr.path.length === 1) {
533+
hasNamespaceOnlyRef = true
534+
}
514535
}
515536
}
516537
break
@@ -537,6 +558,7 @@ function analyzeWhereClause(
537558
return {
538559
expression: clause,
539560
touchedSources,
561+
hasNamespaceOnlyRef,
540562
}
541563
}
542564

@@ -557,15 +579,15 @@ function groupWhereClauses(
557579

558580
// Categorize each clause based on how many sources it touches
559581
for (const clause of analyzedClauses) {
560-
if (clause.touchedSources.size === 1) {
561-
// Single source clause - can be optimized
582+
if (clause.touchedSources.size === 1 && !clause.hasNamespaceOnlyRef) {
583+
// Single source clause without namespace-only references - can be optimized
562584
const source = Array.from(clause.touchedSources)[0]!
563585
if (!singleSource.has(source)) {
564586
singleSource.set(source, [])
565587
}
566588
singleSource.get(source)!.push(clause.expression)
567-
} else if (clause.touchedSources.size > 1) {
568-
// Multi-source clause - must stay in main query
589+
} else if (clause.touchedSources.size > 1 || clause.hasNamespaceOnlyRef) {
590+
// Multi-source clause or namespace-only reference - must stay in main query
569591
multiSource.push(clause.expression)
570592
}
571593
// Skip clauses that touch no sources (constants) - they don't need optimization

packages/db/tests/query/join.test.ts

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
import { beforeEach, describe, expect, test } from "vitest"
2-
import { concat, createLiveQueryCollection, eq } from "../../src/query/index.js"
2+
import {
3+
concat,
4+
createLiveQueryCollection,
5+
eq,
6+
isUndefined,
7+
} from "../../src/query/index.js"
38
import { createCollection } from "../../src/collection/index.js"
49
import { mockSyncCollectionOptions } from "../utils.js"
510

@@ -1429,6 +1434,47 @@ function createJoinTests(autoIndex: `off` | `eager`): void {
14291434
`Invalid join condition: both expressions refer to the same table "user"`
14301435
)
14311436
})
1437+
1438+
test(`should not push down isUndefined(namespace) to subquery`, () => {
1439+
const usersCollection = createUsersCollection()
1440+
const specialUsersCollection = createCollection(
1441+
mockSyncCollectionOptions({
1442+
id: `special-users`,
1443+
getKey: (user) => user.id,
1444+
initialData: [{ id: 1, special: true }],
1445+
})
1446+
)
1447+
1448+
const joinQuery = createLiveQueryCollection({
1449+
startSync: true,
1450+
query: (q) =>
1451+
q
1452+
.from({ user: usersCollection })
1453+
.leftJoin(
1454+
{ special: specialUsersCollection },
1455+
({ user, special }) => eq(user.id, special.id)
1456+
)
1457+
.where(({ special }) => isUndefined(special)),
1458+
})
1459+
1460+
// Should return users that don't have a matching special user
1461+
// This should be users with IDs 2, 3, 4 (since only user 1 has a special record)
1462+
const results = joinQuery.toArray
1463+
expect(results).toHaveLength(3)
1464+
1465+
// All results should have special as undefined
1466+
for (const row of results) {
1467+
expect(row.special).toBeUndefined()
1468+
}
1469+
1470+
// Should not include user 1 (Alice) since she has a special record
1471+
const aliceResult = results.find((r) => r.user.id === 1)
1472+
expect(aliceResult).toBeUndefined()
1473+
1474+
// Should include users 2, 3, 4 (Bob, Charlie, Dave)
1475+
const userIds = results.map((r) => r.user.id).sort()
1476+
expect(userIds).toEqual([2, 3, 4])
1477+
})
14321478
})
14331479
}
14341480

0 commit comments

Comments
 (0)