From 57007e62696bc808946fd96d49eb7f71cde9540c Mon Sep 17 00:00:00 2001 From: Sam Willis Date: Fri, 3 Oct 2025 19:37:56 +0100 Subject: [PATCH] fix orderBy field alias bug --- .changeset/good-papers-knock.md | 5 ++ packages/db/src/query/compiler/group-by.ts | 11 +--- packages/db/src/query/compiler/order-by.ts | 18 ++--- packages/db/tests/query/order-by.test.ts | 77 ++++++++++++++++++++++ 4 files changed, 89 insertions(+), 22 deletions(-) create mode 100644 .changeset/good-papers-knock.md diff --git a/.changeset/good-papers-knock.md b/.changeset/good-papers-knock.md new file mode 100644 index 000000000..056f2c7ba --- /dev/null +++ b/.changeset/good-papers-knock.md @@ -0,0 +1,5 @@ +--- +"@tanstack/db": patch +--- + +Fixed bug where orderBy would fail when a collection alias had the same name as one of its schema fields. For example, .from({ email: emailCollection }).orderBy(({ email }) => email.createdAt) now works correctly even when the collection has an email field in its schema. diff --git a/packages/db/src/query/compiler/group-by.ts b/packages/db/src/query/compiler/group-by.ts index e59ccfe91..c37520b76 100644 --- a/packages/db/src/query/compiler/group-by.ts +++ b/packages/db/src/query/compiler/group-by.ts @@ -417,16 +417,7 @@ export function replaceAggregatesByRefs( } case `ref`: { - const refExpr = havingExpr - // Check if this is a direct reference to a SELECT alias - if (refExpr.path.length === 1) { - const alias = refExpr.path[0]! - if (selectClause[alias]) { - // This is a reference to a SELECT alias, convert to result.alias - return new PropRef([resultAlias, alias]) - } - } - // Return as-is for other refs + // Non-aggregate refs are passed through unchanged (they reference table columns) return havingExpr as BasicExpression } diff --git a/packages/db/src/query/compiler/order-by.ts b/packages/db/src/query/compiler/order-by.ts index 777b21049..d5a78342d 100644 --- a/packages/db/src/query/compiler/order-by.ts +++ b/packages/db/src/query/compiler/order-by.ts @@ -54,18 +54,12 @@ export function processOrderBy( // Create a value extractor function for the orderBy operator const valueExtractor = (row: NamespacedRow & { __select_results?: any }) => { - // For ORDER BY expressions, we need to provide access to both: - // 1. The original namespaced row data (for direct table column references) - // 2. The __select_results (for SELECT alias references) - - // Create a merged context for expression evaluation - const orderByContext = { ...row } - - // If there are select results, merge them at the top level for alias access - if (row.__select_results) { - // Add select results as top-level properties for alias access - Object.assign(orderByContext, row.__select_results) - } + // The namespaced row contains: + // 1. Table aliases as top-level properties (e.g., row["tableName"]) + // 2. SELECT results in __select_results (e.g., row.__select_results["aggregateAlias"]) + // The replaceAggregatesByRefs function has already transformed any aggregate expressions + // that match SELECT aggregates to use the __select_results namespace. + const orderByContext = row if (orderByClause.length > 1) { // For multiple orderBy columns, create a composite key diff --git a/packages/db/tests/query/order-by.test.ts b/packages/db/tests/query/order-by.test.ts index f5ea8263a..9059cf5d9 100644 --- a/packages/db/tests/query/order-by.test.ts +++ b/packages/db/tests/query/order-by.test.ts @@ -2172,3 +2172,80 @@ describe(`Query2 OrderBy Compiler`, () => { createOrderByTests(`off`) createOrderByTests(`eager`) }) + +describe(`OrderBy with collection alias conflicts`, () => { + type EmailSchema = { + email: string + createdAt: Date + } + + const date1 = new Date(`2024-01-01`) + const date2 = new Date(`2024-01-02`) + const date3 = new Date(`2024-01-03`) + + const emailCollection = createCollection({ + ...mockSyncCollectionOptions({ + id: `emails`, + getKey: (item) => item.email, + initialData: [ + { email: `first@test.com`, createdAt: date1 }, + { email: `second@test.com`, createdAt: date2 }, + { email: `third@test.com`, createdAt: date3 }, + ], + }), + }) + + it(`should work when alias does not conflict with field name`, () => { + // This should work fine - alias "t" doesn't conflict with any field + const liveCollection = createLiveQueryCollection({ + startSync: true, + query: (q) => + q.from({ t: emailCollection }).orderBy(({ t }) => t.createdAt, `desc`), + }) + + const result = liveCollection.toArray + + expect(result).toHaveLength(3) + expect(result[0]?.email).toBe(`third@test.com`) + expect(result[1]?.email).toBe(`second@test.com`) + expect(result[2]?.email).toBe(`first@test.com`) + }) + + it(`should work when alias DOES conflict with field name`, () => { + // This breaks - alias "email" conflicts with field "email" + const liveCollection = createLiveQueryCollection({ + startSync: true, + query: (q) => + q + .from({ email: emailCollection }) + .orderBy(({ email }) => email.createdAt, `desc`), + }) + + const result = liveCollection.toArray + + expect(result).toHaveLength(3) + // The sorting should work - most recent first + expect(result[0]?.email).toBe(`third@test.com`) + expect(result[1]?.email).toBe(`second@test.com`) + expect(result[2]?.email).toBe(`first@test.com`) + }) + + it(`should also work for createdAt alias conflict`, () => { + // This should also work - alias "createdAt" conflicts with field "createdAt" + const liveCollection = createLiveQueryCollection({ + startSync: true, + query: (q) => + q + .from({ createdAt: emailCollection }) + .orderBy(({ createdAt }) => createdAt.email, `asc`), + }) + + const result = liveCollection.toArray as Array + + expect(result).toHaveLength(3) + // The sorting should work - alphabetically by email + expect(result[0]?.email).toBe(`first@test.com`) + expect(result[1]?.email).toBe(`second@test.com`) + expect(result[2]?.email).toBe(`third@test.com`) + }) +})