List:Commits« Previous MessageNext Message »
From:Guilhem Bichot Date:July 30 2010 2:00pm
Subject:Re: bzr commit into mysql-next-mr-opt-backporting branch (roy.lyseng:3222)
Bug#51457
View as plain text  
Hello,

Roy Lyseng a écrit, Le 30.07.2010 15:38:
> Hi Guilhem,
> 
> thanks for the prompt review!
> 
> On 30.07.10 14.43, Guilhem Bichot wrote:
>> Hello Roy,
>>
>> Code fix is ok, comments are only about the test.
>>
>> Roy Lyseng a écrit, Le 30.07.2010 13:54:
>>> #At file:///home/rl136806/mysql/repo/mysql-work2/ based on
>>> revid:roy.lyseng@stripped
>>>
>>> 3222 Roy Lyseng 2010-07-30
>>> Bug#51457: Firstmatch semijoin strategy gives wrong results for
>>> certain query plans
>>> When the optimizer selects a semijoin FirstMatch plan that has
>>> non-correlated tables interspersed with inner tables, the "jumps"
>>> are placed so that execution might become incorrect.
>>> Example: A query with an outer correlated table ot, a non-correlated
>>> table nt and two inner tables it1 and it2 are used in a query and the
>>> optimizer chooses the FirstMatch join order ot - it1 - nt - it2.
>>> The optimizer assigns a "jump" from table it2 to ot. This strategy will
>>> omit duplicates resulting from table nt.
>>> The fix assures that the jump from the last table in a consecutive
>>> range of inner table goes to the most immediately preceeding outer
>>> table.
>>
>> I guess here "outer" means "outer correlated or non-correlated".
>> A very good comment
> 
> I will put in a clarification.

thanks

>>> +SET @@optimizer_join_cache_level=0;
>>
>> I suggest not setting join_cache_level to 0; 0 is not default, it's 
>> never going
>> to be (as 1 is what is 5.1-like, picking 0 as default would be a 
>> functionality
>> regression compared to 5.1).
> 
> My reason for setting join_cache_level to 0 was that any join caching 
> complicates join order calculation, and the chosen order is very 
> sensible to cost, hence I did it like this. If you still want me to 
> change it, I can...

keep it as you like

>>
>> At your option, you could instead add the testcase into 
>> subquery_sj_innodb.inc,
>> as it's about semijoin and innodb.
> 
> The problem is that I need a specific setting of optimizer_switch. All 
> others are just redundant, so I prefer to keep it in subselect_innodb. 
> (Yes, I will never agree with Øystein on this one...)

keep it as you like

>>> +CREATE TABLE empty(a INTEGER);
>>> +CREATE TABLE ones(a INTEGER);
>>> +INSERT INTO ones VALUES(1);
>>> +CREATE TABLE it1(a INTEGER);
>>> +INSERT INTO it1 VALUES(5), (8);
>>> +CREATE TABLE it3(a INTEGER);
>>> +INSERT INTO it3 VALUES(7), (1), (0), (5), (1), (4);
>>> +CREATE TABLE it4(a INTEGER);
>>> +INSERT INTO it4 VALUES(1), (3), (5), (7), (9), (7), (3), (1);
>>> +CREATE TABLE ot1(a INTEGER);
>>> +INSERT INTO ot1 select * from it1;
>>> +CREATE TABLE nt1(a INTEGER);
>>> +INSERT INTO nt1 select * from it1;
>>> +CREATE TABLE nt4(a INTEGER);
>>> +INSERT INTO nt4 select * from it4;
>>> +EXPLAIN
>>> +SELECT *
>>> +FROM nt1 AS nt2
>>> +WHERE 1 IN (SELECT it1.a
>>> +FROM ones AS it1 JOIN it3 ON it1.a=it3.a);
>>
>> Here "ones" is aliased to "it1", but "it1" is also a real table; it's 
>> confusing
>> to pick, as alias name, the name of an existing table. What was the 
>> intention?
> 
> I used some tables from a different test. I see the confusion, so I can 
> try to rename them.

yes, renaming would be good.

>>
>>> +id select_type table type possible_keys key key_len ref rows Extra
>>> +1 PRIMARY it1 ALL NULL NULL NULL NULL 1 Using where; FirstMatch
>>> +1 PRIMARY nt2 ALL NULL NULL NULL NULL 2
>>> +1 PRIMARY it3 ALL NULL NULL NULL NULL 6 Using where; FirstMatch(nt2)
>>> +SELECT *
>>> +FROM nt1 AS nt2
>>> +WHERE 1 IN (SELECT it1.a
>>> +FROM ones AS it1 JOIN it3 ON it1.a=it3.a);
>>> +a
>>> +5
>>> +8
>>> +EXPLAIN
>>> +SELECT *
>>> +FROM nt1 AS nt2, nt4
>>> +WHERE 1 IN (SELECT it1.a
>>> +FROM ones AS it1 JOIN it3 ON it1.a=it3.a);
>>> +id select_type table type possible_keys key key_len ref rows Extra
>>> +1 PRIMARY it1 ALL NULL NULL NULL NULL 1 Using where; FirstMatch
>>> +1 PRIMARY nt2 ALL NULL NULL NULL NULL 2
>>> +1 PRIMARY it3 ALL NULL NULL NULL NULL 6 Using where; FirstMatch(nt2)
>>> +1 PRIMARY nt4 ALL NULL NULL NULL NULL 8
>>> +SELECT *
>>> +FROM nt1 AS nt2, nt4
>>> +WHERE 1 IN (SELECT it1.a
>>> +FROM ones AS it1 JOIN it3 ON it1.a=it3.a);
>>> +a a
>>> +5 1
>>> +5 3
>>> +5 5
>>> +5 7
>>> +5 9
>>> +5 7
>>> +5 3
>>> +5 1
>>> +8 1
>>> +8 3
>>> +8 5
>>> +8 7
>>> +8 9
>>> +8 7
>>> +8 3
>>> +8 1
>>
>> Do you know whether query results are correct (I didn't check)?
>> Would they be incorrect without the code fix?
> 
> I did check that the results were "reasonable" but not "correct" :)

With 5.1-bugteam I get the same so it must be ok.

Thread
bzr commit into mysql-next-mr-opt-backporting branch (roy.lyseng:3222)Bug#51457Roy Lyseng30 Jul
  • Re: bzr commit into mysql-next-mr-opt-backporting branch (roy.lyseng:3222)Bug#51457Guilhem Bichot30 Jul
    • Re: bzr commit into mysql-next-mr-opt-backporting branch (roy.lyseng:3222)Bug#51457Roy Lyseng31 Jul
Re: bzr commit into mysql-next-mr-opt-backporting branch (roy.lyseng:3222)Bug#51457Guilhem Bichot30 Jul