diff options
author | MySQL Build Team <build@mysql.com> | 2009-11-25 17:45:33 +0100 |
---|---|---|
committer | MySQL Build Team <build@mysql.com> | 2009-11-25 17:45:33 +0100 |
commit | 4b93aa6a8244a43dddcd53538bba45e9a5b94ee4 (patch) | |
tree | bb720b85e5594cdbd1376f1ab3dc98ec03dd444c | |
parent | c14bf1ca586a6e77ee7614a3044d86cc2b8e0132 (diff) | |
download | mariadb-git-4b93aa6a8244a43dddcd53538bba45e9a5b94ee4.tar.gz |
Backport into build-200911241145-5.1.40sp1
> ------------------------------------------------------------
> revno: 1810.3964.1
> revision-id: alexey.kopytov@sun.com-20091030155453-0vlfwki805h9os62
> parent: joerg@mysql.com-20091016122941-rf6z0keqvmlgjfto
> committer: Alexey Kopytov <Alexey.Kopytov@Sun.com>
> branch nick: my50-bug48131
> timestamp: Fri 2009-10-30 18:54:53 +0300
> message:
> Bug #48131: crash group by with rollup, distinct, filesort,
> with temporary tables
>
> There were two problems the test case from this bug was
> triggering:
>
> 1. JOIN::rollup_init() was supposed to wrap all constant Items
> into another object for queries with the WITH ROLLUP modifier
> to ensure they are never considered as constants and therefore
> are written into temporary tables if the optimizer chooses to
> employ them for DISTINCT/GROUP BY handling.
>
> However, JOIN::rollup_init() was called before
> make_join_statistics(), so Items corresponding to fields in
> const tables could not be handled as intended, which was
> causing all kinds of problems later in the query execution. In
> particular, create_tmp_table() assumed all constant items
> except "hidden" ones to be removed earlier by remove_const()
> which led to improperly initialized Field objects for the
> temporary table being created. This is what was causing crashes
> and valgrind errors in storage engines.
>
> 2. Even when the above problem had been fixed, the query from
> the test case produced incorrect results due to some
> DISTINCT/GROUP BY optimizations being performed by the
> optimizer that are inapplicable in the WITH ROLLUP case.
>
> Fixed by disabling inapplicable DISTINCT/GROUP BY optimizations
> when the WITH ROLLUP modifier is present, and splitting the
> const-wrapping part of JOIN::rollup_init() into a separate
> method which is now invoked after make_join_statistics() when
> the const tables are already known.
-rw-r--r-- | mysql-test/r/olap.result | 20 | ||||
-rw-r--r-- | mysql-test/t/olap.test | 15 | ||||
-rw-r--r-- | sql/sql_select.cc | 92 | ||||
-rw-r--r-- | sql/sql_select.h | 1 |
4 files changed, 100 insertions, 28 deletions
diff --git a/mysql-test/r/olap.result b/mysql-test/r/olap.result index 4540c9d5218..a7516d97888 100644 --- a/mysql-test/r/olap.result +++ b/mysql-test/r/olap.result @@ -733,4 +733,24 @@ SELECT 1 FROM t1 GROUP BY (DATE(NULL)) WITH ROLLUP; 1 1 DROP TABLE t1; +# +# Bug #48131: crash group by with rollup, distinct, +# filesort, with temporary tables +# +CREATE TABLE t1 (a INT NOT NULL PRIMARY KEY); +INSERT INTO t1 VALUES (1), (2); +CREATE TABLE t2 (b INT); +INSERT INTO t2 VALUES (100); +SELECT a, b FROM t1, t2 GROUP BY a, b WITH ROLLUP; +a b +1 100 +1 NULL +2 100 +2 NULL +NULL NULL +SELECT DISTINCT b FROM t1, t2 GROUP BY a, b WITH ROLLUP; +b +100 +NULL +DROP TABLE t1, t2; End of 5.0 tests diff --git a/mysql-test/t/olap.test b/mysql-test/t/olap.test index d1e40024733..8f672af40a3 100644 --- a/mysql-test/t/olap.test +++ b/mysql-test/t/olap.test @@ -375,4 +375,19 @@ INSERT INTO t1 VALUES(0); SELECT 1 FROM t1 GROUP BY (DATE(NULL)) WITH ROLLUP; DROP TABLE t1; +--echo # +--echo # Bug #48131: crash group by with rollup, distinct, +--echo # filesort, with temporary tables +--echo # + +CREATE TABLE t1 (a INT NOT NULL PRIMARY KEY); +INSERT INTO t1 VALUES (1), (2); +CREATE TABLE t2 (b INT); +INSERT INTO t2 VALUES (100); + +SELECT a, b FROM t1, t2 GROUP BY a, b WITH ROLLUP; +SELECT DISTINCT b FROM t1, t2 GROUP BY a, b WITH ROLLUP; + +DROP TABLE t1, t2; + --echo End of 5.0 tests diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 4b466203800..94f1206f26c 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -970,6 +970,12 @@ JOIN::optimize() DBUG_RETURN(1); } + if (select_lex->olap == ROLLUP_TYPE && rollup_process_const_fields()) + { + DBUG_PRINT("error", ("Error: rollup_process_fields() failed")); + DBUG_RETURN(1); + } + /* Remove distinct if only const tables */ select_distinct= select_distinct && (const_tables != tables); thd_proc_info(thd, "preparing"); @@ -1100,7 +1106,7 @@ JOIN::optimize() join_tab[const_tables].select->quick->get_type() != QUICK_SELECT_I::QS_TYPE_GROUP_MIN_MAX)) { - if (group_list && + if (group_list && rollup.state == ROLLUP::STATE_NONE && list_contains_unique_index(join_tab[const_tables].table, find_field_in_order_list, (void *) group_list)) @@ -1144,7 +1150,8 @@ JOIN::optimize() if (! hidden_group_fields && rollup.state == ROLLUP::STATE_NONE) select_distinct=0; } - else if (select_distinct && tables - const_tables == 1) + else if (select_distinct && tables - const_tables == 1 && + rollup.state == ROLLUP::STATE_NONE) { /* We are only using one table. In this case we change DISTINCT to a @@ -10203,6 +10210,7 @@ create_tmp_table(THD *thd,TMP_TABLE_PARAM *param,List<Item> &fields, for (; cur_group ; cur_group= cur_group->next, key_part_info++) { Field *field=(*cur_group->item)->get_tmp_table_field(); + DBUG_ASSERT(field->table == table); bool maybe_null=(*cur_group->item)->maybe_null; key_part_info->null_bit=0; key_part_info->field= field; @@ -15622,32 +15630,7 @@ bool JOIN::rollup_init() { item->maybe_null= 1; found_in_group= 1; - if (item->const_item()) - { - /* - For ROLLUP queries each constant item referenced in GROUP BY list - is wrapped up into an Item_func object yielding the same value - as the constant item. The objects of the wrapper class are never - considered as constant items and besides they inherit all - properties of the Item_result_field class. - This wrapping allows us to ensure writing constant items - into temporary tables whenever the result of the ROLLUP - operation has to be written into a temporary table, e.g. when - ROLLUP is used together with DISTINCT in the SELECT list. - Usually when creating temporary tables for a intermidiate - result we do not include fields for constant expressions. - */ - Item* new_item= new Item_func_rollup_const(item); - if (!new_item) - return 1; - new_item->fix_fields(thd, (Item **) 0); - thd->change_item_tree(it.ref(), new_item); - for (ORDER *tmp= group_tmp; tmp; tmp= tmp->next) - { - if (*tmp->item == item) - thd->change_item_tree(tmp->item, new_item); - } - } + break; } } if (item->type() == Item::FUNC_ITEM && !found_in_group) @@ -15666,6 +15649,59 @@ bool JOIN::rollup_init() } return 0; } + +/** + Wrap all constant Items in GROUP BY list. + + For ROLLUP queries each constant item referenced in GROUP BY list + is wrapped up into an Item_func object yielding the same value + as the constant item. The objects of the wrapper class are never + considered as constant items and besides they inherit all + properties of the Item_result_field class. + This wrapping allows us to ensure writing constant items + into temporary tables whenever the result of the ROLLUP + operation has to be written into a temporary table, e.g. when + ROLLUP is used together with DISTINCT in the SELECT list. + Usually when creating temporary tables for a intermidiate + result we do not include fields for constant expressions. + + @retval + 0 if ok + @retval + 1 on error +*/ + +bool JOIN::rollup_process_const_fields() +{ + ORDER *group_tmp; + Item *item; + List_iterator<Item> it(all_fields); + + for (group_tmp= group_list; group_tmp; group_tmp= group_tmp->next) + { + if (!(*group_tmp->item)->const_item()) + continue; + while ((item= it++)) + { + if (*group_tmp->item == item) + { + Item* new_item= new Item_func_rollup_const(item); + if (!new_item) + return 1; + new_item->fix_fields(thd, (Item **) 0); + thd->change_item_tree(it.ref(), new_item); + for (ORDER *tmp= group_tmp; tmp; tmp= tmp->next) + { + if (*tmp->item == item) + thd->change_item_tree(tmp->item, new_item); + } + break; + } + } + it.rewind(); + } + return 0; +} /** diff --git a/sql/sql_select.h b/sql/sql_select.h index a0366d47149..4c729fa915c 100644 --- a/sql/sql_select.h +++ b/sql/sql_select.h @@ -502,6 +502,7 @@ public: } bool rollup_init(); + bool rollup_process_const_fields(); bool rollup_make_fields(List<Item> &all_fields, List<Item> &fields, Item_sum ***func); int rollup_send_data(uint idx); |