Thread: can insert timestamp value that can't be read

Example:

  => create table r(t timestamp);
  CREATE TABLE
  => insert into r values(
         timestamp '294277-01-09 00:00:00' + interval '36 hour'
       );
  INSERT 0 1
  => select * from r;
  ERROR:  timestamp out of range

(A similar case with different values exists when compiled with
--disable-integer-datetimes.)

It looks like timestamp[tz]_pl_interval() is not doing proper validation
in all paths.

Patch attached. I looked for other areas that might be affected, but
none jumped out.

The patch does not introduce any tests because I couldn't think of a
good way to test both integer and float timestamps properly.

Regards,
    Jeff Davis

Jeff Davis <pgsql@j-davis.com> writes:
> It looks like timestamp[tz]_pl_interval() is not doing proper validation
> in all paths.

> Patch attached. I looked for other areas that might be affected, but
> none jumped out.

This seems to be adding a heck of a lot of cycles (viz, replacing a
simple int64 or float8 addition with timestamp2tm then tm2timestamp)
to fix a corner case that will probably not matter to anyone anytime
in the next 200 thousand years, give or take a few millenia.  I'm okay
with the concept of detecting overflow here, but not at this price.
Can't we do a more direct overflow check, comparable to what ordinary
int64/float8 addition does?

Also, surely timestamp_mi_interval has got the identical issue, and
probably some other operators.

            regards, tom lane

On Wed, 2014-05-07 at 01:00 -0400, Tom Lane wrote:
> Jeff Davis <pgsql@j-davis.com> writes:
> > It looks like timestamp[tz]_pl_interval() is not doing proper validation
> > in all paths.
>
> > Patch attached. I looked for other areas that might be affected, but
> > none jumped out.
>
> This seems to be adding a heck of a lot of cycles (viz, replacing a
> simple int64 or float8 addition with timestamp2tm then tm2timestamp)
> to fix a corner case that will probably not matter to anyone anytime
> in the next 200 thousand years, give or take a few millenia.  I'm okay
> with the concept of detecting overflow here, but not at this price.
> Can't we do a more direct overflow check, comparable to what ordinary
> int64/float8 addition does?

It's a bit more complex than that, because a similar example of the bug
occurs without overflow when float timestamps are enabled. In
timestamp2tm(), the date can end up larger than INT_MAX.

In principle, I think the check in timestamp_pl_interval() can be quite
cheap, because we essentially just need to check for overflow and do a
bounds check. But it's hard to determine the precise valid bounds, and
it seems a little fragile.

I will see if I can extract the important parts of the validity check
into a new function (or macro). It would need to match the other checks
precisely though, otherwise this bug will persist.

> Also, surely timestamp_mi_interval has got the identical issue, and
> probably some other operators.

timestamp_mi_interval() calls timestamp_pl_interval(). I'll take a look
around for other similar problems though.

Regards,
    Jeff Davis