Re: Fix overflow in DecodeInterval

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Joseph Koshakow <koshy44(at)gmail(dot)com>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Fix overflow in DecodeInterval
Date: 2022-02-11 20:55:42
Message-ID: 2176379.1644612942@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2022-02-11 21:05:04 Re: Per-table storage parameters for TableAM/IndexAM extensions
Previous Message Robert Haas 2022-02-11 20:48:01 Re: [Proposal] Fully WAL logged CREATE DATABASE - No Checkpoints