From: Jorgen Loland
Date: January 25 2011 11:36am
Subject: Re: bzr commit into mysql-trunk branch (olav.sandstaa:3539) Bug#59186
List-Archive: http://lists.mysql.com/commits/129549
Message-Id: <4D3EB5AA.3070107@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 8bit

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;
>   }
>
>
>
>
>

-- 
Jørgen Løland | Senior Software Engineer | +47 73842138
Oracle MySQL
Trondheim, Norway