Skip to content

Commit 9346fe9

Browse files
eredzikymc9
andauthored
fix(orm): fixed postgres orderBy in nested queries (includes/selects) (#2518)
Co-authored-by: ymc9 <104139426+ymc9@users.noreply.github.com>
1 parent 06f6e08 commit 9346fe9

File tree

4 files changed

+224
-8
lines changed

4 files changed

+224
-8
lines changed

packages/orm/src/client/crud/dialects/lateral-join-dialect-base.ts

Lines changed: 52 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,10 @@ import { invariant } from '@zenstackhq/common-helpers';
22
import { type AliasableExpression, type Expression, type ExpressionBuilder, type SelectQueryBuilder } from 'kysely';
33
import type { FieldDef, GetModels, SchemaDef } from '../../../schema';
44
import { DELEGATE_JOINED_FIELD_PREFIX } from '../../constants';
5-
import type { FindArgs } from '../../crud-types';
5+
import type { FindArgs, NullsOrder, SortOrder } from '../../crud-types';
66
import {
77
buildJoinPairs,
8+
ensureArray,
89
getDelegateDescendantModels,
910
getManyToManyRelation,
1011
isRelationField,
@@ -23,7 +24,10 @@ export abstract class LateralJoinDialectBase<Schema extends SchemaDef> extends B
2324
/**
2425
* Builds an array aggregation expression.
2526
*/
26-
protected abstract buildArrayAgg(arg: Expression<any>): AliasableExpression<any>;
27+
protected abstract buildArrayAgg(
28+
arg: Expression<any>,
29+
orderBy?: { expr: Expression<any>; sort: SortOrder; nulls?: NullsOrder }[],
30+
): AliasableExpression<any>;
2731

2832
override buildRelationSelection(
2933
query: SelectQueryBuilder<any, any, any>,
@@ -172,7 +176,8 @@ export abstract class LateralJoinDialectBase<Schema extends SchemaDef> extends B
172176
);
173177

174178
if (relationFieldDef.array) {
175-
return this.buildArrayAgg(this.buildJsonObject(objArgs)).as('$data');
179+
const orderBy = this.buildRelationOrderByExpressions(relationModel, relationModelAlias, payload);
180+
return this.buildArrayAgg(this.buildJsonObject(objArgs), orderBy).as('$data');
176181
} else {
177182
return this.buildJsonObject(objArgs).as('$data');
178183
}
@@ -181,6 +186,50 @@ export abstract class LateralJoinDialectBase<Schema extends SchemaDef> extends B
181186
return qb;
182187
}
183188

189+
/**
190+
* Extracts scalar `orderBy` clauses from the relation payload and maps them to
191+
* the array-aggregation ordering format.
192+
*
193+
* For to-many relations aggregated into a JSON array (via lateral joins), this
194+
* lets us preserve a stable ordering by passing `{ expr, sort, nulls? }` into
195+
* the dialect's `buildArrayAgg` implementation.
196+
*/
197+
private buildRelationOrderByExpressions(
198+
model: string,
199+
modelAlias: string,
200+
payload: true | FindArgs<Schema, GetModels<Schema>, any, true>,
201+
): { expr: Expression<any>; sort: SortOrder; nulls?: NullsOrder }[] | undefined {
202+
if (payload === true || !payload.orderBy) {
203+
return undefined;
204+
}
205+
206+
type ScalarSortValue = SortOrder | { sort: SortOrder; nulls?: NullsOrder };
207+
const items: { expr: Expression<any>; sort: SortOrder; nulls?: NullsOrder }[] = [];
208+
209+
for (const orderBy of ensureArray(payload.orderBy)) {
210+
for (const [field, value] of Object.entries(orderBy) as [string, ScalarSortValue | undefined][]) {
211+
if (!value || requireField(this.schema, model, field).relation) {
212+
continue;
213+
}
214+
215+
const expr = this.fieldRef(model, field, modelAlias);
216+
let sort = typeof value === 'string' ? value : value.sort;
217+
if (payload.take !== undefined && payload.take < 0) {
218+
// negative `take` requires negated sorting, and the result order
219+
// will be corrected during post-read processing
220+
sort = this.negateSort(sort, true);
221+
}
222+
if (typeof value === 'string') {
223+
items.push({ expr, sort });
224+
} else {
225+
items.push({ expr, sort, nulls: value.nulls });
226+
}
227+
}
228+
}
229+
230+
return items.length > 0 ? items : undefined;
231+
}
232+
184233
private buildRelationObjectArgs(
185234
relationModel: string,
186235
relationModelAlias: string,

packages/orm/src/client/crud/dialects/mysql.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import {
1212
} from 'kysely';
1313
import { AnyNullClass, DbNullClass, JsonNullClass } from '../../../common-types';
1414
import type { BuiltinType, FieldDef, SchemaDef } from '../../../schema';
15-
import type { SortOrder } from '../../crud-types';
15+
import type { NullsOrder, SortOrder } from '../../crud-types';
1616
import { createInvalidInputError, createNotSupportedError } from '../../errors';
1717
import type { ClientOptions } from '../../options';
1818
import { isTypeDef } from '../../query-utils';
@@ -192,7 +192,13 @@ export class MySqlCrudDialect<Schema extends SchemaDef> extends LateralJoinDiale
192192
return this.eb.exists(this.eb.selectFrom(innerQuery.as('$exists_sub')).select(this.eb.lit(1).as('_')));
193193
}
194194

195-
protected buildArrayAgg(arg: Expression<any>): AliasableExpression<any> {
195+
protected buildArrayAgg(
196+
arg: Expression<any>,
197+
_orderBy?: { expr: Expression<any>; sort: SortOrder; nulls?: NullsOrder }[],
198+
): AliasableExpression<any> {
199+
// MySQL doesn't support ORDER BY inside JSON_ARRAYAGG.
200+
// For relation queries that need deterministic ordering, ordering is applied
201+
// by the input subquery before aggregation.
196202
return this.eb.fn.coalesce(sql`JSON_ARRAYAGG(${arg})`, sql`JSON_ARRAY()`);
197203
}
198204

packages/orm/src/client/crud/dialects/postgresql.ts

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import {
1111
import { parse as parsePostgresArray } from 'postgres-array';
1212
import { AnyNullClass, DbNullClass, JsonNullClass } from '../../../common-types';
1313
import type { BuiltinType, FieldDef, SchemaDef } from '../../../schema';
14-
import type { SortOrder } from '../../crud-types';
14+
import type { NullsOrder, SortOrder } from '../../crud-types';
1515
import { createInvalidInputError } from '../../errors';
1616
import type { ClientOptions } from '../../options';
1717
import { isEnum, isTypeDef } from '../../query-utils';
@@ -272,8 +272,24 @@ export class PostgresCrudDialect<Schema extends SchemaDef> extends LateralJoinDi
272272

273273
// #region other overrides
274274

275-
protected buildArrayAgg(arg: Expression<any>) {
276-
return this.eb.fn.coalesce(sql`jsonb_agg(${arg})`, sql`'[]'::jsonb`);
275+
protected buildArrayAgg(
276+
arg: Expression<any>,
277+
orderBy?: { expr: Expression<any>; sort: SortOrder; nulls?: NullsOrder }[],
278+
) {
279+
if (!orderBy || orderBy.length === 0) {
280+
return this.eb.fn.coalesce(sql`jsonb_agg(${arg})`, sql`'[]'::jsonb`);
281+
}
282+
283+
const orderBySql = sql.join(
284+
orderBy.map(({ expr, sort, nulls }) => {
285+
const dir = sql.raw(sort.toUpperCase());
286+
const nullsSql = nulls ? sql` NULLS ${sql.raw(nulls.toUpperCase())}` : sql``;
287+
return sql`${expr} ${dir}${nullsSql}`;
288+
}),
289+
sql.raw(', '),
290+
);
291+
292+
return this.eb.fn.coalesce(sql`jsonb_agg(${arg} ORDER BY ${orderBySql})`, sql`'[]'::jsonb`);
277293
}
278294

279295
override buildSkipTake(
Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
import { createTestClient } from '@zenstackhq/testtools';
2+
import { afterEach, describe, expect, it } from 'vitest';
3+
4+
const schema = `
5+
model User {
6+
id String @id
7+
email String @unique
8+
posts Post[]
9+
comments Comment[]
10+
}
11+
12+
model Post {
13+
id String @id
14+
sequence Int
15+
title String
16+
author User @relation(fields: [authorId], references: [id])
17+
authorId String
18+
comments Comment[]
19+
}
20+
21+
model Comment {
22+
id String @id
23+
content String
24+
post Post @relation(fields: [postId], references: [id])
25+
postId String
26+
author User? @relation(fields: [authorId], references: [id])
27+
authorId String?
28+
}
29+
`;
30+
31+
function makePostsData(count: number) {
32+
return Array.from({ length: count }, (_, i) => {
33+
const sequence = count - i; // insert descending
34+
return {
35+
id: `p${sequence}`,
36+
sequence,
37+
title: `P${sequence}`,
38+
// Keep outer relation (User -> posts) required.
39+
authorId: 'u1',
40+
};
41+
});
42+
}
43+
44+
function makeCommentsData(count: number) {
45+
return Array.from({ length: count }, (_, i) => {
46+
const sequence = count - i;
47+
return {
48+
id: `c${sequence}`,
49+
postId: `p${sequence}`,
50+
content: `C${sequence}`,
51+
// Make nested to-one include nullable to vary lateral join execution.
52+
authorId: sequence % 11 === 0 ? null : 'u1',
53+
};
54+
});
55+
}
56+
57+
describe('Relation orderBy with nested includes', () => {
58+
let db: any;
59+
60+
afterEach(async () => {
61+
await db?.$disconnect();
62+
});
63+
64+
it('keeps stable order for to-many include with nested includes', async () => {
65+
const count = 2000;
66+
67+
db = await createTestClient(schema);
68+
69+
await db.user.create({ data: { id: 'u1', email: 'u1@example.com' } });
70+
await db.post.createMany({ data: makePostsData(count) });
71+
await db.comment.createMany({ data: makeCommentsData(count) });
72+
73+
const user = await db.user.findFirst({
74+
where: { id: 'u1' },
75+
include: {
76+
posts: {
77+
orderBy: { sequence: 'asc' },
78+
include: { author: true, comments: { include: { author: true } } },
79+
},
80+
},
81+
});
82+
83+
const ascSequences = user.posts.map((p: any) => p.sequence);
84+
expect(ascSequences).toEqual(Array.from({ length: count }, (_, i) => i + 1));
85+
86+
const userDesc = await db.user.findFirst({
87+
where: { id: 'u1' },
88+
include: {
89+
posts: {
90+
orderBy: { sequence: 'desc' },
91+
include: { author: true, comments: { include: { author: true } } },
92+
},
93+
},
94+
});
95+
96+
const descSequences = userDesc.posts.map((p: any) => p.sequence);
97+
expect(descSequences).toEqual(Array.from({ length: count }, (_, i) => count - i));
98+
});
99+
100+
it('keeps stable order for to-many select with nested selects', async () => {
101+
const count = 2000;
102+
103+
db = await createTestClient(schema);
104+
105+
await db.user.create({ data: { id: 'u1', email: 'u1@example.com' } });
106+
await db.post.createMany({ data: makePostsData(count) });
107+
await db.comment.createMany({ data: makeCommentsData(count) });
108+
109+
const user = await db.user.findFirst({
110+
where: { id: 'u1' },
111+
select: {
112+
id: true,
113+
posts: {
114+
orderBy: { sequence: 'asc' },
115+
select: {
116+
sequence: true,
117+
author: { select: { id: true } },
118+
comments: { select: { author: { select: { id: true } } } },
119+
},
120+
},
121+
},
122+
});
123+
124+
const ascSequences = user.posts.map((p: any) => p.sequence);
125+
expect(ascSequences).toEqual(Array.from({ length: count }, (_, i) => i + 1));
126+
127+
const userDesc = await db.user.findFirst({
128+
where: { id: 'u1' },
129+
select: {
130+
id: true,
131+
posts: {
132+
orderBy: { sequence: 'desc' },
133+
select: {
134+
sequence: true,
135+
author: { select: { id: true } },
136+
comments: { select: { author: { select: { id: true } } } },
137+
},
138+
},
139+
},
140+
});
141+
142+
const descSequences = userDesc.posts.map((p: any) => p.sequence);
143+
expect(descSequences).toEqual(Array.from({ length: count }, (_, i) => count - i));
144+
});
145+
});

0 commit comments

Comments
 (0)