From: Roy Lyseng Date: September 9 2010 8:23am Subject: Re: bzr commit into mysql-next-mr-bugfixing branch (jorgen.loland:3267) Bug#53562 List-Archive: http://lists.mysql.com/commits/117815 Message-Id: <4C88998B.3020201@oracle.com> MIME-Version: 1.0 Content-Type: multipart/alternative; boundary="------------060602040504000106060008" --------------060602040504000106060008 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 8bit Hi Jørgen, fix is approved, but I have a couple of style issues below that you may consider. Thanks, Roy On 01.09.10 15.04, Jorgen Loland wrote: > #Atfile:///export/home/jl208045/mysql/mysql-next-mr-bugfixing-53562/ based onrevid:bar@stripped > > 3267 Jorgen Loland 2010-09-01 > BUG#53562: EXPLAIN statement should hint when index is not used > due to type conversion > > An index may not be used for ref or range access if > a) a type conversion is needed to compare an indexed > field to a value, or > b) there is a collation mismatch between an indexed field > and the value compared to > > Before, the index would not be used, but no hint > was given that this was the case. With this patch, > EXPLAIN EXTENDED will issue a warning for the cases above. > > Note that even if an index cannot be used for ref or range > access, the index may still be scanned. > @ mysql-test/r/explain.result > Added test for BUG#53562 > @ mysql-test/t/explain.test > Added test for BUG#53562 > @ sql/opt_range.cc > Make EXPLAIN EXTENDED issue a warning if an index cannot be used > for range access due to type conversion of collation mismatch. > @ sql/share/errmsg-utf8.txt > Added ER_WARN_INDEX_NOT_APPLICABLE for use when an index cannot > be used for ref or range access due to type conversion or > collation mismatch. > @ sql/sql_select.cc > Make EXPLAIN EXTENDED issue a warning if an index cannot be used > for ref access due to type conversion of collation mismatch. > > modified: > mysql-test/r/explain.result > mysql-test/t/explain.test > sql/opt_range.cc > sql/share/errmsg-utf8.txt > sql/sql_select.cc > === modified file 'mysql-test/r/explain.result' > --- a/mysql-test/r/explain.result 2010-08-26 12:02:59 +0000 > +++ b/mysql-test/r/explain.result 2010-09-01 13:04:01 +0000 > @@ -324,4 +324,77 @@ id select_type table type possible_keys > 25 UNION NULL NULL NULL NULL NULL NULL NULL No tables used > NULL UNION RESULT ALL NULL NULL NULL NULL NULL > # End BUG#30597 > +# > +# BUG#53562: EXPLAIN statement should hint when > +# index is not used due to type conversion > +# > +CREATE TABLE t1 (url char(1) PRIMARY KEY); > +INSERT INTO t1 VALUES ('1'),('2'),('3'),('4'),('5'); > + > +# Normally, lookup access on primary key is done > +EXPLAIN EXTENDED SELECT * FROM t1 WHERE url='1'; > +id select_type table type possible_keys key key_len ref rows filtered Extra > +1 SIMPLE t1 const PRIMARY PRIMARY 1 const 1 100.00 Using index > +Warnings: > +Note 1003 select '1' AS `url` from `test`.`t1` where 1 > + > +# Test that index can't be used for lookup due to type conversion > +# (comparing char and int) > +SELECT * FROM t1 WHERE url=1; > +url > +1 > +EXPLAIN EXTENDED SELECT * FROM t1 WHERE url=1; > +id select_type table type possible_keys key key_len ref rows filtered Extra > +1 SIMPLE t1 index PRIMARY PRIMARY 1 NULL 5 100.00 Using where; Using index > +Warnings: > +Warning 1708 Cannot use ref access on index 'PRIMARY' due to type or collation conversion on field 'url' > +Warning 1708 Cannot use range access on index 'PRIMARY' due to type or collation conversion on field 'url' > +Note 1003 select `test`.`t1`.`url` AS `url` from `test`.`t1` where (`test`.`t1`.`url` = 1) > + > +# Test that index can't be used for lookup due to collation mismatch > +SELECT * FROM t1 WHERE url='1' collate latin1_german2_ci; > +url > +1 > +EXPLAIN EXTENDED SELECT * FROM t1 WHERE url='1' collate latin1_german2_ci; > +id select_type table type possible_keys key key_len ref rows filtered Extra > +1 SIMPLE t1 index PRIMARY PRIMARY 1 NULL 5 100.00 Using where; Using index > +Warnings: > +Warning 1708 Cannot use ref access on index 'PRIMARY' due to type or collation conversion on field 'url' > +Warning 1708 Cannot use range access on index 'PRIMARY' due to type or collation conversion on field 'url' > +Note 1003 select `test`.`t1`.`url` AS `url` from `test`.`t1` where (`test`.`t1`.`url` =(('1' collate latin1_german2_ci))) > + > +# Normally, range access on primary key is done > +EXPLAIN EXTENDED SELECT * FROM t1 WHERE url>'3'; > +id select_type table type possible_keys key key_len ref rows filtered Extra > +1 SIMPLE t1 range PRIMARY PRIMARY 1 NULL 3 100.00 Using where; Using index > +Warnings: > +Note 1003 select `test`.`t1`.`url` AS `url` from `test`.`t1` where (`test`.`t1`.`url`> '3') > + > +# Test that range access on index can't be done due to type conversion > +# (comparing char and int) > +SELECT * FROM t1 WHERE url>3; > +url > +4 > +5 > +EXPLAIN EXTENDED SELECT * FROM t1 WHERE url>3; > +id select_type table type possible_keys key key_len ref rows filtered Extra > +1 SIMPLE t1 index PRIMARY PRIMARY 1 NULL 5 100.00 Using where; Using index > +Warnings: > +Warning 1708 Cannot use range access on index 'PRIMARY' due to type or collation conversion on field 'url' > +Note 1003 select `test`.`t1`.`url` AS `url` from `test`.`t1` where (`test`.`t1`.`url`> 3) > + > +# Test that range access on index can't be done due to collation mismatch > +SELECT * FROM t1 WHERE url>'3' collate latin1_german2_ci; > +url > +4 > +5 > +EXPLAIN EXTENDED SELECT * FROM t1 WHERE url>'3' collate latin1_german2_ci; > +id select_type table type possible_keys key key_len ref rows filtered Extra > +1 SIMPLE t1 index PRIMARY PRIMARY 1 NULL 5 100.00 Using where; Using index > +Warnings: > +Warning 1708 Cannot use range access on index 'PRIMARY' due to type or collation conversion on field 'url' > +Note 1003 select `test`.`t1`.`url` AS `url` from `test`.`t1` where (`test`.`t1`.`url`> (('3' collate latin1_german2_ci))) > + > +DROP TABLE t1; > +# End BUG#53562 > End of 6.0 tests. > > === modified file 'mysql-test/t/explain.test' > --- a/mysql-test/t/explain.test 2010-08-26 12:02:59 +0000 > +++ b/mysql-test/t/explain.test 2010-09-01 13:04:01 +0000 > @@ -279,4 +279,43 @@ EXPLAIN > > --echo # End BUG#30597 > > +--echo # > +--echo # BUG#53562: EXPLAIN statement should hint when > +--echo # index is not used due to type conversion > +--echo # > + > +CREATE TABLE t1 (url char(1) PRIMARY KEY); > +INSERT INTO t1 VALUES ('1'),('2'),('3'),('4'),('5'); > + > +--echo > +--echo # Normally, lookup access on primary key is done > +EXPLAIN EXTENDED SELECT * FROM t1 WHERE url='1'; > +--echo > +--echo # Test that index can't be used for lookup due to type conversion > +--echo # (comparing char and int) > +SELECT * FROM t1 WHERE url=1; > +EXPLAIN EXTENDED SELECT * FROM t1 WHERE url=1; > +--echo > +--echo # Test that index can't be used for lookup due to collation mismatch > +SELECT * FROM t1 WHERE url='1' collate latin1_german2_ci; > +EXPLAIN EXTENDED SELECT * FROM t1 WHERE url='1' collate latin1_german2_ci; > + > +--echo > +--echo # Normally, range access on primary key is done > +EXPLAIN EXTENDED SELECT * FROM t1 WHERE url>'3'; > +--echo > +--echo # Test that range access on index can't be done due to type conversion > +--echo # (comparing char and int) > +SELECT * FROM t1 WHERE url>3; > +EXPLAIN EXTENDED SELECT * FROM t1 WHERE url>3; > +--echo > +--echo # Test that range access on index can't be done due to collation mismatch > +SELECT * FROM t1 WHERE url>'3' collate latin1_german2_ci; > +EXPLAIN EXTENDED SELECT * FROM t1 WHERE url>'3' collate latin1_german2_ci; > + > +--echo > +DROP TABLE t1; > + > +--echo # End BUG#53562 > + > --echo End of 6.0 tests. > > === modified file 'sql/opt_range.cc' > --- a/sql/opt_range.cc 2010-08-04 10:34:01 +0000 > +++ b/sql/opt_range.cc 2010-09-01 13:04:01 +0000 > @@ -5768,7 +5768,17 @@ get_mm_leaf(RANGE_OPT_PARAM *param, Item > ((Field_str*)field)->charset() != conf_func->compare_collation()&& > !(conf_func->compare_collation()->state& MY_CS_BINSORT&& > (type == Item_func::EQUAL_FUNC || type == Item_func::EQ_FUNC))) > + { > + if (param->thd->lex->describe& DESCRIBE_EXTENDED) > + push_warning_printf(param->thd, > + MYSQL_ERROR::WARN_LEVEL_WARN, > + ER_WARN_INDEX_NOT_APPLICABLE, > + ER(ER_WARN_INDEX_NOT_APPLICABLE), > + "range", > + field->table->key_info[param->real_keynr[key_part->key]].name, Extends beyond column 80, see below for proposal. > + field->field_name); > goto end; > + } > > if (key_part->image_type == Field::itMBR) > { > @@ -5889,7 +5899,17 @@ get_mm_leaf(RANGE_OPT_PARAM *param, Item > if (field->result_type() == STRING_RESULT&& > value->result_type() != STRING_RESULT&& > field->cmp_type() != value->result_type()) > + { > + if (param->thd->lex->describe& DESCRIBE_EXTENDED) > + push_warning_printf(param->thd, > + MYSQL_ERROR::WARN_LEVEL_WARN, > + ER_WARN_INDEX_NOT_APPLICABLE, > + ER(ER_WARN_INDEX_NOT_APPLICABLE), > + "range", > + field->table->key_info[param->real_keynr[key_part->key]].name, > + field->field_name); This line extends well beyond column 80. My proposal is that you reformat as: push_warning_printf( param->thd, MYSQL_ERROR::WARN_LEVEL_WARN, ...., but you may have a better idea... > goto end; > + } > /* For comparison purposes allow invalid dates like 2000-01-32 */ > orig_sql_mode= field->table->in_use->variables.sql_mode; > if (value->real_item()->type() == Item::STRING_ITEM&& > > === modified file 'sql/share/errmsg-utf8.txt' > --- a/sql/share/errmsg-utf8.txt 2010-07-29 14:15:38 +0000 > +++ b/sql/share/errmsg-utf8.txt 2010-09-01 13:04:01 +0000 > @@ -6387,3 +6387,5 @@ ER_TABLES_DIFFERENT_METADATA > ER_ROW_DOES_NOT_MATCH_PARTITION > eng "Found row that does not match the partition" > swe "Hittade rad som inte passar i partitionen" > +ER_WARN_INDEX_NOT_APPLICABLE > + eng "Cannot use %-.64s access on index '%-.64s' due to type or collation conversion on field '%-.64s'" > > === modified file 'sql/sql_select.cc' > --- a/sql/sql_select.cc 2010-08-26 13:54:59 +0000 > +++ b/sql/sql_select.cc 2010-09-01 13:04:01 +0000 > @@ -5159,6 +5159,41 @@ static uint get_semi_join_select_list_in > } > > /** Proposal: Add a "@brief" line here and a "@details section before this (very nice) comment. > + If EXPLAIN EXTENDED, add a warning for each index that cannot be > + used for ref access due to either type conversion or different > + collations on the field used for comparison > + > + Example type conversion (char compared to int): > + > + CREATE TABLE t1 (url char(1) PRIMARY KEY); > + SELECT * FROM t1 WHERE url=1; > + > + Example different collations (danish vs german2): > + > + CREATE TABLE t1 (url char(1) PRIMARY KEY) collate latin1_danish_ci; > + SELECT * FROM t1 WHERE url='1' collate latin1_german2_ci; > + > + @param thd Thread for the connection that submitted the query Lots of whitespace after text here... > + @param field Field used in comparision > + @param cant_use_indexes Indexes that cannot be used for lookup > + */ > +static void > +warn_index_not_applicable(THD *thd, const Field *field, > + const key_map cant_use_index) > +{ > + if (thd->lex->describe& DESCRIBE_EXTENDED) > + for (uint j=0 ; j< field->table->s->keys ; j++) > + if (cant_use_index.is_set(j)) > + push_warning_printf(thd, > + MYSQL_ERROR::WARN_LEVEL_WARN, > + ER_WARN_INDEX_NOT_APPLICABLE, > + ER(ER_WARN_INDEX_NOT_APPLICABLE), > + "ref", > + field->table->key_info[j].name, > + field->field_name); > +} > + > +/** > Add a possible key to array of possible keys if it's usable as a key > > @param key_fields Pointer to add key, if usable > @@ -5183,6 +5218,7 @@ add_key_field(KEY_FIELD **key_fields,uin > Field *field, bool eq_func, Item **value, uint num_values, > table_map usable_tables, SARGABLE_PARAM **sargables) > { > + DBUG_PRINT("info",("add_key_field for field %s",field->field_name)); > uint exists_optimize= 0; > if (!(field->flags& PART_KEY_FLAG)) > { > @@ -5284,7 +5320,10 @@ add_key_field(KEY_FIELD **key_fields,uin > if ((*value)->result_type() != STRING_RESULT) > { > if (field->cmp_type() != (*value)->result_type()) > + { > + warn_index_not_applicable(stat->join->thd, field, possible_keys); > return; > + } > } > else > { > @@ -5294,7 +5333,10 @@ add_key_field(KEY_FIELD **key_fields,uin > */ > if (field->cmp_type() == STRING_RESULT&& > ((Field_str*)field)->charset() != cond->compare_collation()) > + { > + warn_index_not_applicable(stat->join->thd, field, possible_keys); > return; > + } > } > } > } > > > > --------------060602040504000106060008--