Skip to content

Commit 2775082

Browse files
authored
fix(db): tighten aggregate value types (#1862)
* fix(db): tighten aggregate value types * fix(db): preserve supported aggregate generics * fix(db): reject null-only aggregate refs * test(db): document aggregate type oracle * test(db): keep mixed aggregate ordering unsettled
1 parent 3ce5366 commit 2775082

3 files changed

Lines changed: 332 additions & 21 deletions

File tree

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+
Restrict built-in aggregate helpers to their supported value domains so numeric aggregates and min/max no longer advertise impossible runtime result types.

‎packages/db/src/query/builder/functions.ts‎

Lines changed: 67 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -91,13 +91,35 @@ type ExtractType<T> =
9191
? U
9292
: T
9393

94-
// Helper type to determine aggregate return type based on input nullability
95-
type AggregateReturnType<T> =
96-
ExtractType<T> extends infer U
97-
? U extends number | undefined | null | Date | bigint | string
98-
? Aggregate<U>
99-
: Aggregate<number | undefined | null | Date | bigint | string>
100-
: Aggregate<number | undefined | null | Date | bigint | string>
94+
type IsAny<T> = 0 extends 1 & T ? true : false
95+
96+
type AggregateArgument<T, Domain> = T &
97+
(IsAny<ExtractType<T>> extends true
98+
? unknown
99+
: [Exclude<ExtractType<T>, null | undefined>] extends [never]
100+
? never
101+
: [Exclude<ExtractType<T>, null | undefined>] extends [Domain]
102+
? unknown
103+
: never)
104+
105+
type OrderableAggregateValue = number | Date | bigint | string
106+
type AggregateWrapper<T> = RefProxy<T> | RefLeaf<T> | BasicExpression<T>
107+
108+
// Constrained overloads compose through supported generics; these conditional
109+
// fallbacks validate concrete optional/nullish unions and reject unknown.
110+
type NumericAggregateWrapperArgument<T> = AggregateArgument<
111+
AggregateWrapper<T>,
112+
number
113+
>
114+
type OrderableAggregateWrapperArgument<T> = AggregateArgument<
115+
AggregateWrapper<T>,
116+
OrderableAggregateValue
117+
>
118+
type NumericAggregateArgument<T> = AggregateArgument<T, number>
119+
type OrderableAggregateArgument<T> = AggregateArgument<
120+
T,
121+
OrderableAggregateValue
122+
>
101123

102124
// Helper type to determine string function return type based on input nullability
103125
type StringFunctionReturnType<T> =
@@ -644,20 +666,44 @@ export function count(arg: ExpressionLike): Aggregate<number> {
644666
return new Aggregate(`count`, [toExpression(arg)])
645667
}
646668

647-
export function avg<T extends ExpressionLike>(arg: T): AggregateReturnType<T> {
648-
return new Aggregate(`avg`, [toExpression(arg)]) as AggregateReturnType<T>
649-
}
650-
651-
export function sum<T extends ExpressionLike>(arg: T): AggregateReturnType<T> {
652-
return new Aggregate(`sum`, [toExpression(arg)]) as AggregateReturnType<T>
653-
}
654-
655-
export function min<T extends ExpressionLike>(arg: T): AggregateReturnType<T> {
656-
return new Aggregate(`min`, [toExpression(arg)]) as AggregateReturnType<T>
657-
}
658-
659-
export function max<T extends ExpressionLike>(arg: T): AggregateReturnType<T> {
660-
return new Aggregate(`max`, [toExpression(arg)]) as AggregateReturnType<T>
669+
export function avg<T extends number>(arg: T): Aggregate<number>
670+
export function avg<T>(
671+
arg: NumericAggregateWrapperArgument<T>,
672+
): Aggregate<number>
673+
export function avg<T extends ExpressionLike>(
674+
arg: NumericAggregateArgument<T>,
675+
): Aggregate<number>
676+
export function avg(arg: ExpressionLike): Aggregate<number> {
677+
return new Aggregate(`avg`, [toExpression(arg)])
678+
}
679+
680+
export function sum<T extends number>(arg: T): Aggregate<number>
681+
export function sum<T>(
682+
arg: NumericAggregateWrapperArgument<T>,
683+
): Aggregate<number>
684+
export function sum<T extends ExpressionLike>(
685+
arg: NumericAggregateArgument<T>,
686+
): Aggregate<number>
687+
export function sum(arg: ExpressionLike): Aggregate<number> {
688+
return new Aggregate(`sum`, [toExpression(arg)])
689+
}
690+
691+
export function min<T extends OrderableAggregateValue>(arg: T): Aggregate<T>
692+
export function min<T>(arg: OrderableAggregateWrapperArgument<T>): Aggregate<T>
693+
export function min<T extends ExpressionLike>(
694+
arg: OrderableAggregateArgument<T>,
695+
): Aggregate<ExtractType<T>>
696+
export function min(arg: ExpressionLike): Aggregate {
697+
return new Aggregate(`min`, [toExpression(arg)])
698+
}
699+
700+
export function max<T extends OrderableAggregateValue>(arg: T): Aggregate<T>
701+
export function max<T>(arg: OrderableAggregateWrapperArgument<T>): Aggregate<T>
702+
export function max<T extends ExpressionLike>(
703+
arg: OrderableAggregateArgument<T>,
704+
): Aggregate<ExtractType<T>>
705+
export function max(arg: ExpressionLike): Aggregate {
706+
return new Aggregate(`max`, [toExpression(arg)])
661707
}
662708

663709
/**
Lines changed: 260 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,260 @@
1+
import { describe, expectTypeOf, test } from 'vitest'
2+
import { createCollection } from '../../src/collection/index.js'
3+
import { createLiveQueryCollection } from '../../src/query/index.js'
4+
import {
5+
add,
6+
avg,
7+
coalesce,
8+
count,
9+
eq,
10+
max,
11+
min,
12+
sum,
13+
} from '../../src/query/builder/functions.js'
14+
import { mockSyncCollectionOptions } from '../utils.js'
15+
import type { Aggregate, BasicExpression } from '../../src/query/ir.js'
16+
import type { RefProxy } from '../../src/query/builder/ref-proxy.js'
17+
import type { RefLeaf } from '../../src/query/builder/types.js'
18+
import type { OutputWithVirtual } from '../utils.js'
19+
20+
/**
21+
* Which values may cross the public aggregate-builder boundary, and which
22+
* result type does each accepted value produce?
23+
*
24+
* Contract and laws:
25+
* - `sum` and `avg` accept numeric values, expressions, and query refs. They
26+
* return `Aggregate<number>` because the runtime reduces them to numbers.
27+
* - `min` and `max` accept number, string, bigint, or Date domains. Their
28+
* result preserves the accepted value domain.
29+
* - A nullable wrapper remains valid when its non-nullish domain is valid.
30+
* A null-only wrapper and `unknown` have no aggregate value domain.
31+
* - A generic helper constrained to a supported domain must forward its value
32+
* through the same public overloads without widening or failing inference.
33+
*
34+
* Production path and observation cut:
35+
* Calls go through the exported overloads in `query/builder/functions.ts`,
36+
* both directly and from real select and left-join callbacks. TypeScript
37+
* overload resolution is the boundary. `expectTypeOf` observes accepted calls
38+
* and exact result types; `@ts-expect-error` observes rejected calls.
39+
*
40+
* Reach witnesses and fault controls:
41+
* Positive assertions cover raw values, branded values, expressions, refs,
42+
* nullable refs, generic forwarders, and projected query results. Negative
43+
* controls would fail the type test if unsupported values became accepted.
44+
* The broad generic forwarder proves that a weak constraint cannot bypass the
45+
* domain law.
46+
*
47+
* Known omission:
48+
* This partial oracle does not require `min` or `max` to reject a union of
49+
* individually orderable domains such as `number | string`. Mixed-domain
50+
* ordering remains outside the settled contract.
51+
*/
52+
type BrandedAmount = number & { readonly __brand: `amount` }
53+
54+
type AggregateRow = {
55+
id: number
56+
group: string
57+
amount: number
58+
maybeAmount?: number | null
59+
label: string
60+
createdAt: Date
61+
sequence: bigint
62+
enabled: boolean
63+
temporalLike: {
64+
year: number
65+
month: number
66+
day: number
67+
}
68+
}
69+
70+
const rows = createCollection(
71+
mockSyncCollectionOptions<AggregateRow>({
72+
id: `aggregate-value-contracts`,
73+
getKey: (row) => row.id,
74+
initialData: [],
75+
}),
76+
)
77+
78+
const details = createCollection(
79+
mockSyncCollectionOptions<{
80+
id: number
81+
rowId: number
82+
amount: BrandedAmount
83+
}>({
84+
id: `aggregate-value-contract-details`,
85+
getKey: (row) => row.id,
86+
initialData: [],
87+
}),
88+
)
89+
90+
describe(`aggregate value contracts`, () => {
91+
test(`numeric and orderable aggregates expose their runtime result domains`, () => {
92+
const result = createLiveQueryCollection({
93+
query: (q) =>
94+
q
95+
.from({ row: rows })
96+
.groupBy(({ row }) => row.group)
97+
.select(({ row }) => {
98+
expectTypeOf(count(row.maybeAmount)).toEqualTypeOf<
99+
Aggregate<number>
100+
>()
101+
expectTypeOf(sum(row.amount)).toEqualTypeOf<Aggregate<number>>()
102+
expectTypeOf(avg(row.amount)).toEqualTypeOf<Aggregate<number>>()
103+
expectTypeOf(sum(row.maybeAmount)).toEqualTypeOf<
104+
Aggregate<number>
105+
>()
106+
expectTypeOf(avg(row.maybeAmount)).toEqualTypeOf<
107+
Aggregate<number>
108+
>()
109+
expectTypeOf(min(row.label)).toEqualTypeOf<Aggregate<string>>()
110+
expectTypeOf(max(row.createdAt)).toEqualTypeOf<Aggregate<Date>>()
111+
expectTypeOf(min(row.sequence)).toEqualTypeOf<Aggregate<bigint>>()
112+
113+
return {
114+
group: row.group,
115+
count: count(row.maybeAmount),
116+
total: sum(row.amount),
117+
average: avg(row.amount),
118+
maybeTotal: sum(row.maybeAmount),
119+
maybeAverage: avg(row.maybeAmount),
120+
firstLabel: min(row.label),
121+
latest: max(row.createdAt),
122+
firstSequence: min(row.sequence),
123+
}
124+
}),
125+
})
126+
127+
expectTypeOf(result.toArray).toMatchTypeOf<
128+
Array<
129+
OutputWithVirtual<{
130+
group: string
131+
count: number
132+
total: number
133+
average: number
134+
maybeTotal: number
135+
maybeAverage: number
136+
firstLabel: string
137+
latest: Date
138+
firstSequence: bigint
139+
}>
140+
>
141+
>()
142+
})
143+
144+
test(`rejects values outside each aggregate's documented domain`, () => {
145+
const loose = undefined as unknown as RefLeaf<any>
146+
const unknownValue = undefined as unknown as RefLeaf<unknown>
147+
const nullLeaf = undefined as unknown as RefLeaf<null>
148+
const nullProxy = undefined as unknown as RefProxy<null>
149+
150+
expectTypeOf(sum(loose)).toEqualTypeOf<Aggregate<number>>()
151+
expectTypeOf(min(loose)).toEqualTypeOf<Aggregate<any>>()
152+
// @ts-expect-error null-only wrappers have no numeric domain
153+
sum(nullLeaf)
154+
// @ts-expect-error null-only wrappers have no orderable domain
155+
min(nullProxy)
156+
157+
createLiveQueryCollection({
158+
query: (q) =>
159+
q.from({ row: rows }).select(({ row }) => ({
160+
// sum() and avg() are numeric aggregates. String coercion would
161+
// return a number while falsely advertising a string result.
162+
// @ts-expect-error string values are not a sum domain
163+
stringSum: sum(row.label),
164+
// @ts-expect-error dates are not an average domain
165+
dateAverage: avg(row.createdAt),
166+
// min()/max() support number, string, bigint, and Date only.
167+
// @ts-expect-error booleans have no supported aggregate ordering
168+
booleanMinimum: min(row.enabled),
169+
// @ts-expect-error Temporal-like objects are not supported yet
170+
temporalMaximum: max(row.temporalLike),
171+
// @ts-expect-error unknown values must be narrowed first
172+
unknownSum: sum(unknownValue),
173+
// @ts-expect-error null alone has no orderable value domain
174+
nullMinimum: min(null),
175+
})),
176+
})
177+
})
178+
179+
test(`supported generic wrappers compose without widening their domains`, () => {
180+
const sumNumber = <T extends number>(value: T) => sum(value)
181+
const sumNumericRef = <T extends RefLeaf<number | null | undefined>>(
182+
value: T,
183+
) => sum(value)
184+
const avgNumericExpression = <
185+
T extends BasicExpression<number | null | undefined>,
186+
>(
187+
value: T,
188+
) => avg(value)
189+
const maxOrderableValue = <T extends number | string | bigint | Date>(
190+
value: T,
191+
) => max(value)
192+
193+
const branded = 1 as BrandedAmount
194+
const nullableBrandedRef = undefined as unknown as RefLeaf<
195+
BrandedAmount | null | undefined,
196+
true
197+
>
198+
expectTypeOf(sumNumber(branded)).toEqualTypeOf<Aggregate<number>>()
199+
expectTypeOf(sum(1)).toEqualTypeOf<Aggregate<number>>()
200+
expectTypeOf(avg(1)).toEqualTypeOf<Aggregate<number>>()
201+
expectTypeOf(sumNumericRef(nullableBrandedRef)).toEqualTypeOf<
202+
Aggregate<number>
203+
>()
204+
expectTypeOf(avgNumericExpression(add(1, 2))).toEqualTypeOf<
205+
Aggregate<number>
206+
>()
207+
expectTypeOf(sum(coalesce(nullableBrandedRef, 0))).toEqualTypeOf<
208+
Aggregate<number>
209+
>()
210+
expectTypeOf(maxOrderableValue(new Date())).toEqualTypeOf<Aggregate<Date>>()
211+
212+
type BroadExpressionLike =
213+
| Aggregate
214+
| BasicExpression
215+
| RefProxy<any>
216+
| RefLeaf<any>
217+
| string
218+
| number
219+
| boolean
220+
| bigint
221+
| Date
222+
| null
223+
| undefined
224+
| Array<unknown>
225+
226+
const unsupportedBroadForwarder = <T extends BroadExpressionLike>(
227+
value: T,
228+
) => {
229+
// @ts-expect-error an unconstrained expression may not be numeric
230+
sum(value)
231+
// @ts-expect-error an unconstrained expression may not be numeric
232+
avg(value)
233+
// @ts-expect-error an unconstrained expression may not be orderable
234+
min(value)
235+
// @ts-expect-error an unconstrained expression may not be orderable
236+
max(value)
237+
}
238+
239+
expectTypeOf(unsupportedBroadForwarder).toBeFunction()
240+
})
241+
242+
test(`left-join nullable branded refs remain valid numeric inputs`, () => {
243+
createLiveQueryCollection({
244+
query: (q) =>
245+
q
246+
.from({ row: rows })
247+
.leftJoin({ detail: details }, ({ row, detail }) =>
248+
eq(row.id, detail.rowId),
249+
)
250+
.groupBy(({ row }) => row.group)
251+
.select(({ row, detail }) => {
252+
expectTypeOf(sum(detail.amount)).toEqualTypeOf<Aggregate<number>>()
253+
return {
254+
group: row.group,
255+
total: sum(detail.amount),
256+
}
257+
}),
258+
})
259+
})
260+
})

0 commit comments

Comments
 (0)