mirror of
https://github.com/MariaDB/server.git
synced 2025-01-31 02:51:44 +01:00
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.
This commit is contained in:
parent
edabd315d6
commit
c5ec57b593
4 changed files with 100 additions and 28 deletions
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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;
|
||||
}
|
||||
|
||||
|
||||
/**
|
||||
|
|
|
@ -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);
|
||||
|
|
Loading…
Add table
Reference in a new issue