Thread: Re: [HACKERS] Re: date conversion (was Re: Re: v7.1.1 branched and released on Tuesday ...)
Re: [HACKERS] Re: date conversion (was Re: Re: v7.1.1 branched and released on Tuesday ...)
From
Tom Lane
Date:
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
Re: [HACKERS] Re: date conversion (was Re: Re: v7.1.1 branched and released on Tuesday ...)
From
Thomas Lockhart
Date:
> 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
Re: [HACKERS] Re: date conversion (was Re: Re: v7.1.1 branched and released on Tuesday ...)
From
Tom Lane
Date:
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