Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions .changeset/curvy-mangos-yell.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@tanstack/db": patch
---

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.
34 changes: 28 additions & 6 deletions packages/db/src/query/optimizer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,8 @@ export interface AnalyzedWhereClause {
expression: BasicExpression<boolean>
/** Set of table/source aliases that this WHERE clause touches */
touchedSources: Set<string>
/** Whether this clause contains namespace-only references that prevent pushdown */
hasNamespaceOnlyRef: boolean
}

/**
Expand Down Expand Up @@ -486,19 +488,31 @@ function splitAndClausesRecursive(
* This determines whether a clause can be pushed down to a specific table
* or must remain in the main query (for multi-source clauses like join conditions).
*
* Special handling for namespace-only references in outer joins:
* WHERE clauses that reference only a table namespace (e.g., isUndefined(special), eq(special, value))
* rather than specific properties (e.g., isUndefined(special.id), eq(special.id, value)) are treated as
* multi-source to prevent incorrect predicate pushdown that would change join semantics.
*
* @param clause - The WHERE expression to analyze
* @returns Analysis result with the expression and touched source aliases
*
* @example
* ```typescript
* // eq(users.department_id, 1) -> touches ['users']
* // eq(users.id, posts.user_id) -> touches ['users', 'posts']
* // eq(users.department_id, 1) -> touches ['users'], hasNamespaceOnlyRef: false
* // eq(users.id, posts.user_id) -> touches ['users', 'posts'], hasNamespaceOnlyRef: false
* // isUndefined(special) -> touches ['special'], hasNamespaceOnlyRef: true (prevents pushdown)
* // eq(special, someValue) -> touches ['special'], hasNamespaceOnlyRef: true (prevents pushdown)
* // isUndefined(special.id) -> touches ['special'], hasNamespaceOnlyRef: false (allows pushdown)
* // eq(special.id, 5) -> touches ['special'], hasNamespaceOnlyRef: false (allows pushdown)
* ```
*/
function analyzeWhereClause(
clause: BasicExpression<boolean>
): AnalyzedWhereClause {
// Track which table aliases this WHERE clause touches
const touchedSources = new Set<string>()
// Track whether this clause contains namespace-only references that prevent pushdown
let hasNamespaceOnlyRef = false

/**
* Recursively collect all table aliases referenced in an expression
Expand All @@ -511,6 +525,13 @@ function analyzeWhereClause(
const firstElement = expr.path[0]
if (firstElement) {
touchedSources.add(firstElement)

// If the path has only one element (just the namespace),
// this is a namespace-only reference that should not be pushed down
// This applies to ANY function, not just existence-checking functions
if (expr.path.length === 1) {
hasNamespaceOnlyRef = true
}
}
}
break
Expand All @@ -537,6 +558,7 @@ function analyzeWhereClause(
return {
expression: clause,
touchedSources,
hasNamespaceOnlyRef,
}
}

Expand All @@ -557,15 +579,15 @@ function groupWhereClauses(

// Categorize each clause based on how many sources it touches
for (const clause of analyzedClauses) {
if (clause.touchedSources.size === 1) {
// Single source clause - can be optimized
if (clause.touchedSources.size === 1 && !clause.hasNamespaceOnlyRef) {
// Single source clause without namespace-only references - can be optimized
const source = Array.from(clause.touchedSources)[0]!
if (!singleSource.has(source)) {
singleSource.set(source, [])
}
singleSource.get(source)!.push(clause.expression)
} else if (clause.touchedSources.size > 1) {
// Multi-source clause - must stay in main query
} else if (clause.touchedSources.size > 1 || clause.hasNamespaceOnlyRef) {
// Multi-source clause or namespace-only reference - must stay in main query
multiSource.push(clause.expression)
}
// Skip clauses that touch no sources (constants) - they don't need optimization
Expand Down
48 changes: 47 additions & 1 deletion packages/db/tests/query/join.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
import { beforeEach, describe, expect, test } from "vitest"
import { concat, createLiveQueryCollection, eq } from "../../src/query/index.js"
import {
concat,
createLiveQueryCollection,
eq,
isUndefined,
} from "../../src/query/index.js"
import { createCollection } from "../../src/collection/index.js"
import { mockSyncCollectionOptions } from "../utils.js"

Expand Down Expand Up @@ -1429,6 +1434,47 @@ function createJoinTests(autoIndex: `off` | `eager`): void {
`Invalid join condition: both expressions refer to the same table "user"`
)
})

test(`should not push down isUndefined(namespace) to subquery`, () => {
const usersCollection = createUsersCollection()
const specialUsersCollection = createCollection(
mockSyncCollectionOptions({
id: `special-users`,
getKey: (user) => user.id,
initialData: [{ id: 1, special: true }],
})
)

const joinQuery = createLiveQueryCollection({
startSync: true,
query: (q) =>
q
.from({ user: usersCollection })
.leftJoin(
{ special: specialUsersCollection },
({ user, special }) => eq(user.id, special.id)
)
.where(({ special }) => isUndefined(special)),
})

// Should return users that don't have a matching special user
// This should be users with IDs 2, 3, 4 (since only user 1 has a special record)
const results = joinQuery.toArray
expect(results).toHaveLength(3)

// All results should have special as undefined
for (const row of results) {
expect(row.special).toBeUndefined()
}

// Should not include user 1 (Alice) since she has a special record
const aliceResult = results.find((r) => r.user.id === 1)
expect(aliceResult).toBeUndefined()

// Should include users 2, 3, 4 (Bob, Charlie, Dave)
const userIds = results.map((r) => r.user.id).sort()
expect(userIds).toEqual([2, 3, 4])
})
})
}

Expand Down
Loading