prismock
prismock copied to clipboard
Aggregate functions don't work with decimal columns
Problem
The aggregateSum
function doesn't work for values in a Decimal
column.
Given a table like
value (Decimal) |
---|
1 |
2 |
3 |
4 |
Running
{
_sum: {
value: true
}
}
Results in
{
_sum: {
value: "1234"
}
}
Instead of
{
_sum: {
value: Decimal(10)
}
}
Looking deeper, it looks like similar problems exist for avg
too, and possibly for min
and max
(haven't tested those yet).
I can get the math to work properly if I pass a primitive number to Prismock when I insert data, but then the results of queries don't match the types that come out of Prisma.
Proposed Solution
If one or both of the inputs is a Decimal type, use Decimal.add
to add them together. Something like:
if (accumulator instanceof Decimal || currentValue instanceof Decimal) {
return new Decimal(accumulator).plus(new Decimal(currentValue))
}
This wraps the values in a new Decimal
to make sure both sides are the correct type before adding.
This obviously requires updating the types around it to use number | Decimal.
The use of instanceof
is illustrative; I'd consider having a more structural type check instead (specifically, you can check if it has fields of d: number[]
, e: number
, and s: number
. But that's probably just my inherent mistrust of instanceof
in JS talking :) .
If we wanted to get more advanced, we could look at the types of the column in the model and see if it's a decimal type, in which case, we switch to a decimal-friendly version of the aggregator. The same approach could be extended for any other type-specific aggregation variants.
I can get a PR in to do the work, I'd just love guidance on how to implement the fix. The two big questions:
- How do we check if we should use decimal arithmetic? Do we do it based on the type of the column in the model, or inline by detecting the type of the actual values?
- If we do it inline, do we use
instanceof
(simple but can be broken by arcane dependency resolution issues) or a structural type check (less clear and is, at best, a really good guess at the type, but resilient against weird dependency issues)?
Personally, I think I'm leaning towards detecting if we have a 'special' column type and swapping out the implementation of the aggregators accordingly. Probably just for number
and Decimal
for now, but it could be expanded if Prisma adds aggregation support for other types too.