Always pass where clause to loadSubset (#851)

kevin-dp · web-flow · commit 846a8309a243 · 2025-11-19T13:37:15.000+01:00
* Unit test to reproduce problem with operators not being passed to loadSubset

* Do not filter non-indexable operators in convertToBasicExpression

* Changeset

* Unit tests in query collection to check that where clause is passed through to the query function's context

* Revert changes to lockfile

* Remove isConvertibleToCollectionFilter function

* Rename convertToBasicExpression to normalizeExpressionPaths and convertOrderByToBasicExpression to normalizeOrderByPaths
diff --git a/.changeset/poor-walls-taste.md b/.changeset/poor-walls-taste.md
@@ -0,0 +1,5 @@
+---
+"@tanstack/db": patch
+---
+
+Pass all operators in where clauses to the collection's loadSubset function
diff --git a/packages/db/src/query/compiler/expressions.ts b/packages/db/src/query/compiler/expressions.ts
@@ -2,61 +2,26 @@ import { Func, PropRef, Value } from "../ir.js"
 import type { BasicExpression, OrderBy } from "../ir.js"
 
 /**
- * Functions supported by the collection index system.
- * These are the only functions that can be used in WHERE clauses
- * that are pushed down to collection subscriptions for index optimization.
- */
-export const SUPPORTED_COLLECTION_FUNCS = new Set([
-  `eq`,
-  `gt`,
-  `lt`,
-  `gte`,
-  `lte`,
-  `and`,
-  `or`,
-  `in`,
-  `isNull`,
-  `isUndefined`,
-  `not`,
-])
-
-/**
- * Determines if a WHERE clause can be converted to collection-compatible BasicExpression format.
- * This checks if the expression only uses functions supported by the collection index system.
+ * Normalizes a WHERE clause expression by removing table aliases from property references.
  *
- * @param whereClause - The WHERE clause to check
- * @returns True if the clause can be converted for collection index optimization
- */
-export function isConvertibleToCollectionFilter(
-  whereClause: BasicExpression<boolean>
-): boolean {
-  const tpe = whereClause.type
-  if (tpe === `func`) {
-    // Check if this function is supported
-    if (!SUPPORTED_COLLECTION_FUNCS.has(whereClause.name)) {
-      return false
-    }
-    // Recursively check all arguments
-    return whereClause.args.every((arg) =>
-      isConvertibleToCollectionFilter(arg as BasicExpression<boolean>)
-    )
-  }
-  return [`val`, `ref`].includes(tpe)
-}
-
-/**
- * Converts a WHERE clause to BasicExpression format compatible with collection indexes.
- * This function creates proper BasicExpression class instances that the collection
- * index system can understand.
+ * This function recursively traverses an expression tree and creates new BasicExpression
+ * instances with normalized paths. The main transformation is removing the collection alias
+ * from property reference paths (e.g., `['user', 'id']` becomes `['id']` when `collectionAlias`
+ * is `'user'`), which is needed when converting query-level expressions to collection-level
+ * expressions for subscriptions.
  *
- * @param whereClause - The WHERE clause to convert
- * @param collectionAlias - The alias of the collection being filtered
- * @returns The converted BasicExpression or null if conversion fails
+ * @param whereClause - The WHERE clause expression to normalize
+ * @param collectionAlias - The alias of the collection being filtered (to strip from paths)
+ * @returns A new BasicExpression with normalized paths
+ *
+ * @example
+ * // Input: ref with path ['user', 'id'] where collectionAlias is 'user'
+ * // Output: ref with path ['id']
  */
-export function convertToBasicExpression(
+export function normalizeExpressionPaths(
   whereClause: BasicExpression<boolean>,
   collectionAlias: string
-): BasicExpression<boolean> | null {
+): BasicExpression<boolean> {
   const tpe = whereClause.type
   if (tpe === `val`) {
     return new Value(whereClause.value)
@@ -74,42 +39,29 @@ export function convertToBasicExpression(
     // Fallback for non-array paths
     return new PropRef(Array.isArray(path) ? path : [String(path)])
   } else {
-    // Check if this function is supported
-    if (!SUPPORTED_COLLECTION_FUNCS.has(whereClause.name)) {
-      return null
-    }
     // Recursively convert all arguments
     const args: Array<BasicExpression> = []
     for (const arg of whereClause.args) {
-      const convertedArg = convertToBasicExpression(
+      const convertedArg = normalizeExpressionPaths(
         arg as BasicExpression<boolean>,
         collectionAlias
       )
-      if (convertedArg == null) {
-        return null
-      }
       args.push(convertedArg)
     }
     return new Func(whereClause.name, args)
   }
 }
 
-export function convertOrderByToBasicExpression(
+export function normalizeOrderByPaths(
   orderBy: OrderBy,
   collectionAlias: string
 ): OrderBy {
   const normalizedOrderBy = orderBy.map((clause) => {
-    const basicExp = convertToBasicExpression(
+    const basicExp = normalizeExpressionPaths(
       clause.expression,
       collectionAlias
     )
 
-    if (!basicExp) {
-      throw new Error(
-        `Failed to convert orderBy expression to a basic expression: ${clause.expression}`
-      )
-    }
-
     return {
       ...clause,
       expression: basicExp,
diff --git a/packages/db/src/query/live/collection-subscriber.ts b/packages/db/src/query/live/collection-subscriber.ts
@@ -1,9 +1,8 @@
 import { MultiSet } from "@tanstack/db-ivm"
 import {
-  convertOrderByToBasicExpression,
-  convertToBasicExpression,
+  normalizeExpressionPaths,
+  normalizeOrderByPaths,
 } from "../compiler/expressions.js"
-import { WhereClauseConversionError } from "../../errors.js"
 import type { MultiSetArray, RootStreamBuilder } from "@tanstack/db-ivm"
 import type { Collection } from "../../collection/index.js"
 import type { ChangeMessage } from "../../types.js"
@@ -41,13 +40,8 @@ export class CollectionSubscriber<
     const whereClause = this.getWhereClauseForAlias()
 
     if (whereClause) {
-      const whereExpression = convertToBasicExpression(whereClause, this.alias)
-
-      if (whereExpression) {
-        return this.subscribeToChanges(whereExpression)
-      }
-
-      throw new WhereClauseConversionError(this.collectionId, this.alias)
+      const whereExpression = normalizeExpressionPaths(whereClause, this.alias)
+      return this.subscribeToChanges(whereExpression)
     }
 
     return this.subscribeToChanges()
@@ -199,10 +193,7 @@ export class CollectionSubscriber<
     subscription.setOrderByIndex(index)
 
     // Normalize the orderBy clauses such that the references are relative to the collection
-    const normalizedOrderBy = convertOrderByToBasicExpression(
-      orderBy,
-      this.alias
-    )
+    const normalizedOrderBy = normalizeOrderByPaths(orderBy, this.alias)
 
     // Load the first `offset + limit` values from the index
     // i.e. the K items from the collection that fall into the requested range: [offset, offset + limit[
@@ -289,10 +280,7 @@ export class CollectionSubscriber<
       : biggestSentRow
 
     // Normalize the orderBy clauses such that the references are relative to the collection
-    const normalizedOrderBy = convertOrderByToBasicExpression(
-      orderBy,
-      this.alias
-    )
+    const normalizedOrderBy = normalizeOrderByPaths(orderBy, this.alias)
 
     // Take the `n` items after the biggest sent value
     subscription.requestLimitedSnapshot({
diff --git a/packages/db/src/query/optimizer.ts b/packages/db/src/query/optimizer.ts
@@ -131,7 +131,6 @@ import {
   getWhereExpression,
   isResidualWhere,
 } from "./ir.js"
-import { isConvertibleToCollectionFilter } from "./compiler/expressions.js"
 import type { BasicExpression, From, QueryIR, Select, Where } from "./ir.js"
 
 /**
@@ -248,14 +247,10 @@ function extractSourceWhereClauses(
   const groupedClauses = groupWhereClauses(analyzedClauses)
 
   // Only include single-source clauses that reference collections directly
-  // and can be converted to BasicExpression format for collection indexes
   for (const [sourceAlias, whereClause] of groupedClauses.singleSource) {
     // Check if this source alias corresponds to a collection reference
     if (isCollectionReference(query, sourceAlias)) {
-      // Check if the WHERE clause can be converted to collection-compatible format
-      if (isConvertibleToCollectionFilter(whereClause)) {
-        sourceWhereClauses.set(sourceAlias, whereClause)
-      }
+      sourceWhereClauses.set(sourceAlias, whereClause)
     }
   }
 
diff --git a/packages/db/tests/query/live-query-collection.test.ts b/packages/db/tests/query/live-query-collection.test.ts
@@ -5,15 +5,17 @@ import {
   and,
   createLiveQueryCollection,
   eq,
+  ilike,
   liveQueryCollectionOptions,
 } from "../../src/query/index.js"
 import { Query } from "../../src/query/builder/index.js"
 import {
+  flushPromises,
   mockSyncCollectionOptions,
   mockSyncCollectionOptionsNoInitialState,
 } from "../utils.js"
 import { createDeferred } from "../../src/deferred"
-import type { ChangeMessage } from "../../src/types.js"
+import type { ChangeMessage, LoadSubsetOptions } from "../../src/types.js"
 
 // Sample user type for tests
 type User = {
@@ -1939,4 +1941,105 @@ describe(`createLiveQueryCollection`, () => {
       throw new Error(`Expected DuplicateKeySyncError to be thrown`)
     })
   })
+
+  describe(`where clauses passed to loadSubset`, () => {
+    it(`passes eq where clause to loadSubset`, async () => {
+      const capturedOptions: Array<LoadSubsetOptions> = []
+      let resolveLoadSubset: () => void
+      const loadSubsetPromise = new Promise<void>((resolve) => {
+        resolveLoadSubset = resolve
+      })
+
+      const baseCollection = createCollection<{ id: number; name: string }>({
+        id: `test-base`,
+        getKey: (item) => item.id,
+        syncMode: `on-demand`,
+        sync: {
+          sync: ({ markReady }) => {
+            markReady()
+            return {
+              loadSubset: (options: LoadSubsetOptions) => {
+                capturedOptions.push(options)
+                return loadSubsetPromise
+              },
+            }
+          },
+        },
+      })
+
+      // Create a live query collection with a where clause
+      // This will go through convertToBasicExpression
+      const liveQueryCollection = createLiveQueryCollection((q) =>
+        q.from({ item: baseCollection }).where(({ item }) => eq(item.id, 2))
+      )
+
+      // Trigger sync which will call loadSubset
+      await liveQueryCollection.preload()
+      await flushPromises()
+
+      expect(capturedOptions.length).toBeGreaterThan(0)
+      const lastCall = capturedOptions[capturedOptions.length - 1]
+      expect(lastCall?.where).toBeDefined()
+      // The where clause should be normalized (alias removed), so it should be eq(ref(['id']), 2)
+      expect(lastCall?.where?.type).toBe(`func`)
+      if (lastCall?.where?.type === `func`) {
+        expect(lastCall.where.name).toBe(`eq`)
+      }
+
+      resolveLoadSubset!()
+      await flushPromises()
+    })
+
+    it(`passes ilike where clause to loadSubset`, async () => {
+      const capturedOptions: Array<LoadSubsetOptions> = []
+      let resolveLoadSubset: () => void
+      const loadSubsetPromise = new Promise<void>((resolve) => {
+        resolveLoadSubset = resolve
+      })
+
+      const baseCollection = createCollection<{ id: number; name: string }>({
+        id: `test-base`,
+        getKey: (item) => item.id,
+        syncMode: `on-demand`,
+        sync: {
+          sync: ({ markReady }) => {
+            markReady()
+            return {
+              loadSubset: (options: LoadSubsetOptions) => {
+                capturedOptions.push(options)
+                return loadSubsetPromise
+              },
+            }
+          },
+        },
+      })
+
+      // Create a live query collection with an ilike where clause
+      // This will go through convertToBasicExpression
+      const liveQueryCollection = createLiveQueryCollection((q) =>
+        q
+          .from({ item: baseCollection })
+          .where(({ item }) => ilike(item.name, `%test%`))
+      )
+
+      // Trigger sync which will call loadSubset
+      await liveQueryCollection.preload()
+      await flushPromises()
+
+      expect(capturedOptions.length).toBeGreaterThan(0)
+      const lastCall = capturedOptions[capturedOptions.length - 1]
+      // Without the fix: where would be undefined/null
+      // With the fix: where should be defined with the ilike expression
+      expect(lastCall?.where).toBeDefined()
+      expect(lastCall?.where).not.toBeNull()
+      // The where clause should be normalized (alias removed), so it should be ilike(ref(['name']), '%test%')
+      expect(lastCall?.where?.type).toBe(`func`)
+      if (lastCall?.where?.type === `func`) {
+        expect(lastCall.where.name).toBe(`ilike`)
+      }
+
+      resolveLoadSubset!()
+      await flushPromises()
+    })
+  })
 })
diff --git a/packages/query-db-collection/tests/query.test.ts b/packages/query-db-collection/tests/query.test.ts

-Original file line number
+Diff line change
@@ @@ -0,0 +1,5 @@ @@
 +---
 +"@tanstack/db": patch
 +---
++
 +Pass all operators in where clauses to the collection's loadSubset function