Hi, Serg, On 06/13/2013 08:16 PM, Sergei Golubchik wrote:
Hi, Alexander!
On Jun 13, Alexander Barkov wrote:
please review a fix for MDEV-4635
Is it for 5.1 or 5.5?
For 5.3.
=== modified file 'mysql-test/r/func_time.result' --- mysql-test/r/func_time.result 2013-03-20 15:13:00 +0000 +++ mysql-test/r/func_time.result 2013-06-13 08:39:15 +0000 @@ -1910,3 +1910,13 @@ SELECT 1 FROM DUAL WHERE MINUTE(TIMEDIFF 1 SELECT 1 FROM DUAL WHERE SECOND(TIMEDIFF(NULL, '12:12:12')); 1 +# +# MDEV-4635 Crash in UNIX_TIMESTAMP(STR_TO_DATE('2020','%Y')) +# +SET TIME_ZONE='+02:00'; +SELECT UNIX_TIMESTAMP(STR_TO_DATE('2020','%Y')); +UNIX_TIMESTAMP(STR_TO_DATE('2020','%Y')) +NULL +Warnings: +Error 1411 Incorrect datetime value: '2020' for function str_to_date +SET TIME_ZONE=DEFAULT;
=== modified file 'mysql-test/r/strict.result' --- mysql-test/r/strict.result 2011-06-05 02:56:06 +0000 +++ mysql-test/r/strict.result 2013-06-13 12:15:43 +0000 @@ -213,11 +213,11 @@ ERROR 22007: Incorrect date value: '2004 INSERT INTO t1 (col1) VALUES(STR_TO_DATE('0.10.2004 15.30','%d.%m.%Y %H.%i')); ERROR 22007: Incorrect date value: '2004-10-00 15:30:00' for column 'col1' at row 1 INSERT INTO t1 (col1) VALUES(STR_TO_DATE('31.9.2004 15.30','%d.%m.%Y %H.%i')); -ERROR 22007: Incorrect date value: '2004-09-31 15:30:00' for column 'col1' at row 1 +ERROR HY000: Incorrect datetime value: '31.9.2004 15.30' for function str_to_date
Hmm, I don't know. I liked the old error message more. Indeed, datetime value '31.9.2004 15.30' is just fine for a function str_to_date(). But '2004-09-31 15:30:00' is not fine for column 'col1' in the strict mode.
Why did that change?
Before the change the warning was generated in Field_timestamp::store_TIME_with_warning. Now this warning is generated much earlier, in Item_func_str_to_date::get_date(), after check_date() is called. The problem was that Item_func_str_to_date: a. checked TIME_NO_ZERO_DATE b. did not check TIME_NO_ZERO_IN_DATE (the bug) c. did not check invalid dates After the patch it: a. checks TIME_NO_ZERO_DATE b. checks TIME_NO_ZERO_IN_DATE c. checks invalid dates I thought it would be good to check everything, for consistency. But a side effect is a different warning. But we can suppress invalid dates detection, so it: a. checks TIME_NO_ZERO_DATE b. checks TIME_NO_ZERO_IN_DATE c. does not check invalid dates The test for invalid dates can be suppressed by passing "fuzzy_date | TIME_INVALID_DATES" to check_date(). See this version of the patch attached. I agree that the old warning is nicer. But I'm not sure what's better. Suppressing the test rather slightly looks like hiding more potential bugs...
=== modified file 'sql/item_timefunc.cc' --- sql/item_timefunc.cc 2013-03-20 15:13:00 +0000 +++ sql/item_timefunc.cc 2013-06-13 08:20:13 +0000 @@ -1126,7 +1125,7 @@ bool Item_func_unix_timestamp::get_times }
MYSQL_TIME ltime; - if (get_arg0_date(<ime, 0)) + if (get_arg0_date(<ime, TIME_NO_ZERO_IN_DATE)) return 1;
This is correct, of course.
Regards, Sergei