Re: [Maria-developers] Review request: mdev-4635
Hi Sergei, On 06/14/2013 04:30 PM, Sergei Golubchik wrote:
Hi, Alexander!
On Jun 13, Alexander Barkov wrote:
On 06/13/2013 08:16 PM, Sergei Golubchik wrote:
=== 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.
Which, I think, it wrong. The date is perfectly ok, as far as STR_TO_DATE is concerted, it's wrong for a field.
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.
I agree, it should check everything. But, perhaps, the field should not pass TIME_NO_ZERO_DATE|TIME_NO_ZERO_IN_DATE down to the arg->get_date() ?
It does not. The above warnings are not about zeros. They are about wrong dates: for example, there is no September 31, like in '31.9.2004 15.30'. The last version of the patch does not change the warnings. It works as follows: - str_to_date checks TIME_NO_ZERO_DATE and TIME_NO_ZERO_IN_DATE with help of check_date(), but only if the caller wants it (CONVERT_TZ wants, field does not). - str_to_date suppresses checking of invalid dates inside check_date() and leaves this responsibility to the caller (like it was before the patch). If we want check invalid dates inside check_date() in str_to_date, then this warning "Incorrect date value: '2004-09-31 15:30:00' for column 'col1' at row 1" won't be possible, because str_to_date does not know the name of the column it's being inserted into. But we could try to produce something like this: "Incorrect datetime value: '2004-09-31 15:30:00'" I.e. a better formatted value, but without the column name.
Regards, Sergei
participants (1)
-
Alexander Barkov