Hi, Alexander! On May 05, Alexander Barkov wrote:
=== modified file 'mysql-test/r/cast.result' --- mysql-test/r/cast.result 2014-03-26 21:25:38 +0000 +++ mysql-test/r/cast.result 2014-04-09 13:32:17 +0000 @@ -327,7 +327,7 @@ cast(cast(120010203101112.121314 as doub NULL select cast(cast(1.1 as double) as datetime); cast(cast(1.1 as double) as datetime) -0000-00-00 00:00:01 +NULL
Why? I'd expect 2000-00-00 00:00:01, in line with your other changes
select cast(cast(-1.1 as double) as datetime); cast(cast(-1.1 as double) as datetime) NULL
Before the fix: +---------------------+-----------------------+-------------------------+ | CAST(1 AS DATETIME) | CAST(1.1 AS DATETIME) | CAST(1.1e0 AS DATETIME) | +---------------------+-----------------------+-------------------------+ | NULL | 0000-00-00 00:00:01 | 0000-00-00 00:00:01 | +---------------------+-----------------------+-------------------------+ 1 row in set, 1 warning (0.01 sec)
After the fix: +---------------------+-----------------------+-------------------------+ | CAST(1 AS DATETIME) | CAST(1.1 AS DATETIME) | CAST(1.1e0 AS DATETIME) | +---------------------+-----------------------+-------------------------+ | NULL | NULL | NULL | +---------------------+-----------------------+-------------------------+ 1 row in set, 3 warnings (0.00 sec)
The version "after the fix" is more consistent, and is MySQL-5.6 compatible.
Okay, it's the same issue as below. 1 is interpreted as date, YYYYMMDD, 0000-00-01, while 1.1 is read as time, HHMMSS.uuuuuu, 00:00:01.100000 See below.
=== modified file 'mysql-test/r/func_time.result' --- mysql-test/r/func_time.result 2014-03-23 10:22:44 +0000 +++ mysql-test/r/func_time.result 2014-04-15 06:56:53 +0000 @@ -1091,9 +1091,9 @@ NULL select isnull(week(now() + 0)), isnull(week(now() + 0.2)), week(20061108), week(20061108.01), week(20061108085411.000002); isnull(week(now() + 0)) isnull(week(now() + 0.2)) week(20061108) week(20061108.01) week(20061108085411.000002) -0 0 45 NULL 45 +0 0 45 45 45
Why? Here you make 20061108.01 to be casted to datetime as "2006-11-06 ??:??:??" I'm not sure that's a good idea. Old code assumed that a non-zero fractional part *always* means sub-seconds, which automatically means the number should be interpreted as YYYYMMDDHHMMSS.uuuuuu, it cannot be possibly interpreted as YYYYMMDD.uuuuuu
Producing very different results for "CAST(20061108.01 AS DATETIME)" and "CAST(20061108.00 AS DATETIME)" looks confusing for me. I think this is the integer part who should determine how the number converts to date/datetime.
Ignoring the fractional part is confusing too. I'd say that *guessing* what the integer means is always error-prone, it's guessing, after all. So any additional hint that helps to parse the number unambiguosly is, actually, good. We use the logic that a fractional part always means microseconds, and that also allows to interpret the number unambiguosly. I'm not sure this can be changed in a GA version.
Also, the version after fix is MySQL-5.6 compatible:
Yes, but still we don't have to be bug-compatible with 5.6.
=== modified file 'mysql-test/r/ps_2myisam.result' --- mysql-test/r/ps_2myisam.result 2013-11-20 11:05:39 +0000 +++ mysql-test/r/ps_2myisam.result 2014-04-15 04:25:48 +0000 @@ -3256,7 +3256,7 @@ values ( 50, 1.0e+10, 1.0e+10, 1.0e+10, 1.0e+10, 1.0e+10 ) ; Warnings: Warning 1265 Data truncated for column 'c15' at row 1 -Warning 1265 Data truncated for column 'c16' at row 1 +Note 1265 Data truncated for column 'c16' at row 1
What bug number is this for? Doesn't look like any of those five that you've listed.
It converts 1.0e+10 to datetime "2001-00-00 00:00:00". It looks correct to produce a note instead of a warning here.
Would you like it to be reported separately?
At least, committed separately, if possible. I found "bzr shelve" command to be very useful in cases like that.
=== modified file 'sql/field.h' --- sql/field.h 2014-03-26 21:25:38 +0000 +++ sql/field.h 2014-04-11 10:32:12 +0000 @@ -94,6 +94,28 @@ inline uint get_set_pack_length(int elem
/** + Tests if field type is temporal and has date part, + i.e. represents DATE, DATETIME or TIMESTAMP types in SQL. + + @param type Field type, as returned by field->type(). + @retval true If field type is temporal type with date part. + @retval false If field type is not temporal type with date part. +*/ +inline bool is_temporal_type_with_date(enum_field_types type) +{ + switch (type) + { + case MYSQL_TYPE_DATE: + case MYSQL_TYPE_DATETIME: + case MYSQL_TYPE_TIMESTAMP:
What about MYSQL_TYPE_DATETIME2, MYSQL_TYPE_TIMESTAMP2 types?
They should never appear in this context.
MYSQL_TYPE_XXX2 can only be returned from field->real_type(), and should never be returned from item->field_type().
Could you add them with an assert then, please? Like case MYSQL_TYPE_DATE2: case MYSQL_TYPE_DATETIME2: case MYSQL_TYPE_TIMESTAMP2: DBUG_ASSERT(0); // impossible case MYSQL_TYPE_DATE: case MYSQL_TYPE_DATETIME: case MYSQL_TYPE_TIMESTAMP: return true; etc
=== modified file 'sql/item_cmpfunc.cc' --- sql/item_cmpfunc.cc 2014-03-26 21:25:38 +0000 +++ sql/item_cmpfunc.cc 2014-04-15 02:15:58 +0000 @@ -881,7 +886,8 @@ void Arg_comparator::set_datetime_cmp_fu
longlong get_datetime_value(THD *thd, Item ***item_arg, Item **cache_arg, - Item *warn_item, bool *is_null) + Item *warn_item, bool *is_null, + bool convert_time_to_datetime)
do you need this convert_time_to_datetime? It might be enough to use is_temporal_type_with_date(warn_item->field_type())
Conversion is needed only if args[0] is datetime and arg[1] is TIME, or the other way around.
When both args[0] and args[1] are TIMEs, no conversion is needed for performance purposes. So knowing only warn_item->field_type() is not enough.
This chunk implements this logic:
--- sql/item_cmpfunc.cc 2014-03-26 21:25:38 +0000 +++ sql/item_cmpfunc.cc 2014-04-15 02:15:58 +0000 @@ -593,6 +593,11 @@ int Arg_comparator::set_compare_func(Ite switch (type) { case TIME_RESULT: cmp_collation.collation= &my_charset_numeric; + if ((a[0]->field_type() == MYSQL_TYPE_TIME && + is_temporal_type_with_date(b[0]->field_type())) || + (b[0]->field_type() == MYSQL_TYPE_TIME && + is_temporal_type_with_date(a[0]->field_type()))) + convert_time_to_datetime= true;
Yes, that's what I mean. Instead of passing convert_time_to_datetime from the caller, you can put this logic inside get_datetime_value(): if ((item->field_type() == MYSQL_TYPE_TIME && is_temporal_type_with_date(warn_item->field_type())) ... this if() means that TIME value will be compared as datetime, and thus needs to be converted. Regards, Sergei