Re: Fix overflow in DecodeInterval

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:
> Joseph Koshakow <koshy44(at)gmail(dot)com> writes:
> > The attached patch fixes an overflow bug in DecodeInterval when applying
> > the units week, decade, century, and millennium. The overflow check logic
> > was modelled after the overflow check at the beginning of `int
> > tm2interval(struct pg_tm *tm, fsec_t fsec, Interval *span);` in timestamp.c.
>
>
> Good catch, but I don't think that tm2interval code is best practice
> anymore.  Rather than bringing "double" arithmetic into the mix,
> you should use the overflow-detecting arithmetic functions in
> src/include/common/int.h.  The existing code here is also pretty
> faulty in that it doesn't notice addition overflow when combining
> multiple units.  So for example, instead of
>
>
>     tm->tm_mday += val * 7;
>
>
> I think we should write something like
>
>
>     if (pg_mul_s32_overflow(val, 7, &tmp))
>         return DTERR_FIELD_OVERFLOW;
>     if (pg_add_s32_overflow(tm->tm_mday, tmp, &tm->tm_mday))
>         return DTERR_FIELD_OVERFLOW;
>
>
> Perhaps some macros could be used to make this more legible?
>
>
>             regards, tom lane
>
>
>     @postgresql

Thanks for the feedback Tom, I've attached an updated patch with
your suggestions. Feel free to rename the horribly named macro.

Also while fixing this I noticed that fractional intervals can also
cause an overflow issue.
postgres=# SELECT INTERVAL '0.1 months 2147483647 days';
     interval
------------------
 -2147483646 days
(1 row)
I haven't looked into it, but it's probably a similar cause.