List:Commits« Previous MessageNext Message »
From:Guilhem Bichot Date:July 30 2010 12:43pm
Subject:Re: bzr commit into mysql-next-mr-opt-backporting branch (roy.lyseng:3222)
Bug#51457
View as plain text  
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
<cut>
> === modified file 'mysql-test/r/subselect_innodb.result'
> --- a/mysql-test/r/subselect_innodb.result	2010-06-19 13:22:14 +0000
> +++ b/mysql-test/r/subselect_innodb.result	2010-07-30 11:54:24 +0000
> @@ -392,3 +392,93 @@ and t2.a='1' AND t1.a=t3.b) > 0;
>  a
>  2
>  DROP TABLE t1,t2,t3;
> +#
> +# Bug#51457 Firstmatch semijoin strategy gives wrong results for
> +#           certain query plans
> +#
> +DROP TABLE IF EXISTS empty, ones, ot1, nt1, nt4, it1, it3, it4;
> +SET @@default_storage_engine='innodb';
> +SET
> @@optimizer_switch='semijoin=on,materialization=off,firstmatch=on,loosescan=off';
> +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).

At your option, you could instead add the testcase into 
subquery_sj_innodb.inc, as it's about semijoin and innodb.

> +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?

> +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?

> +EXPLAIN
> +SELECT *
> +FROM empty as ot1, nt1 AS nt3
> +WHERE ot1.a IN (SELECT it2.a
> +FROM ones AS it2 JOIN it4 ON it2.a=it4.a);
> +id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
> +1	PRIMARY	ot1	ALL	NULL	NULL	NULL	NULL	1	
> +1	PRIMARY	it2	ALL	NULL	NULL	NULL	NULL	1	Using where; FirstMatch(ot1)
> +1	PRIMARY	nt3	ALL	NULL	NULL	NULL	NULL	2	
> +1	PRIMARY	it4	ALL	NULL	NULL	NULL	NULL	8	Using where; FirstMatch(nt3)
> +SELECT *
> +FROM empty as ot1, nt1 AS nt3
> +WHERE ot1.a IN (SELECT it2.a
> +FROM ones AS it2 JOIN it4 ON it2.a=it4.a);
> +a	a
> +DROP TABLE  empty, ones, ot1, nt1, nt4, it1, it3, it4;
> +SET @@default_storage_engine=default;
> +SET @@optimizer_switch=default;
> +SET @@optimizer_join_cache_level=default;
> +# End of bug#51457
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