Обсуждение: Re: [HACKERS] Re: date conversion (was Re: Re: v7.1.1 branched and released on Tuesday ...)

I extracted from Ayal the info that he was using timezone
'Asia/Jerusalem'.  That zone has the interesting property that
the DST transitions happen *at midnight*, not at a sane hour like 2AM.
I suspect that that is triggering various & sundry bugs in older
versions of mktime().

On a relatively recent Linux (LinuxPPC 2000/Q4) the worst misbehavior
I can find is
regression=# select timestamp('1993-04-02');       timestamp        ------------------------ 1993-04-02 01:00:00+03(1
row)

which is about the best we can do, seeing as how midnight local time
just plain does not exist on that date in that timezone.

However on an older Linux (RedHat 5.1) I get:
regression=# select timestamp('1993-04-02');       timestamp        ------------------------ 2027-04-11 17:45:25+03(1
row)

which is a tad startling.  Tracing through DecodeDateTime tells the
tale:

(gdb) s
875                                     mktime(tm);
(gdb) p *tm
$2 = {tm_sec = 0, tm_min = 0, tm_hour = 0, tm_mday = 2, tm_mon = 3,  tm_year = 93, tm_wday = 0, tm_yday = 0, tm_isdst =
-1, tm_gmtoff = -1073745925, tm_zone = 0x81420c0 "\203�\ff�E�\001"}
 
(gdb) n
876                                     tm->tm_year += 1900;
(gdb) p *tm
$3 = {tm_sec = 0, tm_min = 0, tm_hour = 0, tm_mday = 2, tm_mon = 3,  tm_year = 93, tm_wday = 0, tm_yday = 0, tm_isdst =
-1, tm_gmtoff = -1073745925, tm_zone = 0x81420c0 "\203�\ff�E�\001"}
 
(gdb) s
877                                     tm->tm_mon += 1;
(gdb) s
880                                     *tzp = -(tm->tm_gmtoff);               /* tm_gmtoff is

Ooops.

I recommend that all uses of tm->tm_gmtoff from mktime() be guarded
along the lines ofif (tm->tm_isdst >= 0)    believe gmtoffelse    assume GMT

However, this still does not account for the reported failure of date()
since that code path doesn't use the returned value of *tzp --- and
indeed I get the right thing from select date('1993-04-02'), despite
the failure of mktime().  Probably the behavior of mktime() in this
situation varies across different glibc releases.  Would some other
folk try
set timezone to 'Asia/Jerusalem';select timestamp('1993-04-02');select date('1993-04-02');

and report what you see?

BTW, I also see
regression=# select timestamp(date('1993-04-02'));ERROR:  Unable to convert date to tm

which is just what you'd expect if mktime() fails for this input;
I suppose there's nothing we can do about that except advise people
to update to a less broken libc...
        regards, tom lane

> I extracted from Ayal the info that he was using timezone
> 'Asia/Jerusalem'.  That zone has the interesting property that
> the DST transitions happen *at midnight*, not at a sane hour like 2AM.
> I suspect that that is triggering various & sundry bugs in older
> versions of mktime().

Ahhh! So "GMT+2" was just an approximation, eh? :/

> On a relatively recent Linux (LinuxPPC 2000/Q4) the worst misbehavior
> I can find is
...
> However on an older Linux (RedHat 5.1) I get:
...
> I recommend that all uses of tm->tm_gmtoff from mktime() be guarded
> along the lines of
>         if (tm->tm_isdst >= 0)
>                 believe gmtoff
>         else
>                 assume GMT

I'm not sure that tm_isdst == -1 is a legitimate indicator for mktime()
failure on all platforms; it indicates "don't know", but afaik there is
no defined behavior for the rest of the fields in that case. Can we be
assured that for all platforms the other fields are not damaged?

> However, this still does not account for the reported failure of date()
> since that code path doesn't use the returned value of *tzp --- and
> indeed I get the right thing from select date('1993-04-02'), despite
> the failure of mktime().  Probably the behavior of mktime() in this
> situation varies across different glibc releases.
...
> which is just what you'd expect if mktime() fails for this input;
> I suppose there's nothing we can do about that except advise people
> to update to a less broken libc...

Not sure how much code we should put in to guard for cases we can't even
test (RH 5.1 is pretty old).
                      - Thomas

Thomas Lockhart <lockhart@alumni.caltech.edu> writes:
> I'm not sure that tm_isdst == -1 is a legitimate indicator for mktime()
> failure on all platforms; it indicates "don't know", but afaik there is
> no defined behavior for the rest of the fields in that case. Can we be
> assured that for all platforms the other fields are not damaged?

We can't; further investigation showed that another form of the problem
was mktime() setting the y/m/d/h/m/s fields one hour earlier than what
it was given --- ie, pass it 00:00:00 of a DST forward transition date,
get back neither 00:00:00 nor 01:00:00 (either of which would be
plausible) but 23:00:00 of the day before!

What I did about this was to coalesce all of the three or four places
that use mktime just to probe for DST status into a single routine
(DetermineLocalTimeZone) that is careful to pass mktime a copy of the
original struct tm.  No matter how brain dead the system mktime is,
it can't screw up the other fields that way ;-).  Then we trust
tm_isdst and tm_gmtoff only if tm_isdst >= 0.  Possibly we'll find
that it'd be a good idea to test also for return value == -1, but
the tm_isdst test seems to be sufficient for the known bug cases.

> Not sure how much code we should put in to guard for cases we can't even
> test (RH 5.1 is pretty old).

Yeah, but the above-described behavior is reported on RH 7.1 (by two
different people).  I'm afraid we can't ignore that...
        regards, tom lane