From: | Vitaly Burovoy <vitaly(dot)burovoy(at)gmail(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
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 21:27:46 |
Message-ID: | CAKOSWNm5MzcXJy6WBAigcW=BStzGmcxQL7xujQ+c4DkbYwZLfA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 2016-03-16, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> My feeling is we ought to preserve the old behavior here, which would
> involve making JULIAN_MAXYEAR_FOR_TIMESTAMPS format-dependent and
> adjusting the float values for the two derived constants; not much of a
> problem code-wise. I think though that it would break quite a number of
> the proposed new regression tests for the float case.
I think I can be solved by adding new tests for new upper bound for
float values and creating a new version of expected file like
date_1.out, horology_1.out, timestamp_1.out and timestamptz.out (the
original files are for integer values; with prefix "_1" are for float
ones).
> TBH, I thought
> the number of added test cases was rather excessive anyway, so I wouldn't
> have a problem with just leaving out whichever ones don't pass with both
> build options.
The tests checks edge cases. In case of saving bigger interval of
allowed values for float values you'll remove checks when they should
fail. What's the left cases for?
On 2016-03-16, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Actually, it seems a lot of the provided test cases fail for float
> timestamps anyway; there's an assumption that
> 294276-12-31 23:59:59.999999
> 294277-01-01 00:00:00.000000
> are distinct timestamps, which they are not in float mode.
I'm ready to change fractional part '.999999' to '.5' (to avoid issues
of different implementation of floating point on different
architectures) to emphasize "almost reached the threshold".
On 2016-03-16, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> 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'd be inclined to set the lower limit of timestamps as
> '4713-01-01 00:00 GMT BC' just to keep things simple. (The upper limit
> can stay where it is.)
>
> While dates don't have this timezone rotation problem, the documentation
> says that they have the same lower-bound limit as timestamps, and there
> are places in the code that assume that too. Is it okay to move their
> lower bound as well?
I think it is important (see initial letter) since it is out of
supported interval (according to the documentation) and don't work as
expected in some cases (see "to_char(date_trunc('week', '4714-12-28
BC'::date),'day')"). It seems it leads to change
IS_VALID_JULIAN[_FOR_STAMPS] as well to something like:
#define IS_VALID_JULIAN(y,m,d) \
((JULIAN_MINYEAR < (y) && (y) < JULIAN_MAXYEAR)
||(JULIAN_MINYEAR == (y) && (m) == 12 && (d) == 31)
||(JULIAN_MAXYEAR == (y) && (m) == 01 && (d) == 01))
It can be correct if checks for IS_VALID_DATE is added in date.c to
places marked as "IS_VALID_JULIAN is enough for checking..."
All other places are (I'm sure) covered by IS_VALID_DATE/IS_VALID_TIMESTAMP.
What about the question:
On 2016-02-24, Vitaly Burovoy <vitaly(dot)burovoy(at)gmail(dot)com> wrote:
> Also I'm asking for a help because the query (in default TZ='GMT+1'):
> postgres=# SELECT '4714-11-24 00:00:00.000000+00 BC'::timestamptz;
>
> in psql gives a result "4714-11-23 23:00:00-01 BC",
> but in a testing system gives "Sun Nov 23 23:00:00 4714 GMT BC"
> without TZ offset.
Why there is "GMT" instead of "GMT+01"? Is it bug? If so should it be
fixed in this patch or can be fixed later?
--
Best regards,
Vitaly Burovoy
From | Date | Subject | |
---|---|---|---|
Next Message | Jim Nasby | 2016-03-16 21:41:13 | Minor typos in optimizer/README |
Previous Message | David Rowley | 2016-03-16 21:08:10 | Re: Parallel Aggregate |