List:Commits« Previous MessageNext Message »
From:Evgeny Potemkin Date:August 31 2010 5:39pm
Subject:Re: bzr commit into mysql-next-mr-bugfixing branch
(jorgen.loland:3264) Bug#53562
View as plain text  
IMHO, it worth to generalize the bug report to "it's hard to understand why an 
index isn't used" and an example with a query not using ref access.
In the code you can see that not only ref access is affected. See:

mysql> explain extended select * from t1 where f1 >'999'\G
*************************** 1. row ***************************
            id: 1
   select_type: SIMPLE
         table: t1
          type: range
possible_keys: ci1
           key: ci1
       key_len: 3
           ref: NULL
          rows: 1
      filtered: 100.00
         Extra: Using where; Using index
1 row in set, 1 warning (0.00 sec)

mysql> explain extended select * from t1 where f1 >999\G
*************************** 1. row ***************************
            id: 1
   select_type: SIMPLE
         table: t1
          type: index
possible_keys: ci1
           key: ci1
       key_len: 8
           ref: NULL
          rows: 1076
      filtered: 100.00
         Extra: Using where; Using index
1 row in set, 1 warning (0.00 sec)


As you can see 'range' is _much_ better: 1 row vs 700+ rows (1076 is InnoDB's 
estimate).
It would be wrong to limit this info to ref access only.

Regards, Evgen.


On 08/31/10 12:04, Jorgen Loland wrote:
> 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;
>>> + }
>>> }
>>> }
>>> }
>>>
>>>
>>>
>>>
>>>
>
Thread
bzr commit into mysql-next-mr-bugfixing branch (jorgen.loland:3264) Bug#53562Jorgen Loland31 Aug
  • Re: bzr commit into mysql-next-mr-bugfixing branch(jorgen.loland:3264) Bug#53562Evgeny Potemkin31 Aug
    • Re: bzr commit into mysql-next-mr-bugfixing branch (jorgen.loland:3264)Bug#53562Jorgen Loland31 Aug
      • Re: bzr commit into mysql-next-mr-bugfixing branch(jorgen.loland:3264) Bug#53562Evgeny Potemkin31 Aug
        • Re: bzr commit into mysql-next-mr-bugfixing branch (jorgen.loland:3264)Bug#53562Jorgen Loland1 Sep
          • Re: bzr commit into mysql-next-mr-bugfixing branch(jorgen.loland:3264) Bug#53562Evgeny Potemkin1 Sep