List:Commits« Previous MessageNext Message »
From:Roy Lyseng Date:January 21 2011 4:05pm
Subject:Re: bzr commit into mysql-trunk branch (roy.lyseng:3287) Bug#57431
View as plain text  
Hi Jørgen,

thank you for the review.

On 20.01.11 11.36, Jorgen Loland wrote:
> Hi Roy,
>
> Thank you for the patch. The code looks good, but I have a few requests. See
> inline.
>
> On 11/24/2010 09:17 AM, Roy Lyseng wrote:
>> #At file:///home/rl136806/mysql/repo/mysql-review/ based on
>> revid:tor.didriksen@stripped
>>
>> 3287 Roy Lyseng 2010-11-24
>> Bug#57431: subquery returns wrong result (semijoin=on) with pred AND
>>
>> The test case contains an outer query with a single table and an
>> IN subquery with two outerjoined tables. The MaterializeScan
>> semijoin strategy is selected, meaning that an outerjoin operation
>> is first performed over join_tabs 0 and 1. After the outerjoin,
>> sub_select_sjm() is called to perform a semijoin between the
>> result of the outerjoin in join_tab 1 and the outer table in
>> join_tab 2. However, join_tab 1 still contains some reminiscent data
>> from the outerjoin operation, hence this operation also behaves like
>> an outerjoin.
>>
>> Fixed by temporarily deleting the outerjoin information from the
>> join_tab representing the result of the outerjoin operation.
>>
> ...
>> === modified file 'mysql-test/include/subquery_sj_innodb.inc'
>> --- a/mysql-test/include/subquery_sj_innodb.inc 2010-07-13 08:14:01 +0000
>> +++ b/mysql-test/include/subquery_sj_innodb.inc 2010-11-24 08:16:05 +0000
>> @@ -114,3 +114,35 @@ explain select 1 from t2 where
>> c2 in (select 1 from t3, t2) and
>> c1 in (select convert(c6,char(1)) from t2);
>> drop table t2, t3;
>> +--echo #
>> +--echo # BUG#57431: subquery returns wrong result (semijoin=on) with pred AND
>> +--echo #
>> +CREATE TABLE c (
>> + col_int_nokey INT DEFAULT NULL,
>> + col_int_key INT DEFAULT NULL,
>> + col_varchar_key VARCHAR(1) DEFAULT NULL,
>> + col_varchar_nokey VARCHAR(1) DEFAULT NULL,
>> + KEY col_int_key(col_int_key),
>> + KEY col_varchar_key(col_varchar_key, col_int_key)
>> +) ENGINE=InnoDB;
>> +INSERT INTO c VALUES (NULL,2,'w','w');
>> +INSERT INTO c VALUES (2,9,'t','t');
>> +INSERT INTO c VALUES (2,3,'n','n');
>> +INSERT INTO c VALUES (4,2,'d','d');
>> +CREATE TABLE bb (
>> + col_int_nokey INT DEFAULT NULL,
>> + col_int_key INT DEFAULT NULL,
>> + col_varchar_key VARCHAR(1) DEFAULT NULL,
>> + col_varchar_nokey VARCHAR(1) DEFAULT NULL,
>> + KEY col_int_key(col_int_key),
>> + KEY col_varchar_key(col_varchar_key, col_int_key)
>> +) ENGINE=InnoDB;
>> +INSERT INTO bb VALUES (8,8,NULL,NULL);
>> +
>> +SELECT col_varchar_key
>> +FROM c
>> +WHERE col_int_nokey IN (SELECT INNR.col_int_nokey
>> + FROM bb LEFT JOIN bb INNR ON INNR.col_varchar_nokey)
>> + AND col_int_nokey = 2;
>> +
>> +DROP TABLE c, bb;
>
> This test case can be significantly reduced while still testing outer join
> inside materializescan semijoin strategy. Try this e.g.:
>
> -----------
> CREATE TABLE t1 (
> i INT
> ) ENGINE=InnoDB;
> INSERT INTO t1 VALUES (2),(4);
>
> CREATE TABLE t2 (
> i INT,
> vc VARCHAR(1)
> ) ENGINE=InnoDB;
> INSERT INTO t2 VALUES (8,NULL);
>
> SELECT i
> FROM t1
> WHERE i IN (SELECT innr.i
> FROM t2 LEFT JOIN t2 innr ON innr.vc)
> AND i = 2;
>
> DROP TABLE t1, t2;
> -----------
>
Thank you. I've replaced the old test case with this simpler one.

>> === modified file 'sql/sql_select.cc'
>> --- a/sql/sql_select.cc 2010-11-16 16:17:25 +0000
>> +++ b/sql/sql_select.cc 2010-11-24 08:16:05 +0000
>> @@ -17027,10 +17027,14 @@ sub_select_sjm(JOIN *join, JOIN_TAB *joi
>> /* Do full scan of the materialized table */
>> JOIN_TAB *last_tab= join_tab + (sjm->table_count - 1);
>>
>> +
>> Item *save_cond= last_tab->select_cond;
>> + st_join_table *save_last_inner= last_tab->last_inner;
>> last_tab->set_select_cond(sjm->join_cond, __LINE__);
>> + last_tab->last_inner= NULL;
>> rc= sub_select(join, last_tab, end_of_records);
>> last_tab->set_select_cond(save_cond, __LINE__);
>> + last_tab->last_inner= save_last_inner;
>> DBUG_RETURN(rc);
>> }
>
> Code looks good, but adding a comment why the outer join info is temporarily
> removed wouldn't hurt.

Done.

Thanks,
Roy

Thread
bzr commit into mysql-trunk branch (roy.lyseng:3287) Bug#57431Roy Lyseng24 Nov
  • Re: bzr commit into mysql-trunk branch (roy.lyseng:3287) Bug#57431Jorgen Loland20 Jan
    • Re: bzr commit into mysql-trunk branch (roy.lyseng:3287) Bug#57431Roy Lyseng21 Jan