Skip to content

Commit eb8fd18

Browse files
authored
Also optimize orderBy for singleton array (#477)
1 parent 59baa34 commit eb8fd18

File tree

3 files changed

+102
-6
lines changed

3 files changed

+102
-6
lines changed

.changeset/afraid-plums-clean.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 an optimization bug where orderBy clauses using a single-column array were not recognized as optimizable. Queries that order by a single column are now correctly optimized even when specified as an array.

packages/db/src/query/builder/index.ts

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import type {
1414
BasicExpression,
1515
JoinClause,
1616
OrderBy,
17-
OrderByClause,
1817
OrderByDirection,
1918
QueryIR,
2019
} from "../ir.js"
@@ -501,17 +500,23 @@ export class BaseQueryBuilder<TContext extends Context = Context> {
501500
: undefined,
502501
}
503502

504-
// Create the new OrderBy structure with expression and direction
505-
const orderByClause: OrderByClause = {
506-
expression: toExpression(result),
507-
compareOptions: opts,
503+
const makeOrderByClause = (res: any) => {
504+
return {
505+
expression: toExpression(res),
506+
compareOptions: opts,
507+
}
508508
}
509509

510+
// Create the new OrderBy structure with expression and direction
511+
const orderByClauses = Array.isArray(result)
512+
? result.map((r) => makeOrderByClause(r))
513+
: [makeOrderByClause(result)]
514+
510515
const existingOrderBy: OrderBy = this.query.orderBy || []
511516

512517
return new BaseQueryBuilder({
513518
...this.query,
514-
orderBy: [...existingOrderBy, orderByClause],
519+
orderBy: [...existingOrderBy, ...orderByClauses],
515520
}) as any
516521
}
517522

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

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1241,6 +1241,92 @@ function createOrderByTests(autoIndex: `off` | `eager`): void {
12411241
})
12421242
})
12431243

1244+
describe(`OrderBy Optimization Tests`, () => {
1245+
const itWhenAutoIndex = autoIndex === `eager` ? it : it.skip
1246+
1247+
itWhenAutoIndex(
1248+
`optimizes single-column orderBy when passed as single value`,
1249+
async () => {
1250+
// Patch getConfig to expose the builder on the returned config for test access
1251+
const { CollectionConfigBuilder } = await import(
1252+
`../../src/query/live/collection-config-builder.js`
1253+
)
1254+
const originalGetConfig = CollectionConfigBuilder.prototype.getConfig
1255+
1256+
CollectionConfigBuilder.prototype.getConfig = function (this: any) {
1257+
const cfg = originalGetConfig.call(this)
1258+
;(cfg as any).__builder = this
1259+
return cfg
1260+
}
1261+
1262+
try {
1263+
const collection = createLiveQueryCollection((q) =>
1264+
q
1265+
.from({ employees: employeesCollection })
1266+
.orderBy(({ employees }) => employees.salary, `desc`)
1267+
.limit(3)
1268+
.select(({ employees }) => ({
1269+
id: employees.id,
1270+
name: employees.name,
1271+
salary: employees.salary,
1272+
}))
1273+
)
1274+
1275+
await collection.preload()
1276+
1277+
const builder = (collection as any).config.__builder
1278+
expect(builder).toBeTruthy()
1279+
expect(
1280+
Object.keys(builder.optimizableOrderByCollections)
1281+
).toContain(employeesCollection.id)
1282+
} finally {
1283+
CollectionConfigBuilder.prototype.getConfig = originalGetConfig
1284+
}
1285+
}
1286+
)
1287+
1288+
itWhenAutoIndex(
1289+
`optimizes single-column orderBy when passed as array with single element`,
1290+
async () => {
1291+
// Patch getConfig to expose the builder on the returned config for test access
1292+
const { CollectionConfigBuilder } = await import(
1293+
`../../src/query/live/collection-config-builder.js`
1294+
)
1295+
const originalGetConfig = CollectionConfigBuilder.prototype.getConfig
1296+
1297+
CollectionConfigBuilder.prototype.getConfig = function (this: any) {
1298+
const cfg = originalGetConfig.call(this)
1299+
;(cfg as any).__builder = this
1300+
return cfg
1301+
}
1302+
1303+
try {
1304+
const collection = createLiveQueryCollection((q) =>
1305+
q
1306+
.from({ employees: employeesCollection })
1307+
.orderBy(({ employees }) => [employees.salary], `desc`)
1308+
.limit(3)
1309+
.select(({ employees }) => ({
1310+
id: employees.id,
1311+
name: employees.name,
1312+
salary: employees.salary,
1313+
}))
1314+
)
1315+
1316+
await collection.preload()
1317+
1318+
const builder = (collection as any).config.__builder
1319+
expect(builder).toBeTruthy()
1320+
expect(
1321+
Object.keys(builder.optimizableOrderByCollections)
1322+
).toContain(employeesCollection.id)
1323+
} finally {
1324+
CollectionConfigBuilder.prototype.getConfig = originalGetConfig
1325+
}
1326+
}
1327+
)
1328+
})
1329+
12441330
describe(`String Comparison Tests`, () => {
12451331
it(`handles case differently in lexical vs locale string comparison`, async () => {
12461332
const numericEmployees = [

0 commit comments

Comments
 (0)