List:Commits« Previous MessageNext Message »
From:Jorgen Loland Date:September 1 2010 8:51am
Subject:Re: bzr commit into mysql-next-mr-bugfixing branch (jorgen.loland:3264)
Bug#53562
View as plain text  
I see your point. How about this:

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 do ref access on index 'PRIMARY' due to type conversion on 
field 'url'
Warning	1708	Cannot do range access on index 'PRIMARY' due to type conversion on 
field 'url'
Note	1003	select `test`.`t1`.`url` AS `url` from `test`.`t1` where 
(`test`.`t1`.`url` = 1)

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 do range access on index 'PRIMARY' due to type conversion on 
field 'url'
Note	1003	select `test`.`t1`.`url` AS `url` from `test`.`t1` where 
(`test`.`t1`.`url` > 3)

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	1709	Cannot do range access on index 'PRIMARY' due to collation mismatch 
on field 'url'
Note	1003	select `test`.`t1`.`url` AS `url` from `test`.`t1` where 
(`test`.`t1`.`url` > <cache>(('3' collate latin1_german2_ci)))

With this, we have warnings for
  * ref(_or_null): "Cannot do ref access..."
  * range: "Cannot do range access"

I don't think fulltext, ALL, index* will need warnings.

On 08/31/2010 07:39 PM, Evgeny Potemkin wrote:
> 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;
>>>> + }
>>>> }
>>>> }
>>>> }
>>>>
>>>>
>>>>
>>>>
>>>>
>>
>

-- 
Jørgen Løland | Senior Software Engineer | +47 73842138
Oracle MySQL
Trondheim, Norway
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