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
