mammoth icon indicating copy to clipboard operation
mammoth copied to clipboard

`sum()` doesn't work for non-`number` columns

Open cakoose opened this issue 3 years ago • 4 comments

In my Mammoth schema definition, I'm using big.js for Postgres numeric and JS BigInt for Postgres int8.

The Mammoth sum function doesn't work on those types:

sum: (expression: Expression<number, boolean, any>) => Expression<number, false, "sum">;

The quick fix for me is to define my own sum function with a different type. That's what I'm going to do for now.

But I wonder if there's a more general solution. One option would be to add a new "IsSummable" type parameter to the Expression type. But another type parameter ends up adding a bunch of noise to the IDE auto-complete and error messages.

I think Scala "type members" (called "associated types" in Rust and Swift) might provide a cleaner solution, but I'm not sure if that's possible in TypeScript. Here's one attempt to simulate them in TypeScript: StackOverflow link.

cakoose avatar Dec 29 '20 18:12 cakoose

BTW, this is the workaround I'm using:

import * as mammoth from '@ff00ff/mammoth';

export function sum<T extends BigInt | number | Big>(expression: Expression<T, boolean, any>): Expression<T, true, 'sum'> {
    const casted = expression as Expression<number, boolean, any>;
    return mammoth.sum(casted);
}

Edit: fixed return value from Expression<number, boolean, 'sum'> to Expression<T, false, 'sum'>.

cakoose avatar Dec 29 '20 18:12 cakoose

I think there should be a Number type which can be changed in user-land. Either by passing it to the db (but this requires always importing these function through your db instance) or change the Mammoth declarations in user-land.


So something like type MammothNumber = number; which will be used by sum and all those number operations and it can be altered to e.g. MammothNumber = number | BigInt | Big in user-land.

martijndeh avatar Dec 30 '20 11:12 martijndeh

Somewhat related: I think Mammoth must make a distinction between int8 and int4 in the public interface. In the case of sum() this is probably fine, but when you do math like int8 + int4 we should make sure the correct type is returned (in that case int8). If we just have one type for all numeric types we cannot make the distinction and this must be asserted in user-land.

martijndeh avatar Dec 31 '20 08:12 martijndeh

Hmm I thought I had a fix but it's not working properly yet.

martijndeh avatar Dec 31 '20 10:12 martijndeh