From 665045f985787d77318a17ccddd67ec3ff92f7bc Mon Sep 17 00:00:00 2001 From: Igor Babaev <igor@askmonty.org> Date: Mon, 21 Nov 2016 10:33:06 -0800 Subject: [PATCH] Fixed bug mdev-11081. The idea of this fix was taken from the patch by Roy Lyseng for mysql-5.6 bug iBug#14740889: "Wrong result for aggregate functions when executing query through cursor". Here's Roy's comment for his patch: " The problem was that a grouped query did not behave properly when executed using a cursor. On further inspection, the query used one intermediate temporary table for the grouping. Then, Select_materialize::send_result_set_metadata created a temporary table for storing the query result. Notice that get_unit_column_types() is used to retrieve column meta-data for the query. The items contained in this list are later modified so that their result_field points to the row buffer of the materialized temporary table for the cursor. But prior to this, these result_field objects have been prepared for use in the grouping operation, by JOIN::make_tmp_tables_info(), hence the grouping operation operates on wrong column buffers. The problem is solved by using the list JOIN::fields when copying data to the materialized table. This list is set by JOIN::make_tmp_tables_info() and points to the columns of the last intermediate temporary table of the executed query. For a UNION, it points to the temporary table that is the result of the UNION query. Notice that we have to assign a value to ::fields early in JOIN::optimize() in case the optimization shortcuts due to a const plan detection. A more optimal solution might be to avoid creating the final temporary table when the query result is already stored in a temporary table. " The patch does not contain a test case, but the description of the problem corresponds exactly what could be observed in the test case for mdev-11081. --- mysql-test/r/sp.result | 39 +++++++++++++++++++++++++++++++++++++++ mysql-test/t/sp.test | 38 ++++++++++++++++++++++++++++++++++++++ sql/item_subselect.cc | 2 +- sql/sql_class.h | 2 +- sql/sql_cursor.cc | 2 +- sql/sql_lex.h | 2 +- sql/sql_select.cc | 9 ++++++--- sql/sql_union.cc | 8 +++++--- 8 files changed, 92 insertions(+), 10 deletions(-) diff --git a/mysql-test/r/sp.result b/mysql-test/r/sp.result index 130e789d3eb..aca9458f124 100644 --- a/mysql-test/r/sp.result +++ b/mysql-test/r/sp.result @@ -8001,3 +8001,42 @@ return 1; end | ERROR 0A000: Not allowed to return a result set from a function drop table t1,t2; +# +# MDEV-11081: CURSOR for query with GROUP BY +# +CREATE TABLE t1 (name VARCHAR(10), value INT); +INSERT INTO t1 VALUES ('b',1); +INSERT INTO t1 VALUES ('b',1); +INSERT INTO t1 VALUES ('c',1); +INSERT INTO t1 VALUES ('a',1); +INSERT INTO t1 VALUES ('a',1); +INSERT INTO t1 VALUES ('a',1); +CREATE PROCEDURE p1 () +BEGIN +DECLARE done INT DEFAULT FALSE; +DECLARE v_name VARCHAR(10); +DECLARE v_total INT; +DECLARE c CURSOR FOR +SELECT name, SUM(value) AS total FROM t1 GROUP BY name; +DECLARE CONTINUE HANDLER FOR NOT FOUND SET done = TRUE; +OPEN c; +read_loop: +LOOP +FETCH c INTO v_name, v_total; +IF done THEN +LEAVE read_loop; +END IF; +SELECT v_name, v_total; +END LOOP; +CLOSE c; +END; +| +CALL p1(); +v_name v_total +a 3 +v_name v_total +b 2 +v_name v_total +c 1 +DROP PROCEDURE p1; +DROP TABLE t1; diff --git a/mysql-test/t/sp.test b/mysql-test/t/sp.test index c80e1eaaa3e..aaab59bcb89 100644 --- a/mysql-test/t/sp.test +++ b/mysql-test/t/sp.test @@ -9446,3 +9446,41 @@ end | --delimiter ; drop table t1,t2; + +--echo # +--echo # MDEV-11081: CURSOR for query with GROUP BY +--echo # + +CREATE TABLE t1 (name VARCHAR(10), value INT); +INSERT INTO t1 VALUES ('b',1); +INSERT INTO t1 VALUES ('b',1); +INSERT INTO t1 VALUES ('c',1); +INSERT INTO t1 VALUES ('a',1); +INSERT INTO t1 VALUES ('a',1); +INSERT INTO t1 VALUES ('a',1); +DELIMITER |; +CREATE PROCEDURE p1 () +BEGIN + DECLARE done INT DEFAULT FALSE; + DECLARE v_name VARCHAR(10); + DECLARE v_total INT; + DECLARE c CURSOR FOR + SELECT name, SUM(value) AS total FROM t1 GROUP BY name; + DECLARE CONTINUE HANDLER FOR NOT FOUND SET done = TRUE; + OPEN c; +read_loop: + LOOP + FETCH c INTO v_name, v_total; + IF done THEN + LEAVE read_loop; + END IF; + SELECT v_name, v_total; + END LOOP; + CLOSE c; +END; +| +DELIMITER ;| +CALL p1(); +DROP PROCEDURE p1; +DROP TABLE t1; + diff --git a/sql/item_subselect.cc b/sql/item_subselect.cc index 21c633333f1..e90efbd82be 100644 --- a/sql/item_subselect.cc +++ b/sql/item_subselect.cc @@ -4572,7 +4572,7 @@ subselect_hash_sj_engine::get_strategy_using_schema() return COMPLETE_MATCH; else { - List_iterator<Item> inner_col_it(*item_in->unit->get_unit_column_types()); + List_iterator<Item> inner_col_it(*item_in->unit->get_column_types(false)); Item *outer_col, *inner_col; for (uint i= 0; i < item_in->left_expr->cols(); i++) diff --git a/sql/sql_class.h b/sql/sql_class.h index eeb6c917054..0434f5485cf 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -4392,7 +4392,7 @@ protected: /* All descendant classes have their send_data() skip the first unit->offset_limit_cnt rows sent. Select_materialize - also uses unit->get_unit_column_types(). + also uses unit->get_column_types(). */ SELECT_LEX_UNIT *unit; /* Something used only by the parser: */ diff --git a/sql/sql_cursor.cc b/sql/sql_cursor.cc index 7ecce8a8da3..118c6f23cb0 100644 --- a/sql/sql_cursor.cc +++ b/sql/sql_cursor.cc @@ -433,7 +433,7 @@ void Materialized_cursor::on_table_fill_finished() bool Select_materialize::send_result_set_metadata(List<Item> &list, uint flags) { DBUG_ASSERT(table == 0); - if (create_result_table(unit->thd, unit->get_unit_column_types(), + if (create_result_table(unit->thd, unit->get_column_types(true), FALSE, thd->variables.option_bits | TMP_TABLE_ALL_COLUMNS, "", FALSE, TRUE, TRUE)) diff --git a/sql/sql_lex.h b/sql/sql_lex.h index b044ef42b97..4a554ae7fe2 100644 --- a/sql/sql_lex.h +++ b/sql/sql_lex.h @@ -732,7 +732,7 @@ public: friend void lex_start(THD *thd); friend int subselect_union_engine::exec(); - List<Item> *get_unit_column_types(); + List<Item> *get_column_types(bool for_cursor); select_union *get_union_result() { return union_result; } int save_union_explain(Explain_query *output); diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 1114e1e1f1d..168e0af5d38 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -1121,9 +1121,6 @@ int JOIN::optimize() int JOIN::optimize_inner() { -/* - if (conds) { Item *it_clone= conds->build_clone(thd,thd->mem_root); } -*/ ulonglong select_opts_for_readinfo; uint no_jbuf_after; JOIN_TAB *tab; @@ -1137,6 +1134,12 @@ JOIN::optimize_inner() set_allowed_join_cache_types(); need_distinct= TRUE; + /* + Needed in case optimizer short-cuts, + set properly in make_tmp_tables_info() + */ + fields= &select_lex->item_list; + if (select_lex->first_cond_optimization) { //Do it only for the first execution diff --git a/sql/sql_union.cc b/sql/sql_union.cc index 0d05d201f12..aad3701cca2 100644 --- a/sql/sql_union.cc +++ b/sql/sql_union.cc @@ -1382,7 +1382,9 @@ bool st_select_lex_unit::change_result(select_result_interceptor *new_result, Get column type information for this unit. SYNOPSIS - st_select_lex_unit::get_unit_column_types() + st_select_lex_unit::get_column_types() + @param for_cursor if true return the list the fields + retrieved by the cursor DESCRIPTION For a single-select the column types are taken @@ -1396,7 +1398,7 @@ bool st_select_lex_unit::change_result(select_result_interceptor *new_result, st_select_lex_unit::prepare() */ -List<Item> *st_select_lex_unit::get_unit_column_types() +List<Item> *st_select_lex_unit::get_column_types(bool for_cursor) { SELECT_LEX *sl= first_select(); bool is_procedure= MY_TEST(sl->join->procedure); @@ -1416,7 +1418,7 @@ List<Item> *st_select_lex_unit::get_unit_column_types() return &types; } - return &sl->item_list; + return for_cursor ? sl->join->fields : &sl->item_list; }