Re: [PATCH] Integer overflow in timestamp[tz]_part() and date/time boundaries check

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Vitaly Burovoy <vitaly(dot)burovoy(at)gmail(dot)com>
Cc: Mark Dilger <hornschnorter(at)gmail(dot)com>, David Steele <david(at)pgmasters(dot)net>, Anastasia Lubennikova <a(dot)lubennikova(at)postgrespro(dot)ru>, Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [PATCH] Integer overflow in timestamp[tz]_part() and date/time boundaries check
Date: 2016-03-16 23:19:14
Message-ID: 18671.1458170354@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I wrote:
> I had almost gotten to the point of being willing to commit this patch
> when I noticed that it fails to fix the originally-complained-of-problem:
> ...
> AFAICS the only way that we can avoid a dump/reload hazard is to tighten
> up the allowed range of timestamps by at least one day, so that any
> timestamp that passes IS_VALID_TIMESTAMP() is guaranteed to print, in
> any timezone, with a contained date that the Julian-day routines can
> handle.

I realized that there's a way to do this without reducing the historical
limits of the datatypes. It's already the case that date2j() can handle
dates a little beyond the stated upper limit of the date type, and if
you take a close look at it, you'll realize that it doesn't have a
problem with dates a little before 4714-11-24 BC, either. (It looks
like it would work back to about 4800 BC, though I've not attempted
to verify that independently.) All we really need is for it to return
-1 for 4714-11-23 BC, and it certainly does that. Then, if we allow
such a date to pass IS_VALID_JULIAN, we can accept something like
'4714-11-23 23:00:00-01 BC'::timestamptz and apply the exact range
check only after doing the timestamp adjustment. The same trick
works at the upper end of the range. The key is simply that we have
to have an explicit range check at the end rather than relying entirely
on IS_VALID_JULIAN --- but this patch was about 80% of the way there
already.

So I fixed that up and committed it, with a very short set of new
regression test cases. I seriously doubt that the other ones add
enough value to be worth trying to make them work in both float- and
int-timestamp cases; though if you want to submit a new patch to
add more test cases we could consider it. My feeling about it is that
that kind of testing does nothing for errors of omission (ie functions
that fail to range-check their results), which is the most likely sort
of bug here, and so I doubt it's worth adding them to regression tests
that many people will run many times a day for a long time to come.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David Steele 2016-03-16 23:32:31 Re: Add generate_series(date,date) and generate_series(date,date,integer)
Previous Message David Steele 2016-03-16 23:17:12 Re: Patch: Implement failover on libpq connect level.