Hi Olav,
thank you for the fix, the patch is approved.
Roy
On 03.01.11 15.50, Olav Sandstaa wrote:
> #At file:///export/home/tmp/mysql2/opt-bug58838-trunk/ based on
> revid:guilhem.bichot@stripped
>
> 3451 Olav Sandstaa 2011-01-03
> Fix for Bug#58838 Wrong results with HAVING + LIMIT without GROUP BY
> when ICP is enabled.
>
> The wrong result was caused due to "loosing" the HAVING condition
> when changing to use a different index than originally selected by the
> optimizer.
>
> During the optimization phase it was decided to use a secondary index
> for retrieving data from the table. Since ICP was enabled the WHERE
> condition was pushed down to the storage engine. A copy of the WHERE
> condition was kept in the JOINTAB's pre_idx_push_select_cond
> member. In the start of the execution phase it was determined that the
> HAVING condition could be included in the JOINTAB's select
> condition. Then in test_if_skip_sort_order() we (since part of the
> select condition has been pushed down to the storage engine) replace
> the JOINTAB's select condition with the original select condition
> (found in the pre_idx_push_select_cond member). Since
> test_if_skip_sort_order() determines that is will be less costly to
> use the primary key for doing the retrieval of data (due to LIMIT) it
> changes to use the primary key. If it had not changed index then
> test_if_skip_sort_order() would have restored the select condition to
> what it was when it was called. But due to changing index the part of
> the select condition that has been pushed down must also be included
> in the select condition. To achieve this the condition in
> pre_idx_push_select_cond is kept as the JOINTAB's select
> condition. This does not include the HAVING condition and as a result
> the HAVING condition is no longer part of the JOINTAB's select
> condition.
>
> The fix for this problem: When adding the HAVING condition to the
> JOINTAB's select condition also add it to the pre_idx_push_select_cond
> (if this exists).
> @ mysql-test/include/icp_tests.inc
> Test case for Bug#58838 "Wrong results with HAVING + LIMIT without
> GROUP BY when ICP is enabled".
> @ mysql-test/r/innodb_icp.result
> Test case for Bug#58838 "Wrong results with HAVING + LIMIT without
> GROUP BY when ICP is enabled".
> @ mysql-test/r/innodb_icp_all.result
> Test case for Bug#58838 "Wrong results with HAVING + LIMIT without
> GROUP BY when ICP is enabled".
> @ mysql-test/r/innodb_icp_none.result
> Test case for Bug#58838 "Wrong results with HAVING + LIMIT without
> GROUP BY when ICP is enabled".
> @ mysql-test/r/myisam_icp.result
> Test case for Bug#58838 "Wrong results with HAVING + LIMIT without
> GROUP BY when ICP is enabled".
> @ mysql-test/r/myisam_icp_all.result
> Test case for Bug#58838 "Wrong results with HAVING + LIMIT without
> GROUP BY when ICP is enabled".
> @ mysql-test/r/myisam_icp_none.result
> Test case for Bug#58838 "Wrong results with HAVING + LIMIT without
> GROUP BY when ICP is enabled".
> @ sql/sql_select.cc
> When including the HAVING condition as part of the JOINTAB's
> select condition it should also be added to the original
> select condition (stored in pre_idx_push_select_cond) if
> parts of the select condition has been pushed down to the storage
> engine. This is necessary since the pre_idx_push_select_cond
> is used in test_if_skip_sort_order() and might replace the
> select condition in the cases where test_if_skip_sort_order()
> decides to change to use a different index for retrieving data.
>
> modified:
> mysql-test/include/icp_tests.inc
> mysql-test/r/innodb_icp.result
> mysql-test/r/innodb_icp_all.result
> mysql-test/r/innodb_icp_none.result
> mysql-test/r/myisam_icp.result
> mysql-test/r/myisam_icp_all.result
> mysql-test/r/myisam_icp_none.result
> sql/sql_select.cc
> === modified file 'mysql-test/include/icp_tests.inc'
>
> === modified file 'mysql-test/include/icp_tests.inc'
> --- a/mysql-test/include/icp_tests.inc 2010-12-13 15:22:45 +0000
> +++ b/mysql-test/include/icp_tests.inc 2011-01-03 14:50:49 +0000
> @@ -653,3 +653,28 @@
> insert into t1 values ('',1);
> select 1 from t1 where b<= 1 and a<> '';
> drop table t1;
> +
> +--echo #
> +--echo # Bug#58838 "Wrong results with HAVING + LIMIT without GROUP BY when
> +--echo # ICP is enabled"
> +--echo #
> +
> +CREATE TABLE t1 (
> + pk INT NOT NULL,
> + c1 INT,
> + PRIMARY KEY (pk),
> + KEY col_int_key (c1)
> +);
> +
> +INSERT INTO t1 VALUES (1,37);
> +INSERT INTO t1 VALUES (2,8);
> +INSERT INTO t1 VALUES (3,-25);
> +INSERT INTO t1 VALUES (4,NULL);
> +INSERT INTO t1 VALUES (5,55);
> +
> +SELECT pk FROM t1 WHERE c1<> 1 HAVING pk = 3 ORDER BY pk LIMIT 0;
> +SELECT pk FROM t1 WHERE c1<> 1 HAVING pk = 3 ORDER BY pk LIMIT 1;
> +SELECT pk FROM t1 WHERE c1<> 1 HAVING pk = 3 ORDER BY pk LIMIT 2;
> +SELECT pk FROM t1 WHERE c1<> 1 HAVING pk = 3 ORDER BY pk LIMIT 5;
> +
> +DROP TABLE t1;
>
> === modified file 'mysql-test/r/innodb_icp.result'
> --- a/mysql-test/r/innodb_icp.result 2010-12-13 15:22:45 +0000
> +++ b/mysql-test/r/innodb_icp.result 2011-01-03 14:50:49 +0000
> @@ -598,5 +598,32 @@
> select 1 from t1 where b<= 1 and a<> '';
> 1
> drop table t1;
> +#
> +# Bug#58838 "Wrong results with HAVING + LIMIT without GROUP BY when
> +# ICP is enabled"
> +#
> +CREATE TABLE t1 (
> +pk INT NOT NULL,
> +c1 INT,
> +PRIMARY KEY (pk),
> +KEY col_int_key (c1)
> +);
> +INSERT INTO t1 VALUES (1,37);
> +INSERT INTO t1 VALUES (2,8);
> +INSERT INTO t1 VALUES (3,-25);
> +INSERT INTO t1 VALUES (4,NULL);
> +INSERT INTO t1 VALUES (5,55);
> +SELECT pk FROM t1 WHERE c1<> 1 HAVING pk = 3 ORDER BY pk LIMIT 0;
> +pk
> +SELECT pk FROM t1 WHERE c1<> 1 HAVING pk = 3 ORDER BY pk LIMIT 1;
> +pk
> +3
> +SELECT pk FROM t1 WHERE c1<> 1 HAVING pk = 3 ORDER BY pk LIMIT 2;
> +pk
> +3
> +SELECT pk FROM t1 WHERE c1<> 1 HAVING pk = 3 ORDER BY pk LIMIT 5;
> +pk
> +3
> +DROP TABLE t1;
> set default_storage_engine= @save_storage_engine;
> set optimizer_switch=default;
>
> === modified file 'mysql-test/r/innodb_icp_all.result'
> --- a/mysql-test/r/innodb_icp_all.result 2010-12-13 15:22:45 +0000
> +++ b/mysql-test/r/innodb_icp_all.result 2011-01-03 14:50:49 +0000
> @@ -598,5 +598,32 @@
> select 1 from t1 where b<= 1 and a<> '';
> 1
> drop table t1;
> +#
> +# Bug#58838 "Wrong results with HAVING + LIMIT without GROUP BY when
> +# ICP is enabled"
> +#
> +CREATE TABLE t1 (
> +pk INT NOT NULL,
> +c1 INT,
> +PRIMARY KEY (pk),
> +KEY col_int_key (c1)
> +);
> +INSERT INTO t1 VALUES (1,37);
> +INSERT INTO t1 VALUES (2,8);
> +INSERT INTO t1 VALUES (3,-25);
> +INSERT INTO t1 VALUES (4,NULL);
> +INSERT INTO t1 VALUES (5,55);
> +SELECT pk FROM t1 WHERE c1<> 1 HAVING pk = 3 ORDER BY pk LIMIT 0;
> +pk
> +SELECT pk FROM t1 WHERE c1<> 1 HAVING pk = 3 ORDER BY pk LIMIT 1;
> +pk
> +3
> +SELECT pk FROM t1 WHERE c1<> 1 HAVING pk = 3 ORDER BY pk LIMIT 2;
> +pk
> +3
> +SELECT pk FROM t1 WHERE c1<> 1 HAVING pk = 3 ORDER BY pk LIMIT 5;
> +pk
> +3
> +DROP TABLE t1;
> 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 2010-12-13 15:22:45 +0000
> +++ b/mysql-test/r/innodb_icp_none.result 2011-01-03 14:50:49 +0000
> @@ -597,5 +597,32 @@
> select 1 from t1 where b<= 1 and a<> '';
> 1
> drop table t1;
> +#
> +# Bug#58838 "Wrong results with HAVING + LIMIT without GROUP BY when
> +# ICP is enabled"
> +#
> +CREATE TABLE t1 (
> +pk INT NOT NULL,
> +c1 INT,
> +PRIMARY KEY (pk),
> +KEY col_int_key (c1)
> +);
> +INSERT INTO t1 VALUES (1,37);
> +INSERT INTO t1 VALUES (2,8);
> +INSERT INTO t1 VALUES (3,-25);
> +INSERT INTO t1 VALUES (4,NULL);
> +INSERT INTO t1 VALUES (5,55);
> +SELECT pk FROM t1 WHERE c1<> 1 HAVING pk = 3 ORDER BY pk LIMIT 0;
> +pk
> +SELECT pk FROM t1 WHERE c1<> 1 HAVING pk = 3 ORDER BY pk LIMIT 1;
> +pk
> +3
> +SELECT pk FROM t1 WHERE c1<> 1 HAVING pk = 3 ORDER BY pk LIMIT 2;
> +pk
> +3
> +SELECT pk FROM t1 WHERE c1<> 1 HAVING pk = 3 ORDER BY pk LIMIT 5;
> +pk
> +3
> +DROP TABLE t1;
> 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 2010-12-13 15:22:45 +0000
> +++ b/mysql-test/r/myisam_icp.result 2011-01-03 14:50:49 +0000
> @@ -596,4 +596,31 @@
> select 1 from t1 where b<= 1 and a<> '';
> 1
> drop table t1;
> +#
> +# Bug#58838 "Wrong results with HAVING + LIMIT without GROUP BY when
> +# ICP is enabled"
> +#
> +CREATE TABLE t1 (
> +pk INT NOT NULL,
> +c1 INT,
> +PRIMARY KEY (pk),
> +KEY col_int_key (c1)
> +);
> +INSERT INTO t1 VALUES (1,37);
> +INSERT INTO t1 VALUES (2,8);
> +INSERT INTO t1 VALUES (3,-25);
> +INSERT INTO t1 VALUES (4,NULL);
> +INSERT INTO t1 VALUES (5,55);
> +SELECT pk FROM t1 WHERE c1<> 1 HAVING pk = 3 ORDER BY pk LIMIT 0;
> +pk
> +SELECT pk FROM t1 WHERE c1<> 1 HAVING pk = 3 ORDER BY pk LIMIT 1;
> +pk
> +3
> +SELECT pk FROM t1 WHERE c1<> 1 HAVING pk = 3 ORDER BY pk LIMIT 2;
> +pk
> +3
> +SELECT pk FROM t1 WHERE c1<> 1 HAVING pk = 3 ORDER BY pk LIMIT 5;
> +pk
> +3
> +DROP TABLE t1;
> set optimizer_switch=default;
>
> === modified file 'mysql-test/r/myisam_icp_all.result'
> --- a/mysql-test/r/myisam_icp_all.result 2010-12-13 15:22:45 +0000
> +++ b/mysql-test/r/myisam_icp_all.result 2011-01-03 14:50:49 +0000
> @@ -596,4 +596,31 @@
> select 1 from t1 where b<= 1 and a<> '';
> 1
> drop table t1;
> +#
> +# Bug#58838 "Wrong results with HAVING + LIMIT without GROUP BY when
> +# ICP is enabled"
> +#
> +CREATE TABLE t1 (
> +pk INT NOT NULL,
> +c1 INT,
> +PRIMARY KEY (pk),
> +KEY col_int_key (c1)
> +);
> +INSERT INTO t1 VALUES (1,37);
> +INSERT INTO t1 VALUES (2,8);
> +INSERT INTO t1 VALUES (3,-25);
> +INSERT INTO t1 VALUES (4,NULL);
> +INSERT INTO t1 VALUES (5,55);
> +SELECT pk FROM t1 WHERE c1<> 1 HAVING pk = 3 ORDER BY pk LIMIT 0;
> +pk
> +SELECT pk FROM t1 WHERE c1<> 1 HAVING pk = 3 ORDER BY pk LIMIT 1;
> +pk
> +3
> +SELECT pk FROM t1 WHERE c1<> 1 HAVING pk = 3 ORDER BY pk LIMIT 2;
> +pk
> +3
> +SELECT pk FROM t1 WHERE c1<> 1 HAVING pk = 3 ORDER BY pk LIMIT 5;
> +pk
> +3
> +DROP TABLE t1;
> set optimizer_switch=default;
>
> === modified file 'mysql-test/r/myisam_icp_none.result'
> --- a/mysql-test/r/myisam_icp_none.result 2010-12-13 15:22:45 +0000
> +++ b/mysql-test/r/myisam_icp_none.result 2011-01-03 14:50:49 +0000
> @@ -595,4 +595,31 @@
> select 1 from t1 where b<= 1 and a<> '';
> 1
> drop table t1;
> +#
> +# Bug#58838 "Wrong results with HAVING + LIMIT without GROUP BY when
> +# ICP is enabled"
> +#
> +CREATE TABLE t1 (
> +pk INT NOT NULL,
> +c1 INT,
> +PRIMARY KEY (pk),
> +KEY col_int_key (c1)
> +);
> +INSERT INTO t1 VALUES (1,37);
> +INSERT INTO t1 VALUES (2,8);
> +INSERT INTO t1 VALUES (3,-25);
> +INSERT INTO t1 VALUES (4,NULL);
> +INSERT INTO t1 VALUES (5,55);
> +SELECT pk FROM t1 WHERE c1<> 1 HAVING pk = 3 ORDER BY pk LIMIT 0;
> +pk
> +SELECT pk FROM t1 WHERE c1<> 1 HAVING pk = 3 ORDER BY pk LIMIT 1;
> +pk
> +3
> +SELECT pk FROM t1 WHERE c1<> 1 HAVING pk = 3 ORDER BY pk LIMIT 2;
> +pk
> +3
> +SELECT pk FROM t1 WHERE c1<> 1 HAVING pk = 3 ORDER BY pk LIMIT 5;
> +pk
> +3
> +DROP TABLE t1;
> set optimizer_switch=default;
>
> === modified file 'sql/sql_select.cc'
> --- a/sql/sql_select.cc 2010-12-29 00:38:59 +0000
> +++ b/sql/sql_select.cc 2011-01-03 14:50:49 +0000
> @@ -3219,6 +3219,29 @@
> DBUG_EXECUTE("where",print_where(curr_table->select->cond,
> "select and having",
> QT_ORDINARY););
> +
> + /*
> + If we have pushed parts of the condition down to the handler
> + then we need to add this to the original pre-ICP select
> + condition since the original select condition may be used in
> + test_if_skip_sort_order().
> + */
> + if (curr_table->pre_idx_push_select_cond)
> + {
> + sort_table_cond= make_cond_for_table(curr_join->tmp_having,
> + used_tables, used_tables, 0);
> + if (!sort_table_cond)
> + DBUG_VOID_RETURN;
> + Item* new_pre_idx_push_select_cond=
> + new Item_cond_and(curr_table->pre_idx_push_select_cond,
> + sort_table_cond);
> + if (!new_pre_idx_push_select_cond)
> + DBUG_VOID_RETURN;
> + if (new_pre_idx_push_select_cond->fix_fields(thd, 0))
> + DBUG_VOID_RETURN;
> + curr_table->pre_idx_push_select_cond= new_pre_idx_push_select_cond;
> + }
> +
> curr_join->tmp_having= make_cond_for_table(curr_join->tmp_having,
> ~ (table_map) 0,
> ~used_tables, 0);
>
>
>
>
