Skip to content

Commit f623990

Browse files
authored
fix orderBy field alias bug (#637)
1 parent c4c2399 commit f623990

File tree

4 files changed

+89
-22
lines changed

4 files changed

+89
-22
lines changed

.changeset/good-papers-knock.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@tanstack/db": patch
3+
---
4+
5+
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.

packages/db/src/query/compiler/group-by.ts

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -417,16 +417,7 @@ export function replaceAggregatesByRefs(
417417
}
418418

419419
case `ref`: {
420-
const refExpr = havingExpr
421-
// Check if this is a direct reference to a SELECT alias
422-
if (refExpr.path.length === 1) {
423-
const alias = refExpr.path[0]!
424-
if (selectClause[alias]) {
425-
// This is a reference to a SELECT alias, convert to result.alias
426-
return new PropRef([resultAlias, alias])
427-
}
428-
}
429-
// Return as-is for other refs
420+
// Non-aggregate refs are passed through unchanged (they reference table columns)
430421
return havingExpr as BasicExpression
431422
}
432423

packages/db/src/query/compiler/order-by.ts

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -54,18 +54,12 @@ export function processOrderBy(
5454

5555
// Create a value extractor function for the orderBy operator
5656
const valueExtractor = (row: NamespacedRow & { __select_results?: any }) => {
57-
// For ORDER BY expressions, we need to provide access to both:
58-
// 1. The original namespaced row data (for direct table column references)
59-
// 2. The __select_results (for SELECT alias references)
60-
61-
// Create a merged context for expression evaluation
62-
const orderByContext = { ...row }
63-
64-
// If there are select results, merge them at the top level for alias access
65-
if (row.__select_results) {
66-
// Add select results as top-level properties for alias access
67-
Object.assign(orderByContext, row.__select_results)
68-
}
57+
// The namespaced row contains:
58+
// 1. Table aliases as top-level properties (e.g., row["tableName"])
59+
// 2. SELECT results in __select_results (e.g., row.__select_results["aggregateAlias"])
60+
// The replaceAggregatesByRefs function has already transformed any aggregate expressions
61+
// that match SELECT aggregates to use the __select_results namespace.
62+
const orderByContext = row
6963

7064
if (orderByClause.length > 1) {
7165
// For multiple orderBy columns, create a composite key

packages/db/tests/query/order-by.test.ts

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2172,3 +2172,80 @@ describe(`Query2 OrderBy Compiler`, () => {
21722172
createOrderByTests(`off`)
21732173
createOrderByTests(`eager`)
21742174
})
2175+
2176+
describe(`OrderBy with collection alias conflicts`, () => {
2177+
type EmailSchema = {
2178+
email: string
2179+
createdAt: Date
2180+
}
2181+
2182+
const date1 = new Date(`2024-01-01`)
2183+
const date2 = new Date(`2024-01-02`)
2184+
const date3 = new Date(`2024-01-03`)
2185+
2186+
const emailCollection = createCollection<EmailSchema>({
2187+
...mockSyncCollectionOptions({
2188+
id: `emails`,
2189+
getKey: (item) => item.email,
2190+
initialData: [
2191+
{ email: `[email protected]`, createdAt: date1 },
2192+
{ email: `[email protected]`, createdAt: date2 },
2193+
{ email: `[email protected]`, createdAt: date3 },
2194+
],
2195+
}),
2196+
})
2197+
2198+
it(`should work when alias does not conflict with field name`, () => {
2199+
// This should work fine - alias "t" doesn't conflict with any field
2200+
const liveCollection = createLiveQueryCollection({
2201+
startSync: true,
2202+
query: (q) =>
2203+
q.from({ t: emailCollection }).orderBy(({ t }) => t.createdAt, `desc`),
2204+
})
2205+
2206+
const result = liveCollection.toArray
2207+
2208+
expect(result).toHaveLength(3)
2209+
expect(result[0]?.email).toBe(`[email protected]`)
2210+
expect(result[1]?.email).toBe(`[email protected]`)
2211+
expect(result[2]?.email).toBe(`[email protected]`)
2212+
})
2213+
2214+
it(`should work when alias DOES conflict with field name`, () => {
2215+
// This breaks - alias "email" conflicts with field "email"
2216+
const liveCollection = createLiveQueryCollection({
2217+
startSync: true,
2218+
query: (q) =>
2219+
q
2220+
.from({ email: emailCollection })
2221+
.orderBy(({ email }) => email.createdAt, `desc`),
2222+
})
2223+
2224+
const result = liveCollection.toArray
2225+
2226+
expect(result).toHaveLength(3)
2227+
// The sorting should work - most recent first
2228+
expect(result[0]?.email).toBe(`[email protected]`)
2229+
expect(result[1]?.email).toBe(`[email protected]`)
2230+
expect(result[2]?.email).toBe(`[email protected]`)
2231+
})
2232+
2233+
it(`should also work for createdAt alias conflict`, () => {
2234+
// This should also work - alias "createdAt" conflicts with field "createdAt"
2235+
const liveCollection = createLiveQueryCollection({
2236+
startSync: true,
2237+
query: (q) =>
2238+
q
2239+
.from({ createdAt: emailCollection })
2240+
.orderBy(({ createdAt }) => createdAt.email, `asc`),
2241+
})
2242+
2243+
const result = liveCollection.toArray as Array<EmailSchema>
2244+
2245+
expect(result).toHaveLength(3)
2246+
// The sorting should work - alphabetically by email
2247+
expect(result[0]?.email).toBe(`[email protected]`)
2248+
expect(result[1]?.email).toBe(`[email protected]`)
2249+
expect(result[2]?.email).toBe(`[email protected]`)
2250+
})
2251+
})

0 commit comments

Comments
 (0)