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 <union1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,...,25> 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` =<cache>(('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`> <cache>(('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;
> + }
> }
> }
> }
>
>
>
>
