search

graphile / pg-aggregates

Aggregates for PostGraphile connections
83 stars 17 forks source link

What's the best way to aggregate on duration? #55

Open psirenny opened 1 year ago

psirenny commented 1 year ago

Hey @benjie!

Our team is trying to add support for aggregating on durations with an interval data type but noticed that's not supported by the library yet. Is it straightforward to add that support? If so, we can take a stab at implementing it and submit PR. Alternatively, we could switch to representing durations with float instead if you recommend that.

benjie commented 1 year ago

V4 or V5?

psirenny commented 1 year ago

Sorry, I should have mentioned that it's on V5.

benjie commented 1 year ago

Weirdly... we already have interval in the codebase:

https://github.com/graphile/pg-aggregates/blob/975eefb2c13589fe856d14ba66830e29d0b48d72/src/AggregateSpecsPlugin.ts#L92

I think the isNumberLike detection is too narrow: https://github.com/graphile/pg-aggregates/blob/975eefb2c13589fe856d14ba66830e29d0b48d72/src/AggregateSpecsPlugin.ts#L75 / https://github.com/graphile/pg-aggregates/blob/975eefb2c13589fe856d14ba66830e29d0b48d72/src/AggregateSpecsPlugin.ts#L30-L35.

Looks like it should be relatively straightforward to fix this; just expand to isNumberLike || isIntervalLike (or similar).

psirenny commented 1 year ago

Copy that! We'll submit a PR to add support.