List:Commits« Previous MessageNext Message »
From:Olav Sandstaa Date:January 28 2011 11:06am
Subject:Re: bzr commit into mysql-trunk branch (olav.sandstaa:3539) Bug#59186
View as plain text  
Hi Jørgen,

Thanks for the review.

I agree with your comments about the Item::marker and that it should be 
cleaned-up - but not part of this bug fix. I will see if I get the time 
to make a follow-up patch for this, if not we should add it to a "todo 
list". I think is should be a fairly simple change.

Olav


On 01/25/2011 12:36 PM, Jorgen Loland wrote:
> Hi Olav,
>
> Patch approved.
>
> Oh my, what a mess Item::marker is! Your code changes are OK, but 
> Item::marker deserves a proper cleanup, e.g. by replacing the randomly 
> chosen number values with an enum.
>
> I suggest you write a follow-up patch to make marker more clear. If 
> you disagree, we can add it to the long list of optimizer todos instead.
>
> On 01/20/2011 11:32 PM, Olav Sandstaa wrote:
>> #At file:///home/olav/mysql/develop/trunk-tmp/ based on 
>> revid:olav.sandstaa@stripped
>>
>>   3539 Olav Sandstaa    2011-01-20
>>        Fix for Bug#59186 Wrong results of join when ICP is enabled
>>
>>        When index condition pushdown was used an extra row appeared 
>> in the
>>        result set due to parts of the select condition for the second 
>> table
>>        in the JOIN was not evaluated.
>>
>>        This was caused when computing the "remainder" for the second 
>> table's
>>        select condition after pushing down parts of it to the storage
>>        engine. If parts of the select condition was common for both 
>> tables in
>>        the JOIN and the common part was pushed down for the first 
>> table the
>>        common part could have the marker field set to
>>        ICP_COND_USES_INDEX_ONLY during evaluation of 
>> make_cond_for_index()
>>        for the first table. If the common part of the select 
>> condition was
>>        not pushed down for the second table it would still be marked 
>> with
>>        ICP_COND_USES_INDEX_ONLY when computing the remainder for the 
>> select
>>        condition for the second table. This would cause that this 
>> part of the
>>        select condition neither was pushed down to the storage engine 
>> nor
>>        included in the select condition to be evaluated by the server.
>>
>>        The fix for this is to extend make_cond_for_index() so that it 
>> clears
>>        the marker field for the parts of the item tree that it 
>> decides should
>>        not be pushed down to the storage engine. This will prevent that
>>        common items in select conditions for different tables that 
>> have been
>>        marked with ICP_COND_USES_INDEX_ONLY when evaluating one table 
>> does
>>        not keep this value when computing the "remainder" for a 
>> following
>>        table in a JOIN.
>>       @ mysql-test/include/icp_tests.inc
>>          Test case for Bug#59186 Wrong results of join when ICP is 
>> enabled.
>>       @ mysql-test/r/innodb_icp.result
>>          Result file for the test for Bug#59186 Wrong results of join 
>> when
>>          ICP is enabled.
>>       @ mysql-test/r/innodb_icp_none.result
>>          Result file for the test for Bug#59186 Wrong results of join 
>> when
>>          ICP is enabled.
>>       @ mysql-test/r/myisam_icp.result
>>          Result file for the test for Bug#59186 Wrong results of join 
>> when
>>          ICP is enabled.
>>       @ mysql-test/r/myisam_icp_none.result
>>          Result file for the test for Bug#59186 Wrong results of join 
>> when
>>          ICP is enabled.
>>       @ sql/sql_select.cc
>>          Reset the condition's marker field in make_cond_for_index() 
>> when it has
>>          determined that this part of the select condition should not be
>>          included in the condition to be pushed down to the storage 
>> engine.
>>
>>          The reason this must be cleared is that if the condition
>>          is a common part of for the select condition of two tables in
>>          a JOIN operation then the marker field might have gotten the 
>> value
>>          set to ICP_COND_USES_INDEX_ONLY when evaluating the select
>>          condition for the first table. If this is the case we need
>>          to reset it to avoid that this part of the select condition
>>          is wrongly concluded to not be needed in the select condition
>>          to be evaluated by the server after pushing parts of the
>>          condition down to the storage engine.
>>
>>      modified:
>>        mysql-test/include/icp_tests.inc
>>        mysql-test/r/innodb_icp.result
>>        mysql-test/r/innodb_icp_none.result
>>        mysql-test/r/myisam_icp.result
>>        mysql-test/r/myisam_icp_none.result
>>        sql/sql_select.cc
>> === modified file 'mysql-test/include/icp_tests.inc'
>> --- a/mysql-test/include/icp_tests.inc    2011-01-20 14:57:03 +0000
>> +++ b/mysql-test/include/icp_tests.inc    2011-01-20 22:32:32 +0000
>> @@ -724,3 +724,34 @@ WHERE NOT EXISTS
>>   --echo
>>
>>   DROP TABLE t1,t2;
>> +
>> +--echo #
>> +--echo # Bug#59186 Wrong results of join when ICP is enabled
>> +--echo #
>> +
>> +CREATE TABLE t1 (
>> +  pk INTEGER NOT NULL,
>> +  c1 VARCHAR(3) NOT NULL,
>> +  PRIMARY KEY (pk)
>> +);
>> +
>> +INSERT INTO t1 VALUES (1,'y'),(0,'or');
>> +
>> +CREATE TABLE t2 (
>> +  pk INTEGER NOT NULL,
>> +  c1 VARCHAR(3) NOT NULL,
>> +  c2 VARCHAR(6) NOT NULL,
>> +  PRIMARY KEY (pk)
>> +);
>> +
>> +INSERT INTO t2 VALUES (6,'y','RPOYT'),(10,'m','JINQE');
>> +
>> +let query=
>> +SELECT c2 FROM t1 JOIN t2 ON t1.c1 = t2.c1
>> +WHERE (t2.pk<= 4 AND t1.pk IN (2,1)) OR
>> +      (t1.pk>  1 AND t2.pk BETWEEN 6 AND 6);
>> +
>> +eval EXPLAIN $query;
>> +eval $query;
>> +
>> +DROP TABLE t1, t2;
>>
>> === modified file 'mysql-test/r/innodb_icp.result'
>> --- a/mysql-test/r/innodb_icp.result    2011-01-20 14:57:03 +0000
>> +++ b/mysql-test/r/innodb_icp.result    2011-01-20 22:32:32 +0000
>> @@ -672,5 +672,32 @@ id    select_type    table    type    possible_keys
>>   3    DEPENDENT SUBQUERY    t2    unique_subquery    PRIMARY    
>> PRIMARY    4    func    1    Using where; Full scan on NULL key
>>
>>   DROP TABLE t1,t2;
>> +#
>> +# Bug#59186 Wrong results of join when ICP is enabled
>> +#
>> +CREATE TABLE t1 (
>> +pk INTEGER NOT NULL,
>> +c1 VARCHAR(3) NOT NULL,
>> +PRIMARY KEY (pk)
>> +);
>> +INSERT INTO t1 VALUES (1,'y'),(0,'or');
>> +CREATE TABLE t2 (
>> +pk INTEGER NOT NULL,
>> +c1 VARCHAR(3) NOT NULL,
>> +c2 VARCHAR(6) NOT NULL,
>> +PRIMARY KEY (pk)
>> +);
>> +INSERT INTO t2 VALUES (6,'y','RPOYT'),(10,'m','JINQE');
>> +EXPLAIN SELECT c2 FROM t1 JOIN t2 ON t1.c1 = t2.c1
>> +WHERE (t2.pk<= 4 AND t1.pk IN (2,1)) OR
>> +(t1.pk>  1 AND t2.pk BETWEEN 6 AND 6);
>> +id    select_type    table    type    possible_keys    key    
>> key_len    ref    rows    Extra
>> +1    SIMPLE    t1    range    PRIMARY    PRIMARY    4    NULL    
>> 1    Using index condition
>> +1    SIMPLE    t2    range    PRIMARY    PRIMARY    4    NULL    
>> 2    Using index condition; Using where; Using join buffer (BNL, 
>> incremental buffers)
>> +SELECT c2 FROM t1 JOIN t2 ON t1.c1 = t2.c1
>> +WHERE (t2.pk<= 4 AND t1.pk IN (2,1)) OR
>> +(t1.pk>  1 AND t2.pk BETWEEN 6 AND 6);
>> +c2
>> +DROP TABLE t1, t2;
>>   set default_storage_engine= @save_storage_engine;
>>   set optimizer_switch=default;
>>
>> === modified file 'mysql-test/r/innodb_icp_none.result'
>> --- a/mysql-test/r/innodb_icp_none.result    2011-01-20 14:57:03 +0000
>> +++ b/mysql-test/r/innodb_icp_none.result    2011-01-20 22:32:32 +0000
>> @@ -671,5 +671,32 @@ id    select_type    table    type    possible_keys
>>   3    DEPENDENT SUBQUERY    t2    unique_subquery    PRIMARY    
>> PRIMARY    4    func    1    Using where; Full scan on NULL key
>>
>>   DROP TABLE t1,t2;
>> +#
>> +# Bug#59186 Wrong results of join when ICP is enabled
>> +#
>> +CREATE TABLE t1 (
>> +pk INTEGER NOT NULL,
>> +c1 VARCHAR(3) NOT NULL,
>> +PRIMARY KEY (pk)
>> +);
>> +INSERT INTO t1 VALUES (1,'y'),(0,'or');
>> +CREATE TABLE t2 (
>> +pk INTEGER NOT NULL,
>> +c1 VARCHAR(3) NOT NULL,
>> +c2 VARCHAR(6) NOT NULL,
>> +PRIMARY KEY (pk)
>> +);
>> +INSERT INTO t2 VALUES (6,'y','RPOYT'),(10,'m','JINQE');
>> +EXPLAIN SELECT c2 FROM t1 JOIN t2 ON t1.c1 = t2.c1
>> +WHERE (t2.pk<= 4 AND t1.pk IN (2,1)) OR
>> +(t1.pk>  1 AND t2.pk BETWEEN 6 AND 6);
>> +id    select_type    table    type    possible_keys    key    
>> key_len    ref    rows    Extra
>> +1    SIMPLE    t1    range    PRIMARY    PRIMARY    4    NULL    
>> 1    Using where
>> +1    SIMPLE    t2    range    PRIMARY    PRIMARY    4    NULL    
>> 2    Using where; Using join buffer (BNL, incremental buffers)
>> +SELECT c2 FROM t1 JOIN t2 ON t1.c1 = t2.c1
>> +WHERE (t2.pk<= 4 AND t1.pk IN (2,1)) OR
>> +(t1.pk>  1 AND t2.pk BETWEEN 6 AND 6);
>> +c2
>> +DROP TABLE t1, t2;
>>   set default_storage_engine= @save_storage_engine;
>>   set optimizer_switch=default;
>>
>> === modified file 'mysql-test/r/myisam_icp.result'
>> --- a/mysql-test/r/myisam_icp.result    2011-01-20 14:57:03 +0000
>> +++ b/mysql-test/r/myisam_icp.result    2011-01-20 22:32:32 +0000
>> @@ -670,4 +670,31 @@ id    select_type    table    type    possible_keys
>>   3    DEPENDENT SUBQUERY    NULL    NULL    NULL    NULL    NULL    
>> NULL    NULL    Impossible WHERE
>>
>>   DROP TABLE t1,t2;
>> +#
>> +# Bug#59186 Wrong results of join when ICP is enabled
>> +#
>> +CREATE TABLE t1 (
>> +pk INTEGER NOT NULL,
>> +c1 VARCHAR(3) NOT NULL,
>> +PRIMARY KEY (pk)
>> +);
>> +INSERT INTO t1 VALUES (1,'y'),(0,'or');
>> +CREATE TABLE t2 (
>> +pk INTEGER NOT NULL,
>> +c1 VARCHAR(3) NOT NULL,
>> +c2 VARCHAR(6) NOT NULL,
>> +PRIMARY KEY (pk)
>> +);
>> +INSERT INTO t2 VALUES (6,'y','RPOYT'),(10,'m','JINQE');
>> +EXPLAIN SELECT c2 FROM t1 JOIN t2 ON t1.c1 = t2.c1
>> +WHERE (t2.pk<= 4 AND t1.pk IN (2,1)) OR
>> +(t1.pk>  1 AND t2.pk BETWEEN 6 AND 6);
>> +id    select_type    table    type    possible_keys    key    
>> key_len    ref    rows    Extra
>> +1    SIMPLE    t1    range    PRIMARY    PRIMARY    4    NULL    
>> 2    Using index condition
>> +1    SIMPLE    t2    range    PRIMARY    PRIMARY    4    NULL    
>> 2    Using index condition; Using where; Using join buffer (BNL, 
>> incremental buffers)
>> +SELECT c2 FROM t1 JOIN t2 ON t1.c1 = t2.c1
>> +WHERE (t2.pk<= 4 AND t1.pk IN (2,1)) OR
>> +(t1.pk>  1 AND t2.pk BETWEEN 6 AND 6);
>> +c2
>> +DROP TABLE t1, t2;
>>   set optimizer_switch=default;
>>
>> === modified file 'mysql-test/r/myisam_icp_none.result'
>> --- a/mysql-test/r/myisam_icp_none.result    2011-01-20 14:57:03 +0000
>> +++ b/mysql-test/r/myisam_icp_none.result    2011-01-20 22:32:32 +0000
>> @@ -669,4 +669,31 @@ id    select_type    table    type    possible_keys
>>   3    DEPENDENT SUBQUERY    NULL    NULL    NULL    NULL    NULL    
>> NULL    NULL    Impossible WHERE
>>
>>   DROP TABLE t1,t2;
>> +#
>> +# Bug#59186 Wrong results of join when ICP is enabled
>> +#
>> +CREATE TABLE t1 (
>> +pk INTEGER NOT NULL,
>> +c1 VARCHAR(3) NOT NULL,
>> +PRIMARY KEY (pk)
>> +);
>> +INSERT INTO t1 VALUES (1,'y'),(0,'or');
>> +CREATE TABLE t2 (
>> +pk INTEGER NOT NULL,
>> +c1 VARCHAR(3) NOT NULL,
>> +c2 VARCHAR(6) NOT NULL,
>> +PRIMARY KEY (pk)
>> +);
>> +INSERT INTO t2 VALUES (6,'y','RPOYT'),(10,'m','JINQE');
>> +EXPLAIN SELECT c2 FROM t1 JOIN t2 ON t1.c1 = t2.c1
>> +WHERE (t2.pk<= 4 AND t1.pk IN (2,1)) OR
>> +(t1.pk>  1 AND t2.pk BETWEEN 6 AND 6);
>> +id    select_type    table    type    possible_keys    key    
>> key_len    ref    rows    Extra
>> +1    SIMPLE    t1    range    PRIMARY    PRIMARY    4    NULL    
>> 2    Using where
>> +1    SIMPLE    t2    range    PRIMARY    PRIMARY    4    NULL    
>> 2    Using where; Using join buffer (BNL, incremental buffers)
>> +SELECT c2 FROM t1 JOIN t2 ON t1.c1 = t2.c1
>> +WHERE (t2.pk<= 4 AND t1.pk IN (2,1)) OR
>> +(t1.pk>  1 AND t2.pk BETWEEN 6 AND 6);
>> +c2
>> +DROP TABLE t1, t2;
>>   set optimizer_switch=default;
>>
>> === modified file 'sql/sql_select.cc'
>> --- a/sql/sql_select.cc    2011-01-20 09:09:26 +0000
>> +++ b/sql/sql_select.cc    2011-01-20 22:32:32 +0000
>> @@ -10068,7 +10068,15 @@ Item *make_cond_for_index(Item *cond, TA
>>     }
>>
>>     if (!uses_index_fields_only(cond, table, keyno, other_tbls_ok))
>> +  {
>> +    /*
>> +      Reset marker since it might have the value
>> +      ICP_COND_USES_INDEX_ONLY if this condition is part of the select
>> +      condition for multiple tables.
>> +    */
>> +    cond->marker= 0;
>>       return (Item*) 0;
>> +  }
>>     cond->marker= ICP_COND_USES_INDEX_ONLY;
>>     return cond;
>>   }
>>
>>
>>
>>
>>
>

Thread
bzr commit into mysql-trunk branch (olav.sandstaa:3539) Bug#59186Olav Sandstaa20 Jan
  • Re: bzr commit into mysql-trunk branch (olav.sandstaa:3539) Bug#59186Jorgen Loland25 Jan
    • Re: bzr commit into mysql-trunk branch (olav.sandstaa:3539) Bug#59186Olav Sandstaa28 Jan
  • Re: bzr commit into mysql-trunk branch (olav.sandstaa:3539) Bug#59186Roy Lyseng26 Jan
    • Re: bzr commit into mysql-trunk branch (olav.sandstaa:3539) Bug#59186Olav Sandstaa28 Jan
      • Re: bzr commit into mysql-trunk branch (olav.sandstaa:3539) Bug#59186Roy Lyseng28 Jan
        • Re: bzr commit into mysql-trunk branch (olav.sandstaa:3539) Bug#59186Olav Sandstaa28 Jan