Skip to content

Commit 32affa3

Browse files
committed
fix(orm): recurse into composite expressions in bindWhereExpr, reject invalid HAVING predicates
bindWhereExpr: 6 composite expression handlers (subquery, operation, aggregate, jsonObject, jsonArrayAgg, list) were incorrectly implemented as passthroughs, skipping binding of nested SelectAsts. Now they call bindExpression to recursively bind inner expressions. validateGroupedHavingExpr: 10 passthrough handlers silently allowed expression types that are not valid HAVING predicates (columnRef, identifierRef, subquery, operation, aggregate, jsonObject, jsonArrayAgg, literal, param, list). Now they throw, consistent with the existing exists rejection. ParamRef throws with the specific ParamRef message. Added exhaustive tests for both functions covering all expression types, edge cases, error paths, and nested logical expressions.
1 parent c7c108d commit 32affa3

4 files changed

Lines changed: 506 additions & 35 deletions

File tree

packages/3-extensions/sql-orm-client/src/query-plan-aggregate.ts

Lines changed: 15 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -60,38 +60,24 @@ function validateGroupedMetricExpr(expr: AnyExpression): AggregateExpr {
6060
return expr;
6161
}
6262

63+
function rejectHavingExpr(expr: { kind: string }): never {
64+
throw new Error(`Unsupported grouped having expression kind "${expr.kind}"`);
65+
}
66+
6367
function validateGroupedHavingExpr(expr: AnyExpression): AnyExpression {
6468
return expr.accept<AnyExpression>({
65-
columnRef(expr) {
66-
return expr;
67-
},
68-
identifierRef(expr) {
69-
return expr;
70-
},
71-
subquery(expr) {
72-
return expr;
73-
},
74-
operation(expr) {
75-
return expr;
76-
},
77-
aggregate(expr) {
78-
return expr;
79-
},
80-
jsonObject(expr) {
81-
return expr;
82-
},
83-
jsonArrayAgg(expr) {
84-
return expr;
85-
},
86-
literal(expr) {
87-
return expr;
88-
},
89-
param(expr) {
90-
return expr;
91-
},
92-
list(expr) {
93-
return expr;
69+
columnRef: rejectHavingExpr,
70+
identifierRef: rejectHavingExpr,
71+
subquery: rejectHavingExpr,
72+
operation: rejectHavingExpr,
73+
aggregate: rejectHavingExpr,
74+
jsonObject: rejectHavingExpr,
75+
jsonArrayAgg: rejectHavingExpr,
76+
literal: rejectHavingExpr,
77+
param() {
78+
throw new Error('ParamRef is not supported in grouped having expressions');
9479
},
80+
list: rejectHavingExpr,
9581
and(expr) {
9682
return AndExpr.of(expr.exprs.map((child) => validateGroupedHavingExpr(child)));
9783
},

packages/3-extensions/sql-orm-client/src/where-binding.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,19 +36,19 @@ function bindWhereExprNode(contract: SqlContract<SqlStorage>, expr: AnyExpressio
3636
return expr;
3737
},
3838
subquery(expr) {
39-
return expr;
39+
return bindExpression(contract, expr);
4040
},
4141
operation(expr) {
42-
return expr;
42+
return bindExpression(contract, expr);
4343
},
4444
aggregate(expr) {
45-
return expr;
45+
return bindExpression(contract, expr);
4646
},
4747
jsonObject(expr) {
48-
return expr;
48+
return bindExpression(contract, expr);
4949
},
5050
jsonArrayAgg(expr) {
51-
return expr;
51+
return bindExpression(contract, expr);
5252
},
5353
literal(expr) {
5454
return expr;
@@ -57,7 +57,7 @@ function bindWhereExprNode(contract: SqlContract<SqlStorage>, expr: AnyExpressio
5757
return expr;
5858
},
5959
list(expr) {
60-
return expr;
60+
return bindExpression(contract, expr);
6161
},
6262
binary(expr) {
6363
const left = bindExpression(contract, expr.left);

packages/3-extensions/sql-orm-client/test/query-plan-aggregate.test.ts

Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,38 @@
11
import {
22
AggregateExpr,
33
AndExpr,
4+
type AnyExpression,
45
BinaryExpr,
56
ColumnRef,
67
ExistsExpr,
8+
IdentifierRef,
9+
JsonArrayAggExpr,
10+
JsonObjectExpr,
711
ListExpression,
812
LiteralExpr,
13+
NotExpr,
914
NullCheckExpr,
15+
OperationExpr,
1016
OrExpr,
1117
ParamRef,
1218
ProjectionItem,
1319
SelectAst,
20+
SubqueryExpr,
1421
TableSource,
1522
} from '@prisma-next/sql-relational-core/ast';
1623
import { describe, expect, it } from 'vitest';
1724
import { compileAggregate, compileGroupedAggregate } from '../src/query-plan';
1825
import { bindWhereExpr } from '../src/where-binding';
1926
import { baseContract } from './collection-fixtures';
2027

28+
const defaultAggSpec = {
29+
totalViews: { kind: 'aggregate' as const, fn: 'sum' as const, column: 'views' },
30+
};
31+
32+
function compileWithHaving(having: AnyExpression) {
33+
return compileGroupedAggregate(baseContract, 'posts', [], ['user_id'], defaultAggSpec, having);
34+
}
35+
2136
describe('query plan aggregate', () => {
2237
const filteredViews = bindWhereExpr(
2338
baseContract,
@@ -182,4 +197,161 @@ describe('query plan aggregate', () => {
182197
},
183198
]);
184199
});
200+
201+
describe('validateGroupedHavingExpr rejects non-predicate expression types', () => {
202+
it('rejects ColumnRef', () => {
203+
expect(() => compileWithHaving(ColumnRef.of('posts', 'views'))).toThrow(
204+
'Unsupported grouped having expression kind "column-ref"',
205+
);
206+
});
207+
208+
it('rejects IdentifierRef', () => {
209+
expect(() => compileWithHaving(IdentifierRef.of('some_name'))).toThrow(
210+
'Unsupported grouped having expression kind "identifier-ref"',
211+
);
212+
});
213+
214+
it('rejects SubqueryExpr', () => {
215+
const sub = SubqueryExpr.of(
216+
SelectAst.from(TableSource.named('posts')).withProjection([
217+
ProjectionItem.of('id', ColumnRef.of('posts', 'id')),
218+
]),
219+
);
220+
expect(() => compileWithHaving(sub)).toThrow(
221+
'Unsupported grouped having expression kind "subquery"',
222+
);
223+
});
224+
225+
it('rejects OperationExpr', () => {
226+
const op = OperationExpr.function({
227+
method: 'contains',
228+
forTypeId: 'pg/text@1',
229+
self: ColumnRef.of('posts', 'title'),
230+
args: [LiteralExpr.of('test')],
231+
returns: { kind: 'builtin', type: 'boolean' },
232+
template: 'position({1} in {0}) > 0',
233+
});
234+
expect(() => compileWithHaving(op)).toThrow(
235+
'Unsupported grouped having expression kind "operation"',
236+
);
237+
});
238+
239+
it('rejects bare AggregateExpr', () => {
240+
expect(() => compileWithHaving(AggregateExpr.count())).toThrow(
241+
'Unsupported grouped having expression kind "aggregate"',
242+
);
243+
});
244+
245+
it('rejects JsonObjectExpr', () => {
246+
const json = JsonObjectExpr.fromEntries([
247+
JsonObjectExpr.entry('x', ColumnRef.of('posts', 'id')),
248+
]);
249+
expect(() => compileWithHaving(json)).toThrow(
250+
'Unsupported grouped having expression kind "json-object"',
251+
);
252+
});
253+
254+
it('rejects JsonArrayAggExpr', () => {
255+
const agg = JsonArrayAggExpr.of(ColumnRef.of('posts', 'id'));
256+
expect(() => compileWithHaving(agg)).toThrow(
257+
'Unsupported grouped having expression kind "json-array-agg"',
258+
);
259+
});
260+
261+
it('rejects LiteralExpr', () => {
262+
expect(() => compileWithHaving(LiteralExpr.of(true))).toThrow(
263+
'Unsupported grouped having expression kind "literal"',
264+
);
265+
});
266+
267+
it('rejects top-level ParamRef', () => {
268+
expect(() => compileWithHaving(ParamRef.of(1, { name: 'x', codecId: 'pg/int4@1' }))).toThrow(
269+
'ParamRef is not supported in grouped having expressions',
270+
);
271+
});
272+
273+
it('rejects ListExpression', () => {
274+
expect(() =>
275+
compileWithHaving(ListExpression.of([LiteralExpr.of(1), LiteralExpr.of(2)])),
276+
).toThrow('Unsupported grouped having expression kind "list"');
277+
});
278+
});
279+
280+
describe('validateGroupedHavingExpr rejects invalid expressions inside logical operators', () => {
281+
it('rejects invalid expression inside AND', () => {
282+
expect(() =>
283+
compileWithHaving(
284+
AndExpr.of([
285+
BinaryExpr.gte(AggregateExpr.count(), LiteralExpr.of(5)),
286+
ColumnRef.of('posts', 'views'),
287+
]),
288+
),
289+
).toThrow('Unsupported grouped having expression kind "column-ref"');
290+
});
291+
292+
it('rejects invalid expression inside OR', () => {
293+
expect(() =>
294+
compileWithHaving(
295+
OrExpr.of([
296+
BinaryExpr.gte(AggregateExpr.count(), LiteralExpr.of(5)),
297+
LiteralExpr.of(true),
298+
]),
299+
),
300+
).toThrow('Unsupported grouped having expression kind "literal"');
301+
});
302+
303+
it('rejects invalid expression inside NOT', () => {
304+
expect(() => compileWithHaving(new NotExpr(AggregateExpr.count()))).toThrow(
305+
'Unsupported grouped having expression kind "aggregate"',
306+
);
307+
});
308+
});
309+
310+
describe('validateGroupedHavingExpr accepts valid predicate expressions', () => {
311+
it('accepts NOT wrapping a valid binary', () => {
312+
const plan = compileWithHaving(
313+
new NotExpr(BinaryExpr.gte(AggregateExpr.count(), LiteralExpr.of(5))),
314+
);
315+
expect((plan.ast as SelectAst).having).toBeInstanceOf(NotExpr);
316+
});
317+
318+
it('accepts NOT wrapping NullCheck', () => {
319+
const plan = compileWithHaving(
320+
new NotExpr(NullCheckExpr.isNull(AggregateExpr.sum(ColumnRef.of('posts', 'views')))),
321+
);
322+
expect((plan.ast as SelectAst).having).toBeInstanceOf(NotExpr);
323+
});
324+
325+
it('accepts nested NOT(AND(binary, binary))', () => {
326+
const plan = compileWithHaving(
327+
new NotExpr(
328+
AndExpr.of([
329+
BinaryExpr.gte(AggregateExpr.count(), LiteralExpr.of(1)),
330+
BinaryExpr.lte(AggregateExpr.sum(ColumnRef.of('posts', 'views')), LiteralExpr.of(100)),
331+
]),
332+
),
333+
);
334+
expect((plan.ast as SelectAst).having).toBeInstanceOf(NotExpr);
335+
});
336+
});
337+
338+
describe('validateGroupedComparable rejects invalid right-side expressions', () => {
339+
it('rejects SubqueryExpr on right side of binary', () => {
340+
const sub = SubqueryExpr.of(
341+
SelectAst.from(TableSource.named('posts')).withProjection([
342+
ProjectionItem.of('id', ColumnRef.of('posts', 'id')),
343+
]),
344+
);
345+
expect(() => compileWithHaving(BinaryExpr.gte(AggregateExpr.count(), sub))).toThrow(
346+
'Unsupported comparable kind in grouped having: "subquery"',
347+
);
348+
});
349+
350+
it('rejects JsonObjectExpr on right side of binary', () => {
351+
const json = JsonObjectExpr.fromEntries([JsonObjectExpr.entry('x', LiteralExpr.of(1))]);
352+
expect(() => compileWithHaving(BinaryExpr.gte(AggregateExpr.count(), json))).toThrow(
353+
'Unsupported comparable kind in grouped having: "json-object"',
354+
);
355+
});
356+
});
185357
});

0 commit comments

Comments
 (0)