Hello Georgi!
Here are my comments about your patch:
* Georgi Kodinov <kgeorge@stripped> [08/12/01 16:11]:
> #At file:///home/kgeorge/mysql/work/B39920-5.0-bugteam/ based on
> revid:gshchepa@stripped
>
> 2717 Georgi Kodinov 2008-12-01
> Bug #39920: MySQL cannot deal with Leap Second expression in string literal.
>
> Updated MySQL time handling code to react correctly on UTC leap second
> additions
> that can occur on 30 Jun/1 Jul and 31 Dec/1 Jan in the second after 23:59:59.
> MySQL functions that return the OS current time, like e.g. CURDATE(), NOW()
> etc
> will return :59:59 instead of :59:60.
> As a result the reader will receive :59:59 for two consecutive seconds during
> the
> leap second.
> This fix will not affect the values returned by UNIX_TIMESTAMP() for leap
> seconds.
I think it is worth mentioning that UNIX_TIMESTAMP() will continue to return
0 and warning/error (depending on active sql_mode) for datetimes with leap
(i.e. 60th) second.
> But note that with this change when converting the value returned by
> UNIX_TIMESTAMP()
> to broken down time the correction of leap seconds will still be applied.
> Note that this fix will make a difference *only* if the OS is specially
> configured
> to return leap seconds from the OS time calls.
... or if you use MySQL's time zones with enabled leap second support.
> Added a test case to demonstrate the effect of the fix.
IMO it makes sense to mention that an alternative solution -
enabling full blown support for leap seconds in server was
considered to be too intrusive for 5.0/5.1.
> modified:
> mysql-test/r/timezone3.result
> mysql-test/std_data/Moscow_leap
> mysql-test/t/timezone3.test
> sql/tztime.cc
> sql/tztime.h
>
> per-file messages:
> mysql-test/r/timezone3.result
> Bug #39920: test case
> mysql-test/std_data/Moscow_leap
> Bug #39920: updated the Moscow time zone to Dr. Olson's tzdata 2008i
> to accomodate for the 2008 leap second
> mysql-test/t/timezone3.test
> Bug #39920: test case
> sql/tztime.cc
> Bug #39920: adjust 2008's leap second to :59
> sql/tztime.h
> Bug #39920: adjust 2008's leap second to :59
Well, you are no longer adjusting leap seconds only in 2008.
Please reflect this in comment.
> === modified file 'mysql-test/r/timezone3.result'
> --- a/mysql-test/r/timezone3.result 2004-11-12 15:44:17 +0000
> +++ b/mysql-test/r/timezone3.result 2008-12-01 11:24:29 +0000
> @@ -17,6 +17,9 @@ insert into t1 values
> insert into t1 values
> (unix_timestamp('1981-07-01 03:59:59'),'1981-07-01 03:59:59'),
> (unix_timestamp('1981-07-01 04:00:00'),'1981-07-01 04:00:00');
> +insert into t1 values
> +(unix_timestamp('2009-01-01 02:59:59'),'2009-01-01 02:59:59'),
> +(unix_timestamp('2009-01-01 03:00:00'),'2009-01-01 03:00:00');
> select i, from_unixtime(i), c from t1;
> i from_unixtime(i) c
> 1072904422 2004-01-01 00:00:00 2004-01-01 00:00:00
> @@ -31,6 +34,8 @@ i from_unixtime(i) c
> 1099180821 2004-10-31 02:59:59 2004-10-31 02:59:59
> 362793608 1981-07-01 03:59:59 1981-07-01 03:59:59
> 362793610 1981-07-01 04:00:00 1981-07-01 04:00:00
> +1230768022 2009-01-01 02:59:59 2009-01-01 02:59:59
> +1230768024 2009-01-01 03:00:00 2009-01-01 03:00:00
> drop table t1;
> create table t1 (ts timestamp);
> insert into t1 values (19730101235900), (20040101235900);
> @@ -39,3 +44,6 @@ ts
> 1973-01-01 23:59:00
> 2004-01-01 23:59:00
> drop table t1;
> +SELECT FROM_UNIXTIME(1230768022), FROM_UNIXTIME(1230768023),
> FROM_UNIXTIME(1230768024);
> +FROM_UNIXTIME(1230768022) FROM_UNIXTIME(1230768023) FROM_UNIXTIME(1230768024)
> +2009-01-01 02:59:59 2009-01-01 02:59:59 2009-01-01 03:00:00
>
> === modified file 'mysql-test/std_data/Moscow_leap'
> Binary files a/mysql-test/std_data/Moscow_leap 2004-11-03 17:59:03 +0000 and
> b/mysql-test/std_data/Moscow_leap 2008-12-01 11:24:29 +0000 differ
>
> === modified file 'mysql-test/t/timezone3.test'
> --- a/mysql-test/t/timezone3.test 2005-07-28 00:22:47 +0000
> +++ b/mysql-test/t/timezone3.test 2008-12-01 11:24:29 +0000
> @@ -45,6 +45,10 @@ insert into t1 values
> (unix_timestamp('1981-07-01 03:59:59'),'1981-07-01 03:59:59'),
> (unix_timestamp('1981-07-01 04:00:00'),'1981-07-01 04:00:00');
>
> +insert into t1 values
> + (unix_timestamp('2009-01-01 02:59:59'),'2009-01-01 02:59:59'),
> + (unix_timestamp('2009-01-01 03:00:00'),'2009-01-01 03:00:00');
> +
> select i, from_unixtime(i), c from t1;
> drop table t1;
>
> @@ -58,4 +62,12 @@ insert into t1 values (19730101235900),
> select * from t1;
> drop table t1;
>
> +#
> +# Test Bug #39920: MySQL cannot deal with Leap Second expression in string
> +# literal
> +#
> +
> +# 2009-01-01 02:59:59, 2009-01-01 02:59:60 and 2009-01-01 03:00:00
> +SELECT FROM_UNIXTIME(1230768022), FROM_UNIXTIME(1230768023),
> FROM_UNIXTIME(1230768024);
> +
> # End of 4.1 tests
>
> === modified file 'sql/tztime.cc'
> --- a/sql/tztime.cc 2008-04-03 17:19:55 +0000
> +++ b/sql/tztime.cc 2008-12-01 11:24:29 +0000
> @@ -1073,6 +1073,7 @@ Time_zone_system::gmt_sec_to_TIME(MYSQL_
> localtime_r(&tmp_t, &tmp_tm);
> localtime_to_TIME(tmp, &tmp_tm);
> tmp->time_type= MYSQL_TIMESTAMP_DATETIME;
> + adjust_leap_second(tmp);
> }
>
>
> @@ -1157,6 +1158,7 @@ Time_zone_utc::gmt_sec_to_TIME(MYSQL_TIM
> gmtime_r(&tmp_t, &tmp_tm);
> localtime_to_TIME(tmp, &tmp_tm);
> tmp->time_type= MYSQL_TIMESTAMP_DATETIME;
> + adjust_leap_second(tmp);
> }
>
>
> @@ -1260,6 +1262,7 @@ void
> Time_zone_db::gmt_sec_to_TIME(MYSQL_TIME *tmp, my_time_t t) const
> {
> ::gmt_sec_to_TIME(tmp, t, tz_info);
> + adjust_leap_second(tmp);
> }
>
>
> @@ -2373,6 +2376,30 @@ Time_zone *my_tz_find_with_opening_tz_ta
> DBUG_RETURN(tz);
> }
>
> +
> +/**
> + Convert leap seconds into non-leap
> +
> + This function will convert the leap seconds added by the OS to
> + non-leap seconds, e.g. 23:59:59, 23:59:60 -> 23:59:59, 00:00:01 ...
> + This check is not checking for years on purpoce : altough it's not a
purpose
> + complete check this way it doesn't require looking (and having installed)
> + the leap seconds table.
> +
> + @param[in,out] broken down time structure as filled in by the OS
> +*/
> +
> +void Time_zone::adjust_leap_second(MYSQL_TIME *t)
> +{
> + if (((t->month == 12 && t->day == 31) ||
> + (t->month == 1 && t->day == 1) ||
> + (t->month == 6 && t->day == 30) ||
> + (t->month == 7 && t->day == 1)
> + ) &&
> + t->minute == 59 && (t->second == 60 || t->second == 61))
> + t->second= 59;
> +}
> +
> #endif /* !defined(TESTTIME) && !defined(TZINFO2SQL) */
After some more thinking, I believe that we can safely assume that
the only case when system can return t->second == 60 or 61 is a leap
second and thus simplify above condition as far as "t->second == 60 ||
t->second == 61". Note that this will also minimize performance overhead
of this check. So please consider changing above check once again :)
> === modified file 'sql/tztime.h'
> --- a/sql/tztime.h 2007-05-16 08:44:59 +0000
> +++ b/sql/tztime.h 2008-12-01 11:24:29 +0000
> @@ -55,6 +55,9 @@ public:
> allocated on MEM_ROOT and should not require destruction.
> */
> virtual ~Time_zone() {};
> +
> +protected:
> + static void adjust_leap_second(MYSQL_TIME *t);
> };
>
> extern Time_zone * my_tz_UTC;
I think this patch is an OK solution for 5.0/5.1 and can be
pushed after considering/fixing above issues.
--
Dmitry Lenev, Software Developer
MySQL AB, www.mysql.com
Are you MySQL certified? http://www.mysql.com/certification
