Re: Fix overflow in DecodeInterval

Joseph Koshakow <koshy44@gmail.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