Hello Bar,
Alexander Barkov a écrit, Le 18.03.2011 13:46:
> #At file:///home/bar/mysql-bzr/mysql-trunk.b58329/ based on
> revid:alexander.nozdrin@stripped
>
> 3302 Alexander Barkov 2011-03-18
> Bug#11765369 - Bug#58329: BETWEEN DOES NOT USE INDEXES FOR DATE OR DATETIME
> FIELDS FOR UCS2+
> Problem:
> WHERE date_column = 'ucs2 literal'
> was rewritten to
> WHERE CONVERT(date_column USING ucs2) = 'ucs2 literal'
> so index on date_column did not work any more.
>
> Fix: rewrite to
> WHERE date_column = CONVERT('ucs2 literal' USING latin1)
Without the patch: why did we choose to convert date_column instead of
the literal? There was probably a rule somewhere?
Why is the bug specific of UCS2?
> === modified file 'mysql-test/t/ctype_ucs.test'
> --- a/mysql-test/t/ctype_ucs.test 2011-03-04 14:59:32 +0000
> +++ b/mysql-test/t/ctype_ucs.test 2011-03-18 12:43:19 +0000
> @@ -779,5 +779,15 @@ DO IFNULL(CHAR(NULL USING ucs2), '');
> DO CAST(CONVERT('' USING ucs2) AS UNSIGNED);
>
> --echo #
> +--echo # Bug#11765369 - 58329: BETWEEN DOES NOT USE INDEXES FOR DATE OR DATETIME
> FIELDS FOR UCS2+
> +--echo #
> +SET NAMES utf8, character_set_connection=ucs2;
> +CREATE TABLE t1 (id INT DEFAULT NULL, date_column DATE DEFAULT NULL, KEY
> (date_column)) engine=myisam;
I'd delete "=engine=myisam"; so that the test can also run in InnoDB, in
case one starts MTR with --mysqld=--default-storage-engine=innodb
> +INSERT INTO t1 VALUES (1,'2010-09-01'), (2,'2010-10-01');
> +EXPLAIN EXTENDED SELECT * FROM t1 WHERE date_column= '2010-09-01';
> +DROP TABLE t1;
> +SET NAMES latin1;
> +
> +--echo #
> --echo # End of 5.6 tests
> --echo #
>
> === modified file 'sql/item_cmpfunc.cc'
> --- a/sql/item_cmpfunc.cc 2011-03-17 11:33:17 +0000
> +++ b/sql/item_cmpfunc.cc 2011-03-18 12:43:19 +0000
> @@ -484,10 +484,31 @@ void Item_bool_func2::fix_length_and_dec
>
> DTCollation coll;
> if (args[0]->result_type() == STRING_RESULT &&
> - args[1]->result_type() == STRING_RESULT &&
> - agg_arg_charsets_for_comparison(coll, args, 2))
> - return;
> -
> + args[1]->result_type() == STRING_RESULT)
> + {
> + if (args[0]->is_datetime() || args[1]->is_datetime())
> + {
> + /*
> + For DATE/TIME we always convert to my_charset_numeric (latin1),
> + to make this:
> + WHERE date_column = 'ucs2 literal';
> +
> + rewrite to
> + WHERE date_column = CONVERT('ucs2 literal' USING latin1);
> +
> + instead of
> + WHERE CONVERT(date_column USING ucs2) = 'ucs2 literal';
> +
> + This makes it possible to use index on date_column for optimization.
> + */
> + coll.set(&my_charset_numeric);
> + if (agg_item_set_converter(coll, func_name(), args, 2, 0, 1))
> + return;
> + }
> + else if (agg_arg_charsets_for_comparison(coll, args, 2))
> + return;
> + }
I worry to have one agg* function called for DATE/TIME and another one
called for other types, i.e. I worry about "if/else". I feel that this
could introduce DATE/TIME specific code paths, and thus bugs in the future.
It looks like agg_arg_charsets_for_comparison() calls
agg_item_set_converter(), so can't we do this instead, to avoid if/else:
> if (args[0]->result_type() == STRING_RESULT &&
> + args[1]->result_type() == STRING_RESULT)
> + {
> + if (args[0]->is_datetime() || args[1]->is_datetime())
> + {
> + /*
> + For DATE/TIME we always convert to my_charset_numeric (latin1),
> + to make this:
> + WHERE date_column = 'ucs2 literal';
> +
> + rewrite to
> + WHERE date_column = CONVERT('ucs2 literal' USING latin1);
> +
> + instead of
> + WHERE CONVERT(date_column USING ucs2) = 'ucs2 literal';
> +
> + This makes it possible to use index on date_column for
optimization.
> + */
> + coll.set(&my_charset_numeric);
> + }
> + if (agg_arg_charsets_for_comparison(coll, args, 2))
> + return;
> + }
?
