From: Jorgen Loland
Date: August 31 2010 12:04pm
Subject: Re: bzr commit into mysql-next-mr-bugfixing branch (jorgen.loland:3264)
 Bug#53562
List-Archive: http://lists.mysql.com/commits/117221
Message-Id: <4C7CEFD6.9040205@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 8bit

Evgen,

I disagree. In the case below, the index is not used for lookup but for 
scanning. That's perfectly fine and the reason why the text is explicit about 
"for lookup".

On 08/31/2010 05:28 PM, Evgeny Potemkin wrote:
> Hi Jorgen,
>
> The fix is good, but there is a room for improvement. Consider the
> example (it would be good if you include it into your test case):
>
> CREATE TABLE t1 (f1 char(3) PRIMARY KEY, f2 int);
> create table t2(f1 int);
> insert into t2 values (1),(2),(3),(4),(5),(6),(7),(8),(9);
> insert into t1(f1,f2) select A.f1 + B.f1*10 + C.f1*100, A.f1 + B.f1*10 +
> C.f1*100 from t2 A,t2 B,t2 C;
> mysql> EXPLAIN EXTENDED SELECT * FROM t1 WHERE f1=333 and f1 >= '333'
> and f1 <444;
> +----+-------------+-------+-------+---------------+---------+---------+------+------+----------+-------------+
>
> | id | select_type | table | type | possible_keys | key | key_len | ref
> | rows | filtered | Extra |
> +----+-------------+-------+-------+---------------+---------+---------+------+------+----------+-------------+
>
> | 1 | SIMPLE | t1 | range | PRIMARY | PRIMARY | 3 | NULL | 546 | 100.00
> | Using where |
> +----+-------------+-------+-------+---------------+---------+---------+------+------+----------+-------------+
>
> 1 row in set, 2 warnings (0.00 sec)
>
> mysql> show warnings;
> +---------+------+-------------------------------------------------------------------------------------
>
> | Level | Code | Message
> +---------+------+-------------------------------------------------------------------------------------
>
> | Warning | 1708 | Can't use index 'PRIMARY' for lookups due to type
> conversion on field 'f1'
> [skip]
> 2 rows in set (0.00 sec)
>
> And there is 2 ways to fix it:
> 1) (Easy) Correct warning message to be like "Type conversion on '%s'
> could prevent index lookups on index '%s'".
> So it could prevent, but the case when index is used is still possible.
> 2) (Harder) Implement more comprehensive check - first, mark keys which
> we "couldn't use", after all keys are gathered compare it with possible
> keys,
> if marked keys are absent in possible keys then type conversion actually
> prevented marked keys usage.
>
> I personally prefer #1 since it gives a clue on why an index isn't used
> and if the key is still used it points to a potential problem. Also,
> it's much easier and error-proof.
>
> Regards, Evgen.
>
>
> On 08/31/10 07:08, Jorgen Loland wrote:
>> #At file:///export/home/jl208045/mysql/mysql-next-mr-bugfixing-53562/
>> based on revid:jorgen.loland@stripped
>>
>> 3264 Jorgen Loland 2010-08-31
>> BUG#53562: EXPLAIN statement should hint when index is not used
>> due to type conversion
>>
>> An index may not be used for lookup 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 for lookup, 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 lookup, 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/share/errmsg-utf8.txt
>> Added ER_WARN_INDEX_NO_LOOKUP_CAST and
>> ER_WARN_INDEX_NO_LOOKUP_COLLATION for use when an index cannot
>> be used for lookup due to type conversion or collation mismatch.
>> @ sql/sql_select.cc
>> Make EXPLAIN EXTENDED issue a warning if an index cannot be used
>> for lookup due to type conversion of collation mismatch.
>>
>> modified:
>> mysql-test/r/explain.result
>> mysql-test/t/explain.test
>> 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-08-31 07:08:36 +0000
>> @@ -324,4 +324,43 @@ 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');
>> +
>> +# Normally, primary key lookup 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 2 100.00 Using where; Using
>> index
>> +Warnings:
>> +Warning 1708 Can't use index 'PRIMARY' for lookups due to type
>> 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 2 100.00 Using where; Using
>> index
>> +Warnings:
>> +Warning 1709 Can't use index 'PRIMARY' for lookups due to collation
>> mismatch on field 'url'
>> +Note 1003 select `test`.`t1`.`url` AS `url` from `test`.`t1` where
>> (`test`.`t1`.`url` =<cache>(('1' 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-08-31 07:08:36 +0000
>> @@ -279,4 +279,30 @@ 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');
>> +
>> +--echo
>> +--echo # Normally, primary key lookup 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
>> +DROP TABLE t1;
>> +
>> +--echo # End BUG#53562
>> +
>> --echo End of 6.0 tests.
>>
>> === 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-08-31 07:08:36 +0000
>> @@ -6387,3 +6387,7 @@ 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_NO_LOOKUP_CAST
>> + eng "Can't use index '%-.64s' for lookups due to type conversion on
>> field '%-.64s'"
>> +ER_WARN_INDEX_NO_LOOKUP_COLLATION
>> + eng "Can't use index '%-.64s' for lookups due to collation mismatch
>> 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-08-31 07:08:36 +0000
>> @@ -5159,6 +5159,41 @@ static uint get_semi_join_select_list_in
>> }
>>
>> /**
>> + If EXPLAIN EXTENDED, add a warning for each index that cannot be
>> + used for lookup 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
>> + @param field Field used in comparision
>> + @param cant_use_indexes Indexes that cannot be used for lookup
>> + @param errorno The error message number to use in the warning
>> + */
>> +static void
>> +warn_index_not_applicable(THD *thd, const Field *field,
>> + const key_map cant_use_index, const uint errorno)
>> +{
>> + 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,
>> + errorno,
>> + ER(errorno),
>> + 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,11 @@ 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,
>> + ER_WARN_INDEX_NO_LOOKUP_CAST);
>> return;
>> + }
>> }
>> else
>> {
>> @@ -5294,7 +5334,11 @@ 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,
>> + ER_WARN_INDEX_NO_LOOKUP_COLLATION);
>> return;
>> + }
>> }
>> }
>> }
>>
>>
>>
>>
>>

-- 
Jørgen Løland | Senior Software Engineer | +47 73842138
Oracle MySQL
Trondheim, Norway