On Tue, Sep 12, 2023 at 2:39 PM Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
>
> On Thu, 24 Aug 2023 at 14:51, Ashutosh Bapat
> <ashutosh.bapat.oss@gmail.com> wrote:
> >
> > The patches still apply. But here's a rebased version with one white
> > space error fixed. Also ran pgindent.
> >
>
> This needs another rebase,
Fixed.
> and it looks like the infinite interval
> input code is broken.
>
The code required to handle 'infinity' as an input value was removed by
d6d1430f404386162831bc32906ad174b2007776. I have added a separate
commit which reverts that commit as 0004, which should be merged into
0003.
> I took a quick look, and had a couple of other review comments:
>
> 1). In interval_mul(), I think "result_sign" would be a more accurate
> name than "result_is_inf" for the local variable.
Fixed as part of 0003.
>
> 2). interval_accum() and interval_accum_inv() don't work correctly
> with infinite intervals. To make them work, they need to count the
> number of infinities seen, to allow them to be subtracted off by the
> inverse function (similar to the code in numeric.c, except for the
> NaN-handling, which will need to be different). Consider, for example:
>
> SELECT x, avg(x) OVER(ROWS BETWEEN CURRENT ROW AND 1 FOLLOWING)
> FROM (VALUES ('1 day'::interval),
> ('3 days'::interval),
> ('infinity'::timestamptz - now()),
> ('4 days'::interval),
> ('6 days'::interval)) v(x);
> ERROR: interval out of range
>
> as compared to:
>
> SELECT x, avg(x) OVER(ROWS BETWEEN CURRENT ROW AND 1 FOLLOWING)
> FROM (VALUES (1::numeric),
> (3::numeric),
> ('infinity'::numeric),
> (4::numeric),
> (6::numeric)) v(x);
>
> x | avg
> ----------+--------------------
> 1 | 2.0000000000000000
> 3 | Infinity
> Infinity | Infinity
> 4 | 5.0000000000000000
> 6 | 6.0000000000000000
> (5 rows)
Nice catch. I agree that we need to do something similar to
numeric_accum and numeric_accum_inv. As part of that also add test for
window aggregates on interval data type. We might also need some fix
to sum(). I am planning to work on this next week but in case somebody
else wants to pick this up here are patches with other things fixed.
--
Best Wishes,
Ashutosh Bapat