Hi, Evgeny.
Thanks for the patch. It seems to me that there is several ways this bug
can be fixed. (See my inline comments) Do you view this as the ideal
way, or have you chosen it because it is the least risky, given that we
are trying to stabilize 5.5?
On 10/01/2010 01:38 PM, Evgeny Potemkin wrote:
> #At file:///work/bzrroot/57095-bug-5.5-bugfixing/ based on
> revid:jon.hauglid@stripped
>
> 3213 Evgeny Potemkin 2010-10-01
> Bug#57095: Wrongly chosen expression cache type led to a wrong result.
> The coalesce function returned DATETIME type due to a DATETIME argument, but
> since it's not a date/time function it can't return correct int value for
It seems to me that it should not be very hard to make coalesce be able
to return the correct int value. I would think it would just be to pass
on the int value from its argument. Is there some inherent reason for
not doing this?
Do we have the same issue with other functions? E.g. CASE?
> it. Nevertheless Item_datetime_cache was chosen to cache coalesce's result
> and that led to a wrong result.
>
> Now Item_datetime_cache is used only for those function that could return
> correct int representation of DATETIME values.
> @ mysql-test/r/type_datetime.result
> Added a test case for the bug#57095.
> @ mysql-test/t/type_datetime.test
> Added a test case for the bug#57095.
> @ sql/item.cc
> Bug#57095: Wrongly chosen expression cache type led to a wrong result.
> Now Item_datetime_cache is used only for those function that could return
> correct int representation of DATETIME values.
>
> modified:
> mysql-test/r/type_datetime.result
> mysql-test/t/type_datetime.test
> sql/item.cc
> === modified file 'mysql-test/r/type_datetime.result'
> --- a/mysql-test/r/type_datetime.result 2010-09-07 06:45:00 +0000
> +++ b/mysql-test/r/type_datetime.result 2010-10-01 11:38:16 +0000
> @@ -680,5 +680,17 @@ id select_type table type possible_keys
> 1 SIMPLE t2 const PRIMARY PRIMARY 8 const 1 Using index
> DROP TABLE t1,t2;
> #
> +# Bug#57095: Wrongly chosen expression cache type led to a wrong
> +# result.
> +#
> +CREATE TABLE t1 (`b` datetime );
> +INSERT INTO t1 VALUES ('2010-01-01 00:00:00'), ('2010-01-01 00:00:00');
> +SELECT * FROM t1 WHERE b<= coalesce(NULL, now());
> +b
> +2010-01-01 00:00:00
> +2010-01-01 00:00:00
> +DROP TABLE t1;
> +#
> +#
> # End of 5.5 tests
> #
>
> === modified file 'mysql-test/t/type_datetime.test'
> --- a/mysql-test/t/type_datetime.test 2010-09-07 06:45:00 +0000
> +++ b/mysql-test/t/type_datetime.test 2010-10-01 11:38:16 +0000
> @@ -485,5 +485,15 @@ explain select * from t2 where f1=STR_TO
> DROP TABLE t1,t2;
>
> --echo #
> +--echo # Bug#57095: Wrongly chosen expression cache type led to a wrong
> +--echo # result.
> +--echo #
> +CREATE TABLE t1 (`b` datetime );
> +INSERT INTO t1 VALUES ('2010-01-01 00:00:00'), ('2010-01-01 00:00:00');
> +SELECT * FROM t1 WHERE b<= coalesce(NULL, now());
> +DROP TABLE t1;
> +--echo #
> +
> +--echo #
Since this, according to the bug report, only happens with InnoDB, I
think we should hard code the engine for this example.
> --echo # End of 5.5 tests
> --echo #
>
> === modified file 'sql/item.cc'
> --- a/sql/item.cc 2010-09-24 13:18:45 +0000
> +++ b/sql/item.cc 2010-10-01 11:38:16 +0000
> @@ -7356,9 +7356,11 @@ Item_cache* Item_cache::get_cache(const
> case DECIMAL_RESULT:
> return new Item_cache_decimal();
> case STRING_RESULT:
> - if (item->field_type() == MYSQL_TYPE_DATE ||
> - item->field_type() == MYSQL_TYPE_DATETIME ||
> - item->field_type() == MYSQL_TYPE_TIME)
> + /* Not all functions that return DATE/TIME are actually DATE/TIME funcs. */
> + if ((item->field_type() == MYSQL_TYPE_DATE ||
> + item->field_type() == MYSQL_TYPE_DATETIME ||
> + item->field_type() == MYSQL_TYPE_TIME)&&
> + (const_cast<Item*>(item))->result_as_longlong())
> return new Item_cache_datetime(item->field_type());
> return new Item_cache_str(item);
If you can use Item_cache_str for TYPE_DATETIME when result is not
available as longlong, why could you not just convert the longlong to a
string in other cases and still use Item_cache_str? What is really the
purpose of Item_cache_datetime?
--
Øystein
> case ROW_RESULT:
>
>
>
>
>
