Skip to content

Commit 862d1c2

Browse files
ErikDakodaclaude
andauthored
fix(orm): alias delegate joins in relation subqueries so they don't shadow outer tables (#2877)
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent ee71982 commit 862d1c2

5 files changed

Lines changed: 273 additions & 17 deletions

File tree

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

Lines changed: 53 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ export abstract class BaseCrudDialect<Schema extends SchemaDef> {
282282
const fieldRef = this.fieldRef(
283283
fieldDef.originModel ?? model,
284284
key,
285-
fieldDef.originModel ?? modelAlias,
285+
this.delegateFieldAlias(model, modelAlias, fieldDef),
286286
true,
287287
computedArgs,
288288
);
@@ -328,10 +328,10 @@ export abstract class BaseCrudDialect<Schema extends SchemaDef> {
328328
const _order = negateOrderBy ? (order === 'asc' ? 'desc' : 'asc') : order;
329329
const op = j === i ? (_order === 'asc' ? '>=' : '<=') : '=';
330330
// Fields inherited from a delegate base live on the base table, which is
331-
// joined with its model name as alias. See buildSelectModel/buildDelegateJoin.
331+
// joined under `delegateJoinAlias`. See buildSelectModel/buildDelegateJoin.
332332
const fieldDef = requireField(this.schema, model, field);
333-
const outerAlias = fieldDef.originModel ?? modelAlias;
334-
const subSelectAlias = fieldDef.originModel ?? subQueryAlias;
333+
const outerAlias = this.delegateFieldAlias(model, modelAlias, fieldDef);
334+
const subSelectAlias = this.delegateFieldAlias(model, subQueryAlias, fieldDef);
335335
andFilters.push(
336336
this.eb(
337337
this.eb.ref(`${outerAlias}.${field}`),
@@ -431,7 +431,7 @@ export abstract class BaseCrudDialect<Schema extends SchemaDef> {
431431
this.schema,
432432
model,
433433
// if field is from a base, use the base model to join
434-
fieldDef.originModel ?? modelAlias,
434+
this.delegateFieldAlias(model, modelAlias, fieldDef),
435435
field,
436436
joinAlias,
437437
);
@@ -1184,7 +1184,13 @@ export abstract class BaseCrudDialect<Schema extends SchemaDef> {
11841184
const buildFieldRef = (model: string, field: string, modelAlias: string, computedArgs?: unknown) => {
11851185
const fieldDef = requireField(this.schema, model, field);
11861186
return fieldDef.originModel
1187-
? this.fieldRef(fieldDef.originModel, field, fieldDef.originModel, true, computedArgs)
1187+
? this.fieldRef(
1188+
fieldDef.originModel,
1189+
field,
1190+
this.delegateJoinAlias(model, modelAlias, fieldDef.originModel),
1191+
true,
1192+
computedArgs,
1193+
)
11881194
: this.fieldRef(model, field, modelAlias, true, computedArgs);
11891195
};
11901196

@@ -1283,12 +1289,25 @@ export abstract class BaseCrudDialect<Schema extends SchemaDef> {
12831289

12841290
// order by to-one relation
12851291
const joinAlias = tmpAlias(`${modelAlias}$ob$${index}`);
1286-
const joined = query.leftJoin(`${relationModel} as ${joinAlias}`, (join) => {
1287-
const joinPairs = buildJoinPairs(this.schema, model, modelAlias, field, joinAlias);
1292+
let joined = query.leftJoin(`${relationModel} as ${joinAlias}`, (join) => {
1293+
const joinPairs = buildJoinPairs(
1294+
this.schema,
1295+
model,
1296+
// if field is from a base, use the base model to join
1297+
this.delegateFieldAlias(model, modelAlias, fieldDef),
1298+
field,
1299+
joinAlias,
1300+
);
12881301
return join.on((eb) =>
12891302
this.and(...joinPairs.map(([left, right]) => eb(this.eb.ref(left), '=', this.eb.ref(right)))),
12901303
);
12911304
});
1305+
// join delegate bases so that the ordering can reference inherited fields
1306+
let joinBase = requireModel(this.schema, relationModel).baseModel;
1307+
while (joinBase) {
1308+
joined = this.buildDelegateJoin(relationModel, joinAlias, joinBase, joined);
1309+
joinBase = requireModel(this.schema, joinBase).baseModel;
1310+
}
12921311
return this.buildOrderBy(joined, relationModel, joinAlias, value, negated, take);
12931312
}
12941313

@@ -1475,6 +1494,7 @@ export abstract class BaseCrudDialect<Schema extends SchemaDef> {
14751494
const descendants = getDelegateDescendantModels(this.schema, model);
14761495
for (const subModel of descendants) {
14771496
result = this.buildDelegateJoin(model, modelAlias, subModel.name, result);
1497+
const subModelAlias = this.delegateJoinAlias(model, modelAlias, subModel.name);
14781498
result = result.select(() => {
14791499
const jsonObject: Record<string, Expression<any>> = {};
14801500
for (const fieldDef of getModelFields(this.schema, subModel.name, { computed: true })) {
@@ -1485,7 +1505,7 @@ export abstract class BaseCrudDialect<Schema extends SchemaDef> {
14851505
if (fieldDef.computed && fieldDef.params) {
14861506
continue;
14871507
}
1488-
jsonObject[fieldDef.name] = this.fieldRef(subModel.name, fieldDef.name, subModel.name);
1508+
jsonObject[fieldDef.name] = this.fieldRef(subModel.name, fieldDef.name, subModelAlias);
14891509
}
14901510
return this.buildJsonObject(jsonObject).as(`${DELEGATE_JOINED_FIELD_PREFIX}${subModel.name}`);
14911511
});
@@ -1546,10 +1566,9 @@ export abstract class BaseCrudDialect<Schema extends SchemaDef> {
15461566
): SelectQueryBuilder<any, any, any> {
15471567
const fieldDef = requireField(this.schema, model, field);
15481568

1549-
// if field is defined on a delegate base, the base model is joined with its
1550-
// model name from outer query, so we should use it directly as the alias
1569+
// if field is defined on a delegate base, it's selected from the joined base table
15511570
const fieldModel = fieldDef.originModel ?? model;
1552-
const alias = fieldDef.originModel ?? modelAlias;
1571+
const alias = this.delegateFieldAlias(model, modelAlias, fieldDef);
15531572

15541573
// `computedArgs` is set for a parameterized computed field selected via `select`/`include`
15551574
return query.select(() => this.fieldRef(fieldModel, field, alias, true, computedArgs).as(field));
@@ -1558,11 +1577,13 @@ export abstract class BaseCrudDialect<Schema extends SchemaDef> {
15581577
buildDelegateJoin(
15591578
thisModel: string,
15601579
thisModelAlias: string,
1561-
otherModelAlias: string,
1580+
otherModel: string,
15621581
query: SelectQueryBuilder<any, any, any>,
15631582
) {
15641583
const idFields = requireIdFields(this.schema, thisModel);
1565-
query = query.leftJoin(otherModelAlias, (qb) => {
1584+
const otherModelAlias = this.delegateJoinAlias(thisModel, thisModelAlias, otherModel);
1585+
const table = otherModelAlias === otherModel ? otherModel : `${otherModel} as ${otherModelAlias}`;
1586+
query = query.leftJoin(table, (qb) => {
15661587
for (const idField of idFields) {
15671588
qb = qb.onRef(`${thisModelAlias}.${idField}`, '=', `${otherModelAlias}.${idField}`);
15681589
}
@@ -1571,6 +1592,24 @@ export abstract class BaseCrudDialect<Schema extends SchemaDef> {
15711592
return query;
15721593
}
15731594

1595+
/**
1596+
* Gets the alias under which a delegate base or sub model is joined to a model selected as `modelAlias`.
1597+
* A model selected under its own name (a top-level query) has them joined under their own model names.
1598+
* A model selected under another alias (a relation subquery) has them joined under aliases derived from
1599+
* it, so they never shadow a table of an enclosing query in the same delegate hierarchy.
1600+
*/
1601+
delegateJoinAlias(model: string, modelAlias: string, joinedModel: string) {
1602+
return modelAlias === model ? joinedModel : tmpAlias(`${modelAlias}$${joinedModel}`);
1603+
}
1604+
1605+
/**
1606+
* Gets the alias of the table holding a field of a model selected as `modelAlias`: the joined base
1607+
* table for a field inherited from a delegate base, otherwise the model's own table.
1608+
*/
1609+
delegateFieldAlias(model: string, modelAlias: string, fieldDef: FieldDef) {
1610+
return fieldDef.originModel ? this.delegateJoinAlias(model, modelAlias, fieldDef.originModel) : modelAlias;
1611+
}
1612+
15741613
buildCountJson(model: string, eb: ExpressionBuilder<any, any>, parentAlias: string, payload: any) {
15751614
const modelDef = requireModel(this.schema, model);
15761615
const toManyRelations = Object.entries(modelDef.fields).filter(([, field]) => field.relation && field.array);

‎packages/orm/src/client/crud/operations/base.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,7 @@ export abstract class BaseOperationHandler<Schema extends SchemaDef> {
258258
result,
259259
fieldDef.originModel,
260260
field,
261-
fieldDef.originModel,
261+
this.dialect.delegateJoinAlias(model, parentAlias, fieldDef.originModel),
262262
payload,
263263
);
264264
} else {

‎packages/plugins/soft-delete/src/plugin.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ class SoftDeleteHandler<Schema extends SchemaDef> extends OperationNodeTransform
207207
// Resolve the single `@deletedAt` column to filter when reading a table. `@@@onceInModel` allows
208208
// at most one marker per inheritance hierarchy, so it's either declared on the model itself or
209209
// inherited from exactly one delegate base — where the column physically lives on the base's
210-
// table (LEFT-joined under its own model name, see buildDelegateJoin), so the filter must key off
210+
// table (LEFT-joined under `delegateJoinAlias`, see buildDelegateJoin), so the filter must key off
211211
// that base rather than the queried table.
212212
private getSoftDeleteTarget(info: TableInfo): { table: string; column: string } | undefined {
213213
const own = this.getDeletedAtField(info.model);
@@ -218,7 +218,10 @@ class SoftDeleteHandler<Schema extends SchemaDef> extends OperationNodeTransform
218218
while (base) {
219219
const marker = this.getDeletedAtField(base);
220220
if (marker) {
221-
return { table: base, column: marker.name };
221+
return {
222+
table: this.dialect.delegateJoinAlias(info.model, info.alias ?? info.model, base),
223+
column: marker.name,
224+
};
222225
}
223226
base = QueryUtils.getModel(this.client.$schema, base)?.baseModel;
224227
}

‎packages/plugins/soft-delete/test/soft-delete.test.ts‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -431,4 +431,43 @@ model Article extends Content {
431431
const row = await raw.article.findUniqueOrThrow({ where: { id: a.id } });
432432
expect(row.title).toBe('a');
433433
});
434+
435+
it('hides soft-deleted delegate rows reached through a relation', async () => {
436+
// In a relation subquery the delegate base is joined under an alias derived from the
437+
// subquery's alias, so the read filter must reference the base through that alias.
438+
const delegateSchema = `
439+
model User {
440+
id Int @id @default(autoincrement())
441+
articles Article[]
442+
}
443+
444+
model Content {
445+
id Int @id @default(autoincrement())
446+
contentType String
447+
deletedAt DateTime? @deletedAt
448+
@@delegate(contentType)
449+
}
450+
451+
model Article extends Content {
452+
title String
453+
author User @relation(fields: [authorId], references: [id])
454+
authorId Int
455+
}
456+
`;
457+
const raw = await createSoftDeleteTestClient(delegateSchema, { usePrismaPush: true });
458+
const db = raw.$use(new SoftDeletePlugin());
459+
460+
const user = await db.user.create({ data: {} });
461+
const kept = await db.article.create({ data: { title: 'kept', authorId: user.id } });
462+
const removed = await db.article.create({ data: { title: 'removed', authorId: user.id } });
463+
await db.article.delete({ where: { id: removed.id } });
464+
465+
await expect(db.user.findUnique({ where: { id: user.id }, include: { articles: true } })).resolves.toEqual(
466+
expect.objectContaining({ articles: [expect.objectContaining({ id: kept.id })] }),
467+
);
468+
await expect(
469+
db.user.findUnique({ where: { id: user.id }, include: { _count: { select: { articles: true } } } }),
470+
).resolves.toMatchObject({ _count: { articles: 1 } });
471+
await expect(db.user.count({ where: { articles: { some: { title: 'removed' } } } })).resolves.toBe(0);
472+
});
434473
});
Lines changed: 175 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,175 @@
1+
import { createPolicyTestClient, createTestClient } from '@zenstackhq/testtools';
2+
import { describe, expect, it } from 'vitest';
3+
4+
// https://github.com/zenstackhq/zenstack/issues/2870
5+
describe('Regression for issue #2870', () => {
6+
const schema = `
7+
model Item {
8+
id String @id @default(cuid())
9+
itemKind String
10+
rank Int @default(0)
11+
notesAsSource Note[] @relation("NoteSource")
12+
pinnedTaskId String?
13+
pinnedTask Task? @relation("Pinned", fields: [pinnedTaskId], references: [id])
14+
tags Tag[] @relation("ItemTags")
15+
16+
@@delegate(itemKind)
17+
}
18+
19+
model Task extends Item {
20+
pinnedBy Item[] @relation("Pinned")
21+
}
22+
23+
model Tag extends Item {
24+
taggedItems Item[] @relation("ItemTags")
25+
}
26+
27+
model Note extends Item {
28+
sourceId String?
29+
source Item? @relation("NoteSource", fields: [sourceId], references: [id])
30+
comments Comment[]
31+
}
32+
33+
model Comment {
34+
id String @id @default(cuid())
35+
noteId String?
36+
note Note? @relation(fields: [noteId], references: [id])
37+
}
38+
`;
39+
40+
// explicit ids so that ordering by id differs from ordering by relation count
41+
async function setup() {
42+
const db = await createTestClient(schema, { usePrismaPush: true });
43+
const task = await db.task.create({ data: { id: 'z-task' } });
44+
const note = await db.note.create({ data: { id: 'a-note', sourceId: task.id, pinnedTaskId: task.id } });
45+
const tag = await db.tag.create({ data: { id: 'm-tag', taggedItems: { connect: { id: note.id } } } });
46+
return { db, task, note, tag };
47+
}
48+
49+
const ids = (rows: { id: string }[]) => rows.map((r) => r.id).sort();
50+
51+
it('filters a base model by a to-many relation into its own hierarchy', async () => {
52+
const { db, task, note, tag } = await setup();
53+
54+
expect(ids(await db.item.findMany({ where: { notesAsSource: { some: {} } } }))).toEqual([task.id]);
55+
expect(ids(await db.item.findMany({ where: { notesAsSource: { none: {} } } }))).toEqual(
56+
[note.id, tag.id].sort(),
57+
);
58+
expect(ids(await db.item.findMany({ where: { notesAsSource: { every: { id: { not: note.id } } } } }))).toEqual(
59+
[note.id, tag.id].sort(),
60+
);
61+
await expect(db.item.count({ where: { notesAsSource: { some: {} } } })).resolves.toBe(1);
62+
expect(ids(await db.item.findMany({ where: { tags: { some: {} } } }))).toEqual([note.id]);
63+
});
64+
65+
it('filters by a to-one relation into the same hierarchy', async () => {
66+
const { db, note } = await setup();
67+
68+
expect(ids(await db.item.findMany({ where: { pinnedTask: { is: {} } } }))).toEqual([note.id]);
69+
expect(ids(await db.note.findMany({ where: { pinnedTask: { is: {} } } }))).toEqual([note.id]);
70+
expect(ids(await db.note.findMany({ where: { source: { is: { itemKind: 'Task' } } } }))).toEqual([note.id]);
71+
});
72+
73+
it('counts a base model relation into its own hierarchy', async () => {
74+
const { db, task, note } = await setup();
75+
76+
await expect(
77+
db.item.findUnique({ where: { id: task.id }, include: { _count: { select: { notesAsSource: true } } } }),
78+
).resolves.toMatchObject({ _count: { notesAsSource: 1 } });
79+
await expect(
80+
db.item.findUnique({
81+
where: { id: task.id },
82+
select: { _count: { select: { notesAsSource: { where: { id: note.id } } } } },
83+
}),
84+
).resolves.toEqual({ _count: { notesAsSource: 1 } });
85+
await expect(
86+
db.item.findUnique({ where: { id: note.id }, include: { _count: { select: { tags: true } } } }),
87+
).resolves.toMatchObject({ _count: { tags: 1 } });
88+
await expect(
89+
db.item.findFirst({ orderBy: [{ notesAsSource: { _count: 'desc' } }, { id: 'asc' }] }),
90+
).resolves.toMatchObject({ id: task.id });
91+
});
92+
93+
it('orders by an inherited field of a to-one relation', async () => {
94+
const db = await createTestClient(schema, { usePrismaPush: true });
95+
const highTask = await db.task.create({ data: { rank: 2 } });
96+
const lowTask = await db.task.create({ data: { rank: 1 } });
97+
// each note's own rank is the opposite of its pinned task's rank
98+
const first = await db.note.create({ data: { rank: 2, pinnedTaskId: lowTask.id } });
99+
const second = await db.note.create({ data: { rank: 1, pinnedTaskId: highTask.id } });
100+
101+
// outer model in the same hierarchy as the related model's base
102+
await expect(db.note.findMany({ orderBy: { pinnedTask: { rank: 'asc' } } })).resolves.toEqual([
103+
expect.objectContaining({ id: first.id }),
104+
expect.objectContaining({ id: second.id }),
105+
]);
106+
107+
// outer model outside the hierarchy
108+
const c1 = await db.comment.create({ data: { noteId: second.id } });
109+
const c2 = await db.comment.create({ data: { noteId: first.id } });
110+
await expect(db.comment.findMany({ orderBy: { note: { rank: 'desc' } } })).resolves.toEqual([
111+
expect.objectContaining({ id: c2.id }),
112+
expect.objectContaining({ id: c1.id }),
113+
]);
114+
});
115+
116+
it('includes relations into the same hierarchy', async () => {
117+
const { db, task, note, tag } = await setup();
118+
119+
const taskWithNotes = await db.item.findUnique({ where: { id: task.id }, include: { notesAsSource: true } });
120+
expect(ids(taskWithNotes!.notesAsSource)).toEqual([note.id]);
121+
122+
await expect(db.note.findUnique({ where: { id: note.id }, include: { source: true } })).resolves.toMatchObject({
123+
source: { id: task.id },
124+
});
125+
await expect(
126+
db.item.findUnique({ where: { id: note.id }, include: { pinnedTask: true } }),
127+
).resolves.toMatchObject({ pinnedTask: { id: task.id } });
128+
129+
const noteWithTags = await db.item.findUnique({ where: { id: note.id }, include: { tags: true } });
130+
expect(ids(noteWithTags!.tags)).toEqual([tag.id]);
131+
});
132+
133+
it('gives the same result through the sub model', async () => {
134+
const { db, task } = await setup();
135+
136+
expect(ids(await db.task.findMany({ where: { notesAsSource: { some: {} } } }))).toEqual([task.id]);
137+
expect(ids(await db.task.findMany({ where: { pinnedBy: { some: {} } } }))).toEqual([task.id]);
138+
await expect(
139+
db.task.findFirst({ include: { _count: { select: { notesAsSource: true } } } }),
140+
).resolves.toMatchObject({ _count: { notesAsSource: 1 } });
141+
});
142+
143+
it('includes relations into the same hierarchy with access policies', async () => {
144+
const db = await createPolicyTestClient(`
145+
model Item {
146+
id String @id @default(cuid())
147+
itemKind String
148+
notesAsSource Note[] @relation("NoteSource")
149+
150+
@@delegate(itemKind)
151+
@@allow('create', true)
152+
@@allow('read', notesAsSource?[true] || itemKind == 'Note')
153+
}
154+
155+
model Task extends Item {}
156+
157+
model Note extends Item {
158+
sourceId String?
159+
source Item? @relation("NoteSource", fields: [sourceId], references: [id])
160+
}
161+
`);
162+
const raw = db.$unuseAll();
163+
const task = await raw.task.create({ data: {} });
164+
const hidden = await raw.task.create({ data: {} });
165+
const note = await raw.note.create({ data: { sourceId: task.id } });
166+
167+
expect(ids(await db.item.findMany())).toEqual([task.id, note.id].sort());
168+
await expect(db.task.findUnique({ where: { id: hidden.id } })).resolves.toBeNull();
169+
await expect(db.note.findFirst({ include: { source: true } })).resolves.toMatchObject({
170+
source: { id: task.id },
171+
});
172+
const taskWithNotes = await db.item.findUnique({ where: { id: task.id }, include: { notesAsSource: true } });
173+
expect(ids(taskWithNotes!.notesAsSource)).toEqual([note.id]);
174+
});
175+
});

0 commit comments

Comments
 (0)