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.