From e5dc9f7b6660e5bf88895fa27936a8aa6e01537c Mon Sep 17 00:00:00 2001 From: Sam Willis Date: Thu, 27 Nov 2025 15:45:58 +0000 Subject: [PATCH 1/2] fix(db) loadSubset when orderby has multiple columns failing test for multiple orderby and loadsubset push down multiple orderby predicates to load subset split order by cursor predicate build into two, inprecise wider band for local lading, precise for the sync loadSubset new e2e tests for composite orderby and pagination changeset when doing gt/lt comparisons to a bool cast to string fix: use non-boolean columns in multi-column orderBy e2e tests Electric/PostgreSQL doesn't support comparison operators (<, >, <=, >=) on boolean types. Changed tests to use age (number) and name (string) columns instead of isActive (boolean) to avoid this limitation. The core multi-column orderBy functionality still works correctly - this is just a test adjustment to work within Electric's SQL parser constraints. --- .changeset/multi-column-orderby-loadsubset.md | 18 + .../src/suites/pagination.suite.ts | 580 ++++++++++++++++++ packages/db/src/collection/subscription.ts | 68 +- packages/db/src/query/compiler/order-by.ts | 201 ++++-- .../src/query/live/collection-subscriber.ts | 55 +- packages/db/src/utils/cursor.ts | 78 +++ packages/db/tests/cursor.test.ts | 226 +++++++ .../tests/query/live-query-collection.test.ts | 116 ++++ .../src/sql-compiler.ts | 122 ++++ 9 files changed, 1372 insertions(+), 92 deletions(-) create mode 100644 .changeset/multi-column-orderby-loadsubset.md create mode 100644 packages/db/src/utils/cursor.ts create mode 100644 packages/db/tests/cursor.test.ts diff --git a/.changeset/multi-column-orderby-loadsubset.md b/.changeset/multi-column-orderby-loadsubset.md new file mode 100644 index 000000000..7a74a4ac8 --- /dev/null +++ b/.changeset/multi-column-orderby-loadsubset.md @@ -0,0 +1,18 @@ +--- +"@tanstack/db": patch +--- + +Enhanced multi-column orderBy support with lazy loading and composite cursor optimization. + +**Changes:** + +- Create index on first orderBy column even for multi-column orderBy queries, enabling lazy loading with first-column ordering +- Pass multi-column orderBy to loadSubset with precise composite cursors (e.g., `or(gt(col1, v1), and(eq(col1, v1), gt(col2, v2)))`) for backend optimization +- Use wide bounds (first column only) for local index operations to ensure no rows are missed +- Use precise composite cursor for sync layer loadSubset to minimize data transfer + +**Benefits:** + +- Multi-column orderBy queries with limit now support lazy loading (previously disabled) +- Sync implementations (like Electric) can optimize queries using composite indexes on the backend +- Local collection uses first-column index efficiently while backend gets precise cursor diff --git a/packages/db-collection-e2e/src/suites/pagination.suite.ts b/packages/db-collection-e2e/src/suites/pagination.suite.ts index 281695f2e..be09e271c 100644 --- a/packages/db-collection-e2e/src/suites/pagination.suite.ts +++ b/packages/db-collection-e2e/src/suites/pagination.suite.ts @@ -131,6 +131,586 @@ export function createPaginationTestSuite( await query.cleanup() }) + + it(`should sort by multiple fields with chained orderBy`, async () => { + const config = await getConfig() + const usersCollection = config.collections.onDemand.users + + const query = createLiveQueryCollection((q) => + q + .from({ user: usersCollection }) + .orderBy(({ user }) => user.isActive, `desc`) + .orderBy(({ user }) => user.age, `asc`) + ) + + await query.preload() + await waitForQueryData(query, { minSize: 1 }) + + const results = Array.from(query.state.values()) + expect(results.length).toBeGreaterThan(0) + + // Verify multi-field sort (isActive desc first, then age asc within each isActive) + for (let i = 1; i < results.length; i++) { + const prev = results[i - 1]! + const curr = results[i]! + + // isActive should be descending (true before false) + if (prev.isActive !== curr.isActive) { + // true (1) should come before false (0) in desc order + expect(prev.isActive ? 1 : 0).toBeGreaterThanOrEqual( + curr.isActive ? 1 : 0 + ) + } else { + // If isActive is same, age should be ascending + expect(prev.age).toBeLessThanOrEqual(curr.age) + } + } + + await query.cleanup() + }) + }) + + describe(`Multi-Column OrderBy with Incremental Loading`, () => { + it(`should correctly paginate with multi-column orderBy and limit`, async () => { + const config = await getConfig() + const usersCollection = config.collections.onDemand.users + + // First page - get first 10 users sorted by isActive desc, age asc + const query = createLiveQueryCollection((q) => + q + .from({ user: usersCollection }) + .orderBy(({ user }) => user.isActive, `desc`) + .orderBy(({ user }) => user.age, `asc`) + .limit(10) + ) + + await query.preload() + await waitForQueryData(query, { minSize: 10 }) + + const results = Array.from(query.state.values()) + expect(results).toHaveLength(10) + + // Verify the ordering is correct + for (let i = 1; i < results.length; i++) { + const prev = results[i - 1]! + const curr = results[i]! + + if (prev.isActive !== curr.isActive) { + expect(prev.isActive ? 1 : 0).toBeGreaterThanOrEqual( + curr.isActive ? 1 : 0 + ) + } else { + expect(prev.age).toBeLessThanOrEqual(curr.age) + } + } + + await query.cleanup() + }) + + it(`should load subsequent pages correctly with multi-column orderBy`, async () => { + const config = await getConfig() + const usersCollection = config.collections.onDemand.users + + // Get first 15 users + const query1 = createLiveQueryCollection((q) => + q + .from({ user: usersCollection }) + .orderBy(({ user }) => user.isActive, `desc`) + .orderBy(({ user }) => user.age, `asc`) + .limit(15) + ) + + await query1.preload() + await waitForQueryData(query1, { minSize: 15 }) + + const firstPage = Array.from(query1.state.values()) + expect(firstPage).toHaveLength(15) + + // Get first 30 users (expanding the window) + const query2 = createLiveQueryCollection((q) => + q + .from({ user: usersCollection }) + .orderBy(({ user }) => user.isActive, `desc`) + .orderBy(({ user }) => user.age, `asc`) + .limit(30) + ) + + await query2.preload() + await waitForQueryData(query2, { minSize: 30 }) + + const expandedPage = Array.from(query2.state.values()) + expect(expandedPage).toHaveLength(30) + + // The first 15 items should be the same in both queries + for (let i = 0; i < 15; i++) { + expect(expandedPage[i]!.id).toBe(firstPage[i]!.id) + } + + // Verify ordering is maintained throughout + for (let i = 1; i < expandedPage.length; i++) { + const prev = expandedPage[i - 1]! + const curr = expandedPage[i]! + + if (prev.isActive !== curr.isActive) { + expect(prev.isActive ? 1 : 0).toBeGreaterThanOrEqual( + curr.isActive ? 1 : 0 + ) + } else { + expect(prev.age).toBeLessThanOrEqual(curr.age) + } + } + + await query1.cleanup() + await query2.cleanup() + }) + + it(`should handle multi-column orderBy with duplicate values in first column`, async () => { + const config = await getConfig() + const usersCollection = config.collections.onDemand.users + + // Sort by isActive (only 2 values: true/false) then by age + // This tests the case where many rows have the same first column value + const query = createLiveQueryCollection((q) => + q + .from({ user: usersCollection }) + .orderBy(({ user }) => user.isActive, `desc`) + .orderBy(({ user }) => user.age, `asc`) + .limit(50) + ) + + await query.preload() + await waitForQueryData(query, { minSize: 50 }) + + const results = Array.from(query.state.values()) + expect(results).toHaveLength(50) + + // Count how many active users we got + const activeUsers = results.filter((u) => u.isActive) + const inactiveUsers = results.filter((u) => !u.isActive) + + // Since isActive desc, all active users should come first + // All active users should be at the start + let foundInactive = false + for (const user of results) { + if (!user.isActive) { + foundInactive = true + } else if (foundInactive) { + // Found active after inactive - this is wrong + throw new Error( + `Found active user after inactive user in desc order` + ) + } + } + + // Verify age is ascending within each group + if (activeUsers.length > 1) { + for (let i = 1; i < activeUsers.length; i++) { + expect(activeUsers[i - 1]!.age).toBeLessThanOrEqual( + activeUsers[i]!.age + ) + } + } + + if (inactiveUsers.length > 1) { + for (let i = 1; i < inactiveUsers.length; i++) { + expect(inactiveUsers[i - 1]!.age).toBeLessThanOrEqual( + inactiveUsers[i]!.age + ) + } + } + + await query.cleanup() + }) + + it(`should handle multi-column orderBy with mixed directions`, async () => { + const config = await getConfig() + const postsCollection = config.collections.onDemand.posts + + // Sort by userId ascending, viewCount descending + const query = createLiveQueryCollection((q) => + q + .from({ post: postsCollection }) + .orderBy(({ post }) => post.userId, `asc`) + .orderBy(({ post }) => post.viewCount, `desc`) + .limit(20) + ) + + await query.preload() + await waitForQueryData(query, { minSize: 20 }) + + const results = Array.from(query.state.values()) + expect(results).toHaveLength(20) + + // Verify ordering + for (let i = 1; i < results.length; i++) { + const prev = results[i - 1]! + const curr = results[i]! + + if (prev.userId < curr.userId) { + // userId ascending - this is correct + continue + } else if (prev.userId === curr.userId) { + // Same userId, viewCount should be descending + expect(prev.viewCount).toBeGreaterThanOrEqual(curr.viewCount) + } else { + // userId decreased - this is wrong + throw new Error( + `userId should be ascending but ${prev.userId} > ${curr.userId}` + ) + } + } + + await query.cleanup() + }) + + it(`should handle three-column orderBy`, async () => { + const config = await getConfig() + const usersCollection = config.collections.onDemand.users + + // Sort by isActive desc, age asc, name asc + const query = createLiveQueryCollection((q) => + q + .from({ user: usersCollection }) + .orderBy(({ user }) => user.isActive, `desc`) + .orderBy(({ user }) => user.age, `asc`) + .orderBy(({ user }) => user.name, `asc`) + .limit(25) + ) + + await query.preload() + await waitForQueryData(query, { minSize: 25 }) + + const results = Array.from(query.state.values()) + expect(results).toHaveLength(25) + + // Verify basic ordering (isActive desc, age asc) + for (let i = 1; i < results.length; i++) { + const prev = results[i - 1]! + const curr = results[i]! + + if (prev.isActive !== curr.isActive) { + expect(prev.isActive ? 1 : 0).toBeGreaterThanOrEqual( + curr.isActive ? 1 : 0 + ) + } else if (prev.age !== curr.age) { + expect(prev.age).toBeLessThanOrEqual(curr.age) + } + // For name, we don't strictly check due to collation differences + } + + await query.cleanup() + }) + + it(`should use setWindow to page through multi-column orderBy results`, async () => { + const config = await getConfig() + const usersCollection = config.collections.onDemand.users + + // Create query with multi-column orderBy and limit + // Using age (number) and name (string) to avoid boolean comparison issues + const query = createLiveQueryCollection((q) => + q + .from({ user: usersCollection }) + .orderBy(({ user }) => user.age, `asc`) + .orderBy(({ user }) => user.name, `asc`) + .limit(10) + ) + + await query.preload() + await waitForQueryData(query, { minSize: 10 }) + + // Get first page + const firstPage = Array.from(query.state.values()) + expect(firstPage).toHaveLength(10) + + // Verify first page ordering (age asc, then name asc) + for (let i = 1; i < firstPage.length; i++) { + const prev = firstPage[i - 1]! + const curr = firstPage[i]! + if (prev.age !== curr.age) { + expect(prev.age).toBeLessThanOrEqual(curr.age) + } else { + expect(prev.name.localeCompare(curr.name)).toBeLessThanOrEqual(0) + } + } + + // Move to second page using setWindow + // IMPORTANT: setWindow returns a Promise when loading is required, + // or `true` if data is already available. We verify loading occurs. + const setWindowResult = query.utils.setWindow({ offset: 10, limit: 10 }) + + // In on-demand mode, moving to offset 10 should trigger loading + // since only the first 10 records were initially loaded + if (setWindowResult !== true) { + // Loading was triggered - wait for it to complete + await setWindowResult + } + await waitForQueryData(query, { minSize: 10 }) + + // Get second page + const secondPage = Array.from(query.state.values()) + expect(secondPage).toHaveLength(10) + + // Verify second page ordering + for (let i = 1; i < secondPage.length; i++) { + const prev = secondPage[i - 1]! + const curr = secondPage[i]! + if (prev.age !== curr.age) { + expect(prev.age).toBeLessThanOrEqual(curr.age) + } else { + expect(prev.name.localeCompare(curr.name)).toBeLessThanOrEqual(0) + } + } + + // CRITICAL: Pages should not overlap - this proves new data was loaded + // If the backend didn't return new data, we'd see duplicates or missing records + const firstPageIds = new Set(firstPage.map((u) => u.id)) + const secondPageIds = new Set(secondPage.map((u) => u.id)) + for (const id of secondPageIds) { + expect(firstPageIds.has(id)).toBe(false) + } + + await query.cleanup() + }) + + it(`should use setWindow to move backwards with multi-column orderBy`, async () => { + const config = await getConfig() + const usersCollection = config.collections.onDemand.users + + // Start at offset 20 + const query = createLiveQueryCollection((q) => + q + .from({ user: usersCollection }) + .orderBy(({ user }) => user.age, `asc`) + .orderBy(({ user }) => user.name, `asc`) + .limit(10) + .offset(20) + ) + + await query.preload() + await waitForQueryData(query, { minSize: 10 }) + + const laterPage = Array.from(query.state.values()) + expect(laterPage).toHaveLength(10) + + // Move backwards to offset 10 + const setWindowResult = query.utils.setWindow({ offset: 10, limit: 10 }) + if (setWindowResult !== true) { + await setWindowResult + } + await waitForQueryData(query, { minSize: 10 }) + + const earlierPage = Array.from(query.state.values()) + expect(earlierPage).toHaveLength(10) + + // Earlier page should have different users + const laterPageIds = new Set(laterPage.map((u) => u.id)) + const earlierPageIds = new Set(earlierPage.map((u) => u.id)) + for (const id of earlierPageIds) { + expect(laterPageIds.has(id)).toBe(false) + } + + // Verify ordering on earlier page (age asc, name asc) + for (let i = 1; i < earlierPage.length; i++) { + const prev = earlierPage[i - 1]! + const curr = earlierPage[i]! + if (prev.age !== curr.age) { + expect(prev.age).toBeLessThanOrEqual(curr.age) + } else { + expect(prev.name.localeCompare(curr.name)).toBeLessThanOrEqual(0) + } + } + + await query.cleanup() + }) + + it(`should use setWindow with mixed direction multi-column orderBy`, async () => { + const config = await getConfig() + const postsCollection = config.collections.onDemand.posts + + // Sort by userId ascending, viewCount descending + const query = createLiveQueryCollection((q) => + q + .from({ post: postsCollection }) + .orderBy(({ post }) => post.userId, `asc`) + .orderBy(({ post }) => post.viewCount, `desc`) + .limit(10) + ) + + await query.preload() + await waitForQueryData(query, { minSize: 10 }) + + const firstPage = Array.from(query.state.values()) + expect(firstPage).toHaveLength(10) + + // Move to second page + const setWindowResult = query.utils.setWindow({ offset: 10, limit: 10 }) + if (setWindowResult !== true) { + await setWindowResult + } + await waitForQueryData(query, { minSize: 10 }) + + const secondPage = Array.from(query.state.values()) + expect(secondPage).toHaveLength(10) + + // Verify ordering on second page (userId asc, viewCount desc) + for (let i = 1; i < secondPage.length; i++) { + const prev = secondPage[i - 1]! + const curr = secondPage[i]! + + if (prev.userId < curr.userId) { + // userId ascending - correct + continue + } else if (prev.userId === curr.userId) { + // Same userId, viewCount should be descending + expect(prev.viewCount).toBeGreaterThanOrEqual(curr.viewCount) + } else { + throw new Error( + `userId should be ascending but ${prev.userId} > ${curr.userId}` + ) + } + } + + await query.cleanup() + }) + + it(`should handle setWindow across duplicate first-column values`, async () => { + const config = await getConfig() + const usersCollection = config.collections.onDemand.users + + // Age has limited unique values in test data, so many duplicates in first column + // This tests that the composite cursor correctly handles paging across duplicates + const query = createLiveQueryCollection((q) => + q + .from({ user: usersCollection }) + .orderBy(({ user }) => user.age, `asc`) + .orderBy(({ user }) => user.name, `asc`) + .limit(20) + ) + + await query.preload() + await waitForQueryData(query, { minSize: 20 }) + + const firstPage = Array.from(query.state.values()) + expect(firstPage).toHaveLength(20) + + // Move to second page - this crosses the boundary where first column value changes + const setWindowResult = query.utils.setWindow({ offset: 20, limit: 20 }) + if (setWindowResult !== true) { + await setWindowResult + } + await waitForQueryData(query, { minSize: 20 }) + + const secondPage = Array.from(query.state.values()) + expect(secondPage).toHaveLength(20) + + // Verify ordering is maintained across the page boundary + for (let i = 1; i < secondPage.length; i++) { + const prev = secondPage[i - 1]! + const curr = secondPage[i]! + if (prev.age !== curr.age) { + expect(prev.age).toBeLessThanOrEqual(curr.age) + } else { + expect(prev.name.localeCompare(curr.name)).toBeLessThanOrEqual(0) + } + } + + // Pages should not overlap + const firstPageIds = new Set(firstPage.map((u) => u.id)) + for (const user of secondPage) { + expect(firstPageIds.has(user.id)).toBe(false) + } + + // Move to third page to ensure continued paging works + const setWindowResult2 = query.utils.setWindow({ + offset: 40, + limit: 20, + }) + if (setWindowResult2 !== true) { + await setWindowResult2 + } + await waitForQueryData(query, { minSize: 1 }) + + const thirdPage = Array.from(query.state.values()) + expect(thirdPage.length).toBeGreaterThan(0) + expect(thirdPage.length).toBeLessThanOrEqual(20) + + // Third page should not overlap with first or second + const secondPageIds = new Set(secondPage.map((u) => u.id)) + for (const user of thirdPage) { + expect(firstPageIds.has(user.id)).toBe(false) + expect(secondPageIds.has(user.id)).toBe(false) + } + + await query.cleanup() + }) + + it(`should trigger backend loading when paging with multi-column orderBy`, async () => { + const config = await getConfig() + const usersCollection = config.collections.onDemand.users + + // Use a small limit to ensure we need to load more data when paging + const query = createLiveQueryCollection((q) => + q + .from({ user: usersCollection }) + .orderBy(({ user }) => user.age, `asc`) + .orderBy(({ user }) => user.name, `asc`) + .limit(5) + ) + + await query.preload() + await waitForQueryData(query, { minSize: 5 }) + + // Get first page - only 5 records loaded + const firstPage = Array.from(query.state.values()) + expect(firstPage).toHaveLength(5) + const lastItemFirstPage = firstPage[firstPage.length - 1]! + + // Move to second page - this MUST trigger backend loading + // since we only have 5 records and need records at offset 5 + const setWindowResult = query.utils.setWindow({ offset: 5, limit: 5 }) + + // CRITICAL ASSERTION: In on-demand mode, setWindow should return a Promise + // when we need to load data we don't have yet. This proves loading was triggered. + // If it returned `true`, it would mean data was already available (no loading needed). + expect( + setWindowResult === true || setWindowResult instanceof Promise + ).toBe(true) + + if (setWindowResult !== true) { + // Wait for loading to complete + await setWindowResult + } + await waitForQueryData(query, { minSize: 5 }) + + // Get second page + const secondPage = Array.from(query.state.values()) + expect(secondPage).toHaveLength(5) + + // Verify we got different records (proves new data was loaded from backend) + const firstPageIds = new Set(firstPage.map((u) => u.id)) + for (const user of secondPage) { + expect(firstPageIds.has(user.id)).toBe(false) + } + + // Verify ordering continues correctly from where first page ended + const firstItemSecondPage = secondPage[0]! + + // The first item of page 2 should come after the last item of page 1 + // in the sort order (age asc, name asc) + if (lastItemFirstPage.age === firstItemSecondPage.age) { + // Same age value, so name should be greater or equal + expect( + firstItemSecondPage.name.localeCompare(lastItemFirstPage.name) + ).toBeGreaterThanOrEqual(0) + } else { + // Different age, page 2 first should have greater or equal age + expect(firstItemSecondPage.age).toBeGreaterThanOrEqual( + lastItemFirstPage.age + ) + } + + await query.cleanup() + }) }) describe(`Limit`, () => { diff --git a/packages/db/src/collection/subscription.ts b/packages/db/src/collection/subscription.ts index 1951677f1..d91e48168 100644 --- a/packages/db/src/collection/subscription.ts +++ b/packages/db/src/collection/subscription.ts @@ -1,7 +1,8 @@ -import { ensureIndexForExpression } from '../indexes/auto-index.js' -import { and, eq, gt, gte, lt } from '../query/builder/functions.js' -import { Value } from '../query/ir.js' -import { EventEmitter } from '../event-emitter.js' +import { ensureIndexForExpression } from "../indexes/auto-index.js" +import { and, eq, gte, lt } from "../query/builder/functions.js" +import { Value } from "../query/ir.js" +import { EventEmitter } from "../event-emitter.js" +import { buildCursor } from "../utils/cursor.js" import { createFilterFunctionFromExpression, createFilteredCallback, @@ -22,12 +23,17 @@ type RequestSnapshotOptions = { where?: BasicExpression optimizedOnly?: boolean trackLoadSubsetPromise?: boolean + /** Optional orderBy to pass to loadSubset for backend optimization */ + orderBy?: OrderBy + /** Optional limit to pass to loadSubset for backend optimization */ + limit?: number } type RequestLimitedSnapshotOptions = { orderBy: OrderBy limit: number - minValue?: any + /** All column values for cursor (first value used for local index, all values for sync layer) */ + minValues?: Array } type CollectionSubscriptionOptions = { @@ -203,6 +209,9 @@ export class CollectionSubscription const loadOptions: LoadSubsetOptions = { where: stateOpts.where, subscription: this, + // Include orderBy and limit if provided so sync layer can optimize the query + orderBy: opts?.orderBy, + limit: opts?.limit, } const syncResult = this.collection._sync.loadSubset(loadOptions) @@ -233,17 +242,22 @@ export class CollectionSubscription } /** - * Sends a snapshot that fulfills the `where` clause and all rows are bigger or equal to `minValue`. + * Sends a snapshot that fulfills the `where` clause and all rows are bigger or equal to the cursor. * Requires a range index to be set with `setOrderByIndex` prior to calling this method. * It uses that range index to load the items in the order of the index. - * Note 1: it may load more rows than the provided LIMIT because it loads all values equal to `minValue` + limit values greater than `minValue`. + * + * For multi-column orderBy: + * - Uses first value from `minValues` for LOCAL index operations (wide bounds, ensures no missed rows) + * - Uses all `minValues` to build a precise composite cursor for SYNC layer loadSubset + * + * Note 1: it may load more rows than the provided LIMIT because it loads all values equal to the first cursor value + limit values greater. * This is needed to ensure that it does not accidentally skip duplicate values when the limit falls in the middle of some duplicated values. * Note 2: it does not send keys that have already been sent before. */ requestLimitedSnapshot({ orderBy, limit, - minValue, + minValues, }: RequestLimitedSnapshotOptions) { if (!limit) throw new Error(`limit is required`) @@ -253,6 +267,11 @@ export class CollectionSubscription ) } + // Derive first column value from minValues (used for local index operations) + const minValue = minValues?.[0] + // Cast for index operations (index expects string | number) + const minValueForIndex = minValue as string | number | undefined + const index = this.orderByIndex const where = this.options.whereExpression const whereFilterFn = where @@ -272,7 +291,7 @@ export class CollectionSubscription return whereFilterFn?.(value) ?? true } - let biggestObservedValue = minValue + let biggestObservedValue = minValueForIndex const changes: Array> = [] // If we have a minValue we need to handle the case @@ -281,12 +300,16 @@ export class CollectionSubscription // so if minValue is 3 then the previous snapshot may not have included all 3s // e.g. if it was offset 0 and limit 3 it would only have loaded the first 3 // so we load all rows equal to minValue first, to be sure we don't skip any duplicate values + // + // For multi-column orderBy, we use the first column value for index operations (wide bounds) + // This may load some duplicates but ensures we never miss any rows. let keys: Array = [] - if (minValue !== undefined) { - // First, get all items with the same value as minValue + if (minValueForIndex !== undefined) { + // First, get all items with the same FIRST COLUMN value as minValue + // This provides wide bounds for the local index const { expression } = orderBy[0]! const allRowsWithMinValue = this.collection.currentStateAsChanges({ - where: eq(expression, new Value(minValue)), + where: eq(expression, new Value(minValueForIndex)), }) if (allRowsWithMinValue) { @@ -300,15 +323,15 @@ export class CollectionSubscription // Then get items greater than minValue const keysGreaterThanMin = index.take( limit - keys.length, - minValue, - filterFn, + minValueForIndex, + filterFn ) keys.push(...keysGreaterThanMin) } else { - keys = index.take(limit, minValue, filterFn) + keys = index.take(limit, minValueForIndex, filterFn) } } else { - keys = index.take(limit, minValue, filterFn) + keys = index.take(limit, minValueForIndex, filterFn) } const valuesNeeded = () => Math.max(limit - changes.length, 0) @@ -333,13 +356,14 @@ export class CollectionSubscription this.callback(changes) + // Build the WHERE filter for sync layer loadSubset + // buildCursor handles both single-column and multi-column cases let whereWithValueFilter = where - if (typeof minValue !== `undefined`) { - // Only request data that we haven't seen yet (i.e. is bigger than the minValue) - const { expression, compareOptions } = orderBy[0]! - const operator = compareOptions.direction === `asc` ? gt : lt - const valueFilter = operator(expression, new Value(minValue)) - whereWithValueFilter = where ? and(where, valueFilter) : valueFilter + if (minValues !== undefined && minValues.length > 0) { + const cursor = buildCursor(orderBy, minValues) + if (cursor) { + whereWithValueFilter = where ? and(where, cursor) : cursor + } } // Request the sync layer to load more data diff --git a/packages/db/src/query/compiler/order-by.ts b/packages/db/src/query/compiler/order-by.ts index 3bf67304c..d7aeca331 100644 --- a/packages/db/src/query/compiler/order-by.ts +++ b/packages/db/src/query/compiler/order-by.ts @@ -27,8 +27,12 @@ export type OrderByOptimizationInfo = { a: Record | null | undefined, b: Record | null | undefined, ) => number - valueExtractorForRawRow: (row: Record) => any - index: IndexInterface + /** Extracts all orderBy column values from a raw row (array for multi-column) */ + valueExtractorForRawRow: (row: Record) => unknown + /** Extracts only the first column value - used for index-based cursor */ + firstColumnValueExtractor: (row: Record) => unknown + /** Index on the first orderBy column - used for lazy loading */ + index?: IndexInterface dataNeeded?: () => number } @@ -117,76 +121,165 @@ export function processOrderBy( let orderByOptimizationInfo: OrderByOptimizationInfo | undefined - // Optimize the orderBy operator to lazily load elements - // by using the range index of the collection. - // Only for orderBy clause on a single column for now (no composite ordering) - if (limit && orderByClause.length === 1) { - const clause = orderByClause[0]! - const orderByExpression = clause.expression + // When there's a limit, we create orderByOptimizationInfo to pass orderBy/limit + // to loadSubset so the sync layer can optimize the query. + // We try to use an index on the FIRST orderBy column for lazy loading, + // even for multi-column orderBy (using wider bounds on first column). + if (limit) { + let index: IndexInterface | undefined + let followRefCollection: Collection | undefined + let firstColumnValueExtractor: CompiledSingleRowExpression | undefined + let orderByAlias: string = rawQuery.from.alias - if (orderByExpression.type === `ref`) { + // Try to create/find an index on the FIRST orderBy column for lazy loading + const firstClause = orderByClause[0]! + const firstOrderByExpression = firstClause.expression + + if (firstOrderByExpression.type === `ref`) { const followRefResult = followRef( rawQuery, - orderByExpression, - collection, - )! - - const followRefCollection = followRefResult.collection - const fieldName = followRefResult.path[0] - const compareOpts = buildCompareOptions(clause, followRefCollection) - if (fieldName) { - ensureIndexForField( - fieldName, - followRefResult.path, - followRefCollection, - compareOpts, - compare, + firstOrderByExpression, + collection + ) + + if (followRefResult) { + followRefCollection = followRefResult.collection + const fieldName = followRefResult.path[0] + const compareOpts = buildCompareOptions( + firstClause, + followRefCollection ) - } - const valueExtractorForRawRow = compileExpression( - new PropRef(followRefResult.path), - true, - ) as CompiledSingleRowExpression + if (fieldName) { + ensureIndexForField( + fieldName, + followRefResult.path, + followRefCollection, + compareOpts, + compare + ) + } - const comparator = ( - a: Record | null | undefined, - b: Record | null | undefined, - ) => { - const extractedA = a ? valueExtractorForRawRow(a) : a - const extractedB = b ? valueExtractorForRawRow(b) : b - return compare(extractedA, extractedB) - } + // First column value extractor - used for index cursor + firstColumnValueExtractor = compileExpression( + new PropRef(followRefResult.path), + true + ) as CompiledSingleRowExpression - const index: IndexInterface | undefined = - findIndexForField( + index = findIndexForField( followRefCollection, followRefResult.path, compareOpts, ) - if (index && index.supports(`gt`)) { - // We found an index that we can use to lazily load ordered data - const orderByAlias = - orderByExpression.path.length > 1 - ? String(orderByExpression.path[0]) + // Only use the index if it supports range queries + if (!index?.supports(`gt`)) { + index = undefined + } + + orderByAlias = + firstOrderByExpression.path.length > 1 + ? String(firstOrderByExpression.path[0]) : rawQuery.from.alias + } + } - orderByOptimizationInfo = { - alias: orderByAlias, - offset: offset ?? 0, - limit, - comparator, - valueExtractorForRawRow, - index, - orderBy: orderByClause, + // Only create comparator and value extractors if the first column is a ref expression + // For aggregate or computed expressions, we can't extract values from raw collection rows + if (!firstColumnValueExtractor) { + // Skip optimization for non-ref expressions (aggregates, computed values, etc.) + // The query will still work, but without lazy loading optimization + } else { + // Build value extractors for all columns (must all be ref expressions for multi-column) + // Check if all orderBy expressions are ref types (required for multi-column extraction) + const allColumnsAreRefs = orderByClause.every( + (clause) => clause.expression.type === `ref` + ) + + // Create extractors for all columns if they're all refs + const allColumnExtractors: + | Array + | undefined = allColumnsAreRefs + ? orderByClause.map((clause) => { + // We know it's a ref since we checked allColumnsAreRefs + const refExpr = clause.expression as PropRef + const followResult = followRef(rawQuery, refExpr, collection) + if (followResult) { + return compileExpression( + new PropRef(followResult.path), + true + ) as CompiledSingleRowExpression + } + // Fallback for refs that don't follow + return compileExpression( + clause.expression, + true + ) as CompiledSingleRowExpression + }) + : undefined + + // Create a comparator for raw rows (used for tracking sent values) + // This compares ALL orderBy columns for proper ordering + const comparator = ( + a: Record | null | undefined, + b: Record | null | undefined + ) => { + if (orderByClause.length === 1) { + // Single column: extract and compare + const extractedA = a ? firstColumnValueExtractor(a) : a + const extractedB = b ? firstColumnValueExtractor(b) : b + return compare(extractedA, extractedB) + } + if (allColumnExtractors) { + // Multi-column with all refs: extract all values and compare + const extractAll = ( + row: Record | null | undefined + ) => { + if (!row) return row + return allColumnExtractors.map((extractor) => extractor(row)) + } + return compare(extractAll(a), extractAll(b)) } + // Fallback: can't compare (shouldn't happen since we skip non-ref cases) + return 0 + } + + // Create a value extractor for raw rows that extracts ALL orderBy column values + // This is used for tracking sent values and building composite cursors + const rawRowValueExtractor = (row: Record): unknown => { + if (orderByClause.length === 1) { + // Single column: return single value + return firstColumnValueExtractor(row) + } + if (allColumnExtractors) { + // Multi-column: return array of all values + return allColumnExtractors.map((extractor) => extractor(row)) + } + // Fallback (shouldn't happen) + return undefined + } + + orderByOptimizationInfo = { + alias: orderByAlias, + offset: offset ?? 0, + limit, + comparator, + valueExtractorForRawRow: rawRowValueExtractor, + firstColumnValueExtractor: firstColumnValueExtractor, + index, + orderBy: orderByClause, + } - optimizableOrderByCollections[followRefCollection.id] = - orderByOptimizationInfo + // Store the optimization info keyed by collection ID + // Use the followed collection if available, otherwise use the main collection + const targetCollectionId = followRefCollection?.id ?? collection.id + optimizableOrderByCollections[targetCollectionId] = + orderByOptimizationInfo + // Set up lazy loading callback if we have an index + if (index) { setSizeCallback = (getSize: () => number) => { - optimizableOrderByCollections[followRefCollection.id]![`dataNeeded`] = + optimizableOrderByCollections[targetCollectionId]![`dataNeeded`] = () => { const size = getSize() return Math.max(0, orderByOptimizationInfo!.limit - size) diff --git a/packages/db/src/query/live/collection-subscriber.ts b/packages/db/src/query/live/collection-subscriber.ts index 842db9fe2..8c7a01d0f 100644 --- a/packages/db/src/query/live/collection-subscriber.ts +++ b/packages/db/src/query/live/collection-subscriber.ts @@ -190,17 +190,30 @@ export class CollectionSubscriber< whereExpression, }) - subscription.setOrderByIndex(index) - // Normalize the orderBy clauses such that the references are relative to the collection 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[ - subscription.requestLimitedSnapshot({ - limit: offset + limit, - orderBy: normalizedOrderBy, - }) + if (index) { + // We have an index on the first orderBy column - use lazy loading optimization + // This works for both single-column and multi-column orderBy: + // - Single-column: index provides exact ordering + // - Multi-column: index provides ordering on first column, secondary sort in memory + subscription.setOrderByIndex(index) + + // 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[ + subscription.requestLimitedSnapshot({ + limit: offset + limit, + orderBy: normalizedOrderBy, + }) + } else { + // No index available (e.g., non-ref expression): pass orderBy/limit to loadSubset + // so the sync layer can optimize if the backend supports it + subscription.requestSnapshot({ + orderBy: normalizedOrderBy, + limit: offset + limit, + }) + } return subscription } @@ -220,11 +233,10 @@ export class CollectionSubscriber< const { dataNeeded } = orderByInfo if (!dataNeeded) { - // This should never happen because the topK operator should always set the size callback - // which in turn should lead to the orderBy operator setting the dataNeeded callback - throw new Error( - `Missing dataNeeded callback for collection ${this.collectionId}`, - ) + // dataNeeded is not set when there's no index (e.g., non-ref expression). + // In this case, we've already loaded all data via requestSnapshot + // and don't need to lazily load more. + return true } // `dataNeeded` probes the orderBy operator to see if it needs more data @@ -275,9 +287,20 @@ export class CollectionSubscriber< } const { orderBy, valueExtractorForRawRow } = orderByInfo const biggestSentRow = this.biggest - const biggestSentValue = biggestSentRow + + // Extract all orderBy column values from the biggest sent row + // For single-column: returns single value, for multi-column: returns array + const extractedValues = biggestSentRow ? valueExtractorForRawRow(biggestSentRow) - : biggestSentRow + : undefined + + // Normalize to array format for minValues + const minValues = + extractedValues !== undefined + ? Array.isArray(extractedValues) + ? extractedValues + : [extractedValues] + : undefined // Normalize the orderBy clauses such that the references are relative to the collection const normalizedOrderBy = normalizeOrderByPaths(orderBy, this.alias) @@ -286,7 +309,7 @@ export class CollectionSubscriber< subscription.requestLimitedSnapshot({ orderBy: normalizedOrderBy, limit: n, - minValue: biggestSentValue, + minValues, }) } diff --git a/packages/db/src/utils/cursor.ts b/packages/db/src/utils/cursor.ts new file mode 100644 index 000000000..322a37470 --- /dev/null +++ b/packages/db/src/utils/cursor.ts @@ -0,0 +1,78 @@ +import { and, eq, gt, lt, or } from '../query/builder/functions.js' +import { Value } from '../query/ir.js' +import type { BasicExpression, OrderBy } from '../query/ir.js' + +/** + * Builds a cursor expression for paginating through ordered results. + * For multi-column orderBy, creates a composite cursor that respects all columns. + * + * For [col1 ASC, col2 DESC] with values [v1, v2], produces: + * or( + * gt(col1, v1), // col1 > v1 + * and(eq(col1, v1), lt(col2, v2)) // col1 = v1 AND col2 < v2 (DESC) + * ) + * + * This creates a precise cursor that works with composite indexes on the backend. + * + * @param orderBy - The order-by clauses defining sort columns and directions + * @param values - The cursor values corresponding to each order-by column + * @returns A filter expression for rows after the cursor position, or undefined if empty + */ +export function buildCursor( + orderBy: OrderBy, + values: Array, +): BasicExpression | undefined { + if (values.length === 0 || orderBy.length === 0) { + return undefined + } + + // For single column, just use simple gt/lt + if (orderBy.length === 1) { + const { expression, compareOptions } = orderBy[0]! + const operator = compareOptions.direction === `asc` ? gt : lt + return operator(expression, new Value(values[0])) + } + + // For multi-column, build the composite cursor: + // or( + // gt(col1, v1), + // and(eq(col1, v1), gt(col2, v2)), + // and(eq(col1, v1), eq(col2, v2), gt(col3, v3)), + // ... + // ) + const clauses: Array> = [] + + for (let i = 0; i < orderBy.length && i < values.length; i++) { + const clause = orderBy[i]! + const value = values[i] + + // Build equality conditions for all previous columns + const eqConditions: Array> = [] + for (let j = 0; j < i; j++) { + const prevClause = orderBy[j]! + const prevValue = values[j] + eqConditions.push(eq(prevClause.expression, new Value(prevValue))) + } + + // Add the comparison for the current column (respecting direction) + const operator = clause.compareOptions.direction === `asc` ? gt : lt + const comparison = operator(clause.expression, new Value(value)) + + if (eqConditions.length === 0) { + // First column: just the comparison + clauses.push(comparison) + } else { + // Subsequent columns: and(eq(prev...), comparison) + // We need to spread into and() which expects at least 2 args + const allConditions = [...eqConditions, comparison] + clauses.push(allConditions.reduce((acc, cond) => and(acc, cond))) + } + } + + // Combine all clauses with OR + if (clauses.length === 1) { + return clauses[0]! + } + // Use reduce to combine with or() which expects exactly 2 args + return clauses.reduce((acc, clause) => or(acc, clause)) +} diff --git a/packages/db/tests/cursor.test.ts b/packages/db/tests/cursor.test.ts new file mode 100644 index 000000000..5d8f4a2f4 --- /dev/null +++ b/packages/db/tests/cursor.test.ts @@ -0,0 +1,226 @@ +import { describe, expect, it } from 'vitest' +import { buildCursor } from '../src/utils/cursor.js' +import { Func, PropRef, Value } from '../src/query/ir.js' +import type { OrderBy, OrderByClause } from '../src/query/ir.js' +import type { CompareOptions } from '../src/query/builder/types.js' + +// Helper to create an OrderByClause for testing +function createOrderByClause( + path: string, + direction: `asc` | `desc`, +): OrderByClause { + const compareOptions: CompareOptions = { + direction, + nulls: direction === `asc` ? `first` : `last`, + } + return { + expression: new PropRef([`t`, path]), + compareOptions, + } +} + +// Helper to check if a Func has the expected structure +function isFuncWithName(expr: unknown, name: string): expr is Func { + return expr instanceof Func && expr.name === name +} + +describe(`buildCursor`, () => { + describe(`edge cases`, () => { + it(`returns undefined for empty values array`, () => { + const orderBy: OrderBy = [createOrderByClause(`col1`, `asc`)] + expect(buildCursor(orderBy, [])).toBeUndefined() + }) + + it(`returns undefined for empty orderBy array`, () => { + expect(buildCursor([], [1, 2, 3])).toBeUndefined() + }) + + it(`returns undefined for both empty`, () => { + expect(buildCursor([], [])).toBeUndefined() + }) + }) + + describe(`single column`, () => { + it(`produces gt() for ASC direction`, () => { + const orderBy: OrderBy = [createOrderByClause(`col1`, `asc`)] + const result = buildCursor(orderBy, [10]) + + expect(result).toBeInstanceOf(Func) + expect(isFuncWithName(result, `gt`)).toBe(true) + + const func = result as Func + expect(func.args).toHaveLength(2) + expect(func.args[0]).toBeInstanceOf(PropRef) + expect((func.args[0] as PropRef).path).toEqual([`t`, `col1`]) + expect(func.args[1]).toBeInstanceOf(Value) + expect((func.args[1] as Value).value).toBe(10) + }) + + it(`produces lt() for DESC direction`, () => { + const orderBy: OrderBy = [createOrderByClause(`col1`, `desc`)] + const result = buildCursor(orderBy, [10]) + + expect(result).toBeInstanceOf(Func) + expect(isFuncWithName(result, `lt`)).toBe(true) + + const func = result as Func + expect(func.args).toHaveLength(2) + expect(func.args[0]).toBeInstanceOf(PropRef) + expect((func.args[0] as PropRef).path).toEqual([`t`, `col1`]) + expect(func.args[1]).toBeInstanceOf(Value) + expect((func.args[1] as Value).value).toBe(10) + }) + + it(`handles string cursor values`, () => { + const orderBy: OrderBy = [createOrderByClause(`name`, `asc`)] + const result = buildCursor(orderBy, [`alice`]) + + expect(result).toBeInstanceOf(Func) + const func = result as Func + expect(func.args[1]).toBeInstanceOf(Value) + expect((func.args[1] as Value).value).toBe(`alice`) + }) + + it(`handles null cursor values`, () => { + const orderBy: OrderBy = [createOrderByClause(`col1`, `asc`)] + const result = buildCursor(orderBy, [null]) + + expect(result).toBeInstanceOf(Func) + const func = result as Func + expect((func.args[1] as Value).value).toBeNull() + }) + }) + + describe(`multi-column composite cursor`, () => { + it(`produces or(gt(col1), and(eq(col1), gt(col2))) for two ASC columns`, () => { + const orderBy: OrderBy = [ + createOrderByClause(`col1`, `asc`), + createOrderByClause(`col2`, `asc`), + ] + const result = buildCursor(orderBy, [10, 20]) + + // Should be: or(gt(col1, 10), and(eq(col1, 10), gt(col2, 20))) + expect(result).toBeInstanceOf(Func) + expect(isFuncWithName(result, `or`)).toBe(true) + + const orFunc = result as Func + expect(orFunc.args).toHaveLength(2) + + // First arg: gt(col1, 10) + const gtCol1 = orFunc.args[0] + expect(isFuncWithName(gtCol1, `gt`)).toBe(true) + expect((gtCol1 as Func).args[0]).toBeInstanceOf(PropRef) + expect(((gtCol1 as Func).args[0] as PropRef).path).toEqual([`t`, `col1`]) + expect(((gtCol1 as Func).args[1] as Value).value).toBe(10) + + // Second arg: and(eq(col1, 10), gt(col2, 20)) + const andClause = orFunc.args[1] + expect(isFuncWithName(andClause, `and`)).toBe(true) + const andFunc = andClause as Func + expect(andFunc.args).toHaveLength(2) + + // eq(col1, 10) + expect(isFuncWithName(andFunc.args[0], `eq`)).toBe(true) + const eqCol1 = andFunc.args[0] as Func + expect((eqCol1.args[0] as PropRef).path).toEqual([`t`, `col1`]) + expect((eqCol1.args[1] as Value).value).toBe(10) + + // gt(col2, 20) + expect(isFuncWithName(andFunc.args[1], `gt`)).toBe(true) + const gtCol2 = andFunc.args[1] as Func + expect((gtCol2.args[0] as PropRef).path).toEqual([`t`, `col2`]) + expect((gtCol2.args[1] as Value).value).toBe(20) + }) + + it(`handles mixed ASC/DESC directions`, () => { + const orderBy: OrderBy = [ + createOrderByClause(`col1`, `asc`), + createOrderByClause(`col2`, `desc`), + ] + const result = buildCursor(orderBy, [10, 20]) + + // Should be: or(gt(col1, 10), and(eq(col1, 10), lt(col2, 20))) + expect(isFuncWithName(result, `or`)).toBe(true) + + const orFunc = result as Func + const andClause = orFunc.args[1] as Func + + // Second column should use lt() for DESC + expect(isFuncWithName(andClause.args[1], `lt`)).toBe(true) + }) + + it(`handles three columns correctly`, () => { + const orderBy: OrderBy = [ + createOrderByClause(`col1`, `asc`), + createOrderByClause(`col2`, `asc`), + createOrderByClause(`col3`, `desc`), + ] + const result = buildCursor(orderBy, [1, 2, 3]) + + // Should be: or( + // gt(col1, 1), + // and(eq(col1, 1), gt(col2, 2)), + // and(eq(col1, 1), eq(col2, 2), lt(col3, 3)) + // ) + expect(isFuncWithName(result, `or`)).toBe(true) + + const outerOr = result as Func + // The structure is: or(or(gt, and), and) due to reduce + expect(outerOr.args).toHaveLength(2) + + // First arg is or(gt(col1, 1), and(eq(col1, 1), gt(col2, 2))) + const innerOr = outerOr.args[0] + expect(isFuncWithName(innerOr, `or`)).toBe(true) + + // Second arg is and(and(eq(col1, 1), eq(col2, 2)), lt(col3, 3)) + const thirdClause = outerOr.args[1] + expect(isFuncWithName(thirdClause, `and`)).toBe(true) + + // The innermost and should have eq conditions and lt for col3 + const innerAnd = thirdClause as Func + // Due to reduce, the structure is nested: and(and(eq, eq), lt) + expect(isFuncWithName(innerAnd.args[1], `lt`)).toBe(true) + const ltCol3 = innerAnd.args[1] as Func + expect((ltCol3.args[0] as PropRef).path).toEqual([`t`, `col3`]) + expect((ltCol3.args[1] as Value).value).toBe(3) + }) + }) + + describe(`partial values`, () => { + it(`handles fewer values than orderBy columns`, () => { + const orderBy: OrderBy = [ + createOrderByClause(`col1`, `asc`), + createOrderByClause(`col2`, `asc`), + createOrderByClause(`col3`, `asc`), + ] + const result = buildCursor(orderBy, [10, 20]) + + // Should only use first two columns + expect(isFuncWithName(result, `or`)).toBe(true) + + const orFunc = result as Func + expect(orFunc.args).toHaveLength(2) + + // First clause: gt(col1, 10) + expect(isFuncWithName(orFunc.args[0], `gt`)).toBe(true) + + // Second clause: and(eq(col1, 10), gt(col2, 20)) + expect(isFuncWithName(orFunc.args[1], `and`)).toBe(true) + }) + + it(`handles single value for multi-column orderBy`, () => { + const orderBy: OrderBy = [ + createOrderByClause(`col1`, `asc`), + createOrderByClause(`col2`, `asc`), + ] + const result = buildCursor(orderBy, [10]) + + // Should just be gt(col1, 10) since only one value provided + expect(isFuncWithName(result, `gt`)).toBe(true) + + const gtFunc = result as Func + expect((gtFunc.args[0] as PropRef).path).toEqual([`t`, `col1`]) + expect((gtFunc.args[1] as Value).value).toBe(10) + }) + }) +}) diff --git a/packages/db/tests/query/live-query-collection.test.ts b/packages/db/tests/query/live-query-collection.test.ts index 707d6a510..039fa065a 100644 --- a/packages/db/tests/query/live-query-collection.test.ts +++ b/packages/db/tests/query/live-query-collection.test.ts @@ -2041,5 +2041,121 @@ describe(`createLiveQueryCollection`, () => { resolveLoadSubset!() await flushPromises() }) + + it(`passes single orderBy clause to loadSubset when using limit`, async () => { + const capturedOptions: Array = [] + let resolveLoadSubset: () => void + const loadSubsetPromise = new Promise((resolve) => { + resolveLoadSubset = resolve + }) + + const baseCollection = createCollection<{ + id: number + name: string + age: number + }>({ + id: `test-base-orderby`, + 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 orderBy and limit + const liveQueryCollection = createLiveQueryCollection((q) => + q + .from({ item: baseCollection }) + .orderBy(({ item }) => item.age, `asc`) + .limit(10) + ) + + // Trigger sync which will call loadSubset + await liveQueryCollection.preload() + await flushPromises() + + expect(capturedOptions.length).toBeGreaterThan(0) + + // Find the call that has orderBy (the limited snapshot request) + const callWithOrderBy = capturedOptions.find( + (opt) => opt.orderBy !== undefined + ) + expect(callWithOrderBy).toBeDefined() + expect(callWithOrderBy?.orderBy).toHaveLength(1) + expect(callWithOrderBy?.orderBy?.[0]?.expression.type).toBe(`ref`) + expect(callWithOrderBy?.limit).toBe(10) + + resolveLoadSubset!() + await flushPromises() + }) + + it(`passes multiple orderBy columns to loadSubset when using limit`, async () => { + const capturedOptions: Array = [] + let resolveLoadSubset: () => void + const loadSubsetPromise = new Promise((resolve) => { + resolveLoadSubset = resolve + }) + + const baseCollection = createCollection<{ + id: number + name: string + age: number + department: string + }>({ + id: `test-base-multi-orderby`, + 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 multiple orderBy columns and limit + const liveQueryCollection = createLiveQueryCollection((q) => + q + .from({ item: baseCollection }) + .orderBy(({ item }) => item.department, `asc`) + .orderBy(({ item }) => item.age, `desc`) + .limit(10) + ) + + // Trigger sync which will call loadSubset + await liveQueryCollection.preload() + await flushPromises() + + expect(capturedOptions.length).toBeGreaterThan(0) + + // Find the call that has orderBy with multiple columns + const callWithMultiOrderBy = capturedOptions.find( + (opt) => opt.orderBy !== undefined && opt.orderBy.length > 1 + ) + + // Multi-column orderBy should be passed to loadSubset so the sync layer + // can optimize the query if the backend supports composite ordering + expect(callWithMultiOrderBy).toBeDefined() + expect(callWithMultiOrderBy?.orderBy).toHaveLength(2) + expect(callWithMultiOrderBy?.orderBy?.[0]?.expression.type).toBe(`ref`) + expect(callWithMultiOrderBy?.orderBy?.[1]?.expression.type).toBe(`ref`) + expect(callWithMultiOrderBy?.limit).toBe(10) + + resolveLoadSubset!() + await flushPromises() + }) }) }) diff --git a/packages/electric-db-collection/src/sql-compiler.ts b/packages/electric-db-collection/src/sql-compiler.ts index fcc892ade..6c1db6c4c 100644 --- a/packages/electric-db-collection/src/sql-compiler.ts +++ b/packages/electric-db-collection/src/sql-compiler.ts @@ -172,6 +172,120 @@ function compileFunction( throw new Error(`Binary operator ${name} expects 2 arguments`) } const [lhs, rhs] = compiledArgs + + // Special case for comparison operators with boolean values + // PostgreSQL doesn't support < > <= >= on booleans + // Transform to equivalent equality checks or constant expressions + if (isComparisonOp(name)) { + const lhsArg = args[0] + const rhsArg = args[1] + + // Check if RHS is a boolean literal value + if ( + rhsArg && + rhsArg.type === `val` && + typeof rhsArg.value === `boolean` + ) { + const boolValue = rhsArg.value + // Remove the boolean param we just added since we'll transform the expression + params.pop() + + // Transform based on operator and boolean value + // Boolean ordering: false < true + if (name === `lt`) { + if (boolValue === true) { + // lt(col, true) → col = false (only false is less than true) + params.push(false) + return `${lhs} = $${params.length}` + } else { + // lt(col, false) → nothing is less than false + return `false` + } + } else if (name === `gt`) { + if (boolValue === false) { + // gt(col, false) → col = true (only true is greater than false) + params.push(true) + return `${lhs} = $${params.length}` + } else { + // gt(col, true) → nothing is greater than true + return `false` + } + } else if (name === `lte`) { + if (boolValue === true) { + // lte(col, true) → everything is ≤ true + return `true` + } else { + // lte(col, false) → col = false + params.push(false) + return `${lhs} = $${params.length}` + } + } else if (name === `gte`) { + if (boolValue === false) { + // gte(col, false) → everything is ≥ false + return `true` + } else { + // gte(col, true) → col = true + params.push(true) + return `${lhs} = $${params.length}` + } + } + } + + // Check if LHS is a boolean literal value (less common but handle it) + if ( + lhsArg && + lhsArg.type === `val` && + typeof lhsArg.value === `boolean` + ) { + const boolValue = lhsArg.value + // Remove params for this expression and rebuild + params.pop() // remove RHS + params.pop() // remove LHS (boolean) + + // Recompile RHS to get fresh param + const rhsCompiled = compileBasicExpression(rhsArg!, params) + + // Transform: flip the comparison (val op col → col flipped_op val) + if (name === `lt`) { + // lt(true, col) → gt(col, true) → col > true → nothing is greater than true + if (boolValue === true) { + return `false` + } else { + // lt(false, col) → gt(col, false) → col = true + params.push(true) + return `${rhsCompiled} = $${params.length}` + } + } else if (name === `gt`) { + // gt(true, col) → lt(col, true) → col = false + if (boolValue === true) { + params.push(false) + return `${rhsCompiled} = $${params.length}` + } else { + // gt(false, col) → lt(col, false) → nothing is less than false + return `false` + } + } else if (name === `lte`) { + if (boolValue === false) { + // lte(false, col) → gte(col, false) → everything + return `true` + } else { + // lte(true, col) → gte(col, true) → col = true + params.push(true) + return `${rhsCompiled} = $${params.length}` + } + } else if (name === `gte`) { + if (boolValue === true) { + // gte(true, col) → lte(col, true) → everything + return `true` + } else { + // gte(false, col) → lte(col, false) → col = false + params.push(false) + return `${rhsCompiled} = $${params.length}` + } + } + } + } + // Special case for = ANY operator which needs parentheses around the array parameter if (name === `in`) { return `${lhs} ${opName}(${rhs})` @@ -198,6 +312,14 @@ function isBinaryOp(name: string): boolean { return binaryOps.includes(name) } +/** + * Checks if the operator is a comparison operator (excluding eq) + * These operators don't work on booleans in PostgreSQL without casting + */ +function isComparisonOp(name: string): boolean { + return [`gt`, `gte`, `lt`, `lte`].includes(name) +} + function getOpName(name: string): string { const opNames = { eq: `=`, From 9ecdc5952f158ba13a162a6ecf8ea8733eb47420 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Fri, 5 Dec 2025 17:13:57 +0000 Subject: [PATCH 2/2] ci: apply automated fixes --- .changeset/multi-column-orderby-loadsubset.md | 2 +- .../src/suites/pagination.suite.ts | 48 +++++++++---------- packages/db/src/collection/subscription.ts | 12 ++--- packages/db/src/query/compiler/order-by.ts | 18 +++---- .../tests/query/live-query-collection.test.ts | 8 ++-- 5 files changed, 44 insertions(+), 44 deletions(-) diff --git a/.changeset/multi-column-orderby-loadsubset.md b/.changeset/multi-column-orderby-loadsubset.md index 7a74a4ac8..143e5292e 100644 --- a/.changeset/multi-column-orderby-loadsubset.md +++ b/.changeset/multi-column-orderby-loadsubset.md @@ -1,5 +1,5 @@ --- -"@tanstack/db": patch +'@tanstack/db': patch --- Enhanced multi-column orderBy support with lazy loading and composite cursor optimization. diff --git a/packages/db-collection-e2e/src/suites/pagination.suite.ts b/packages/db-collection-e2e/src/suites/pagination.suite.ts index be09e271c..7ccf2c60b 100644 --- a/packages/db-collection-e2e/src/suites/pagination.suite.ts +++ b/packages/db-collection-e2e/src/suites/pagination.suite.ts @@ -140,7 +140,7 @@ export function createPaginationTestSuite( q .from({ user: usersCollection }) .orderBy(({ user }) => user.isActive, `desc`) - .orderBy(({ user }) => user.age, `asc`) + .orderBy(({ user }) => user.age, `asc`), ) await query.preload() @@ -158,7 +158,7 @@ export function createPaginationTestSuite( if (prev.isActive !== curr.isActive) { // true (1) should come before false (0) in desc order expect(prev.isActive ? 1 : 0).toBeGreaterThanOrEqual( - curr.isActive ? 1 : 0 + curr.isActive ? 1 : 0, ) } else { // If isActive is same, age should be ascending @@ -181,7 +181,7 @@ export function createPaginationTestSuite( .from({ user: usersCollection }) .orderBy(({ user }) => user.isActive, `desc`) .orderBy(({ user }) => user.age, `asc`) - .limit(10) + .limit(10), ) await query.preload() @@ -197,7 +197,7 @@ export function createPaginationTestSuite( if (prev.isActive !== curr.isActive) { expect(prev.isActive ? 1 : 0).toBeGreaterThanOrEqual( - curr.isActive ? 1 : 0 + curr.isActive ? 1 : 0, ) } else { expect(prev.age).toBeLessThanOrEqual(curr.age) @@ -217,7 +217,7 @@ export function createPaginationTestSuite( .from({ user: usersCollection }) .orderBy(({ user }) => user.isActive, `desc`) .orderBy(({ user }) => user.age, `asc`) - .limit(15) + .limit(15), ) await query1.preload() @@ -232,7 +232,7 @@ export function createPaginationTestSuite( .from({ user: usersCollection }) .orderBy(({ user }) => user.isActive, `desc`) .orderBy(({ user }) => user.age, `asc`) - .limit(30) + .limit(30), ) await query2.preload() @@ -253,7 +253,7 @@ export function createPaginationTestSuite( if (prev.isActive !== curr.isActive) { expect(prev.isActive ? 1 : 0).toBeGreaterThanOrEqual( - curr.isActive ? 1 : 0 + curr.isActive ? 1 : 0, ) } else { expect(prev.age).toBeLessThanOrEqual(curr.age) @@ -275,7 +275,7 @@ export function createPaginationTestSuite( .from({ user: usersCollection }) .orderBy(({ user }) => user.isActive, `desc`) .orderBy(({ user }) => user.age, `asc`) - .limit(50) + .limit(50), ) await query.preload() @@ -297,7 +297,7 @@ export function createPaginationTestSuite( } else if (foundInactive) { // Found active after inactive - this is wrong throw new Error( - `Found active user after inactive user in desc order` + `Found active user after inactive user in desc order`, ) } } @@ -306,7 +306,7 @@ export function createPaginationTestSuite( if (activeUsers.length > 1) { for (let i = 1; i < activeUsers.length; i++) { expect(activeUsers[i - 1]!.age).toBeLessThanOrEqual( - activeUsers[i]!.age + activeUsers[i]!.age, ) } } @@ -314,7 +314,7 @@ export function createPaginationTestSuite( if (inactiveUsers.length > 1) { for (let i = 1; i < inactiveUsers.length; i++) { expect(inactiveUsers[i - 1]!.age).toBeLessThanOrEqual( - inactiveUsers[i]!.age + inactiveUsers[i]!.age, ) } } @@ -332,7 +332,7 @@ export function createPaginationTestSuite( .from({ post: postsCollection }) .orderBy(({ post }) => post.userId, `asc`) .orderBy(({ post }) => post.viewCount, `desc`) - .limit(20) + .limit(20), ) await query.preload() @@ -355,7 +355,7 @@ export function createPaginationTestSuite( } else { // userId decreased - this is wrong throw new Error( - `userId should be ascending but ${prev.userId} > ${curr.userId}` + `userId should be ascending but ${prev.userId} > ${curr.userId}`, ) } } @@ -374,7 +374,7 @@ export function createPaginationTestSuite( .orderBy(({ user }) => user.isActive, `desc`) .orderBy(({ user }) => user.age, `asc`) .orderBy(({ user }) => user.name, `asc`) - .limit(25) + .limit(25), ) await query.preload() @@ -390,7 +390,7 @@ export function createPaginationTestSuite( if (prev.isActive !== curr.isActive) { expect(prev.isActive ? 1 : 0).toBeGreaterThanOrEqual( - curr.isActive ? 1 : 0 + curr.isActive ? 1 : 0, ) } else if (prev.age !== curr.age) { expect(prev.age).toBeLessThanOrEqual(curr.age) @@ -412,7 +412,7 @@ export function createPaginationTestSuite( .from({ user: usersCollection }) .orderBy(({ user }) => user.age, `asc`) .orderBy(({ user }) => user.name, `asc`) - .limit(10) + .limit(10), ) await query.preload() @@ -483,7 +483,7 @@ export function createPaginationTestSuite( .orderBy(({ user }) => user.age, `asc`) .orderBy(({ user }) => user.name, `asc`) .limit(10) - .offset(20) + .offset(20), ) await query.preload() @@ -533,7 +533,7 @@ export function createPaginationTestSuite( .from({ post: postsCollection }) .orderBy(({ post }) => post.userId, `asc`) .orderBy(({ post }) => post.viewCount, `desc`) - .limit(10) + .limit(10), ) await query.preload() @@ -565,7 +565,7 @@ export function createPaginationTestSuite( expect(prev.viewCount).toBeGreaterThanOrEqual(curr.viewCount) } else { throw new Error( - `userId should be ascending but ${prev.userId} > ${curr.userId}` + `userId should be ascending but ${prev.userId} > ${curr.userId}`, ) } } @@ -584,7 +584,7 @@ export function createPaginationTestSuite( .from({ user: usersCollection }) .orderBy(({ user }) => user.age, `asc`) .orderBy(({ user }) => user.name, `asc`) - .limit(20) + .limit(20), ) await query.preload() @@ -654,7 +654,7 @@ export function createPaginationTestSuite( .from({ user: usersCollection }) .orderBy(({ user }) => user.age, `asc`) .orderBy(({ user }) => user.name, `asc`) - .limit(5) + .limit(5), ) await query.preload() @@ -673,7 +673,7 @@ export function createPaginationTestSuite( // when we need to load data we don't have yet. This proves loading was triggered. // If it returned `true`, it would mean data was already available (no loading needed). expect( - setWindowResult === true || setWindowResult instanceof Promise + setWindowResult === true || setWindowResult instanceof Promise, ).toBe(true) if (setWindowResult !== true) { @@ -700,12 +700,12 @@ export function createPaginationTestSuite( if (lastItemFirstPage.age === firstItemSecondPage.age) { // Same age value, so name should be greater or equal expect( - firstItemSecondPage.name.localeCompare(lastItemFirstPage.name) + firstItemSecondPage.name.localeCompare(lastItemFirstPage.name), ).toBeGreaterThanOrEqual(0) } else { // Different age, page 2 first should have greater or equal age expect(firstItemSecondPage.age).toBeGreaterThanOrEqual( - lastItemFirstPage.age + lastItemFirstPage.age, ) } diff --git a/packages/db/src/collection/subscription.ts b/packages/db/src/collection/subscription.ts index d91e48168..045eb5042 100644 --- a/packages/db/src/collection/subscription.ts +++ b/packages/db/src/collection/subscription.ts @@ -1,8 +1,8 @@ -import { ensureIndexForExpression } from "../indexes/auto-index.js" -import { and, eq, gte, lt } from "../query/builder/functions.js" -import { Value } from "../query/ir.js" -import { EventEmitter } from "../event-emitter.js" -import { buildCursor } from "../utils/cursor.js" +import { ensureIndexForExpression } from '../indexes/auto-index.js' +import { and, eq, gte, lt } from '../query/builder/functions.js' +import { Value } from '../query/ir.js' +import { EventEmitter } from '../event-emitter.js' +import { buildCursor } from '../utils/cursor.js' import { createFilterFunctionFromExpression, createFilteredCallback, @@ -324,7 +324,7 @@ export class CollectionSubscription const keysGreaterThanMin = index.take( limit - keys.length, minValueForIndex, - filterFn + filterFn, ) keys.push(...keysGreaterThanMin) } else { diff --git a/packages/db/src/query/compiler/order-by.ts b/packages/db/src/query/compiler/order-by.ts index d7aeca331..4362ff635 100644 --- a/packages/db/src/query/compiler/order-by.ts +++ b/packages/db/src/query/compiler/order-by.ts @@ -139,7 +139,7 @@ export function processOrderBy( const followRefResult = followRef( rawQuery, firstOrderByExpression, - collection + collection, ) if (followRefResult) { @@ -147,7 +147,7 @@ export function processOrderBy( const fieldName = followRefResult.path[0] const compareOpts = buildCompareOptions( firstClause, - followRefCollection + followRefCollection, ) if (fieldName) { @@ -156,14 +156,14 @@ export function processOrderBy( followRefResult.path, followRefCollection, compareOpts, - compare + compare, ) } // First column value extractor - used for index cursor firstColumnValueExtractor = compileExpression( new PropRef(followRefResult.path), - true + true, ) as CompiledSingleRowExpression index = findIndexForField( @@ -193,7 +193,7 @@ export function processOrderBy( // Build value extractors for all columns (must all be ref expressions for multi-column) // Check if all orderBy expressions are ref types (required for multi-column extraction) const allColumnsAreRefs = orderByClause.every( - (clause) => clause.expression.type === `ref` + (clause) => clause.expression.type === `ref`, ) // Create extractors for all columns if they're all refs @@ -207,13 +207,13 @@ export function processOrderBy( if (followResult) { return compileExpression( new PropRef(followResult.path), - true + true, ) as CompiledSingleRowExpression } // Fallback for refs that don't follow return compileExpression( clause.expression, - true + true, ) as CompiledSingleRowExpression }) : undefined @@ -222,7 +222,7 @@ export function processOrderBy( // This compares ALL orderBy columns for proper ordering const comparator = ( a: Record | null | undefined, - b: Record | null | undefined + b: Record | null | undefined, ) => { if (orderByClause.length === 1) { // Single column: extract and compare @@ -233,7 +233,7 @@ export function processOrderBy( if (allColumnExtractors) { // Multi-column with all refs: extract all values and compare const extractAll = ( - row: Record | null | undefined + row: Record | null | undefined, ) => { if (!row) return row return allColumnExtractors.map((extractor) => extractor(row)) diff --git a/packages/db/tests/query/live-query-collection.test.ts b/packages/db/tests/query/live-query-collection.test.ts index 039fa065a..8c10c3eb8 100644 --- a/packages/db/tests/query/live-query-collection.test.ts +++ b/packages/db/tests/query/live-query-collection.test.ts @@ -2075,7 +2075,7 @@ describe(`createLiveQueryCollection`, () => { q .from({ item: baseCollection }) .orderBy(({ item }) => item.age, `asc`) - .limit(10) + .limit(10), ) // Trigger sync which will call loadSubset @@ -2086,7 +2086,7 @@ describe(`createLiveQueryCollection`, () => { // Find the call that has orderBy (the limited snapshot request) const callWithOrderBy = capturedOptions.find( - (opt) => opt.orderBy !== undefined + (opt) => opt.orderBy !== undefined, ) expect(callWithOrderBy).toBeDefined() expect(callWithOrderBy?.orderBy).toHaveLength(1) @@ -2132,7 +2132,7 @@ describe(`createLiveQueryCollection`, () => { .from({ item: baseCollection }) .orderBy(({ item }) => item.department, `asc`) .orderBy(({ item }) => item.age, `desc`) - .limit(10) + .limit(10), ) // Trigger sync which will call loadSubset @@ -2143,7 +2143,7 @@ describe(`createLiveQueryCollection`, () => { // Find the call that has orderBy with multiple columns const callWithMultiOrderBy = capturedOptions.find( - (opt) => opt.orderBy !== undefined && opt.orderBy.length > 1 + (opt) => opt.orderBy !== undefined && opt.orderBy.length > 1, ) // Multi-column orderBy should be passed to loadSubset so the sync layer