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
