Commit 79ec8107 authored by monty@mysql.com's avatar monty@mysql.com

Better bugfix for "HAVING when refering to RAND()" (Bug #8216)

Ensure that references in HAVING, ORDER BY or GROUP BY are calculated after fields in SELECT.
This will ensure that any reference to these has a valid value.
Generalized the code for split_sum_func()
parent 6c253007
......@@ -1007,3 +1007,4 @@ tests/mysql_client_test
libmysqld/examples/mysql_client_test.c
libmysqld/examples/mysql_client_test_embedded
libmysqld/examples/mysqltest_embedded
support-files/ndb-config-2-node.ini
......@@ -655,9 +655,9 @@ insert into t1 (a,b) values (1,2),(1,3),(2,5);
select a, 0.1*0+1 r2, sum(1) r1 from t1 where a = 1 group by a having r1>1 and r2=1;
a r2 r1
1 1.0 2
select a, rand()*0+1 r2, sum(1) r1 from t1 where a = 1 group by a having r1>1 and r2=1;
select a, round(rand(100)*10) r2, sum(1) r1 from t1 where a = 1 group by a having r1>1 and r2<=2;
a r2 r1
1 1 2
1 2 2
select a,sum(b) from t1 where a=1 group by c;
a sum(b)
1 5
......
......@@ -109,8 +109,8 @@ select @a:=0;
select @a, @a:=@a+count(*), count(*), @a from t1 group by i;
@a @a:=@a+count(*) count(*) @a
0 1 1 0
0 3 2 0
0 6 3 0
0 2 2 0
0 3 3 0
select @a:=0;
@a:=0
0
......
......@@ -480,7 +480,8 @@ drop table t1;
create table t1 (a integer, b integer, c integer);
insert into t1 (a,b) values (1,2),(1,3),(2,5);
select a, 0.1*0+1 r2, sum(1) r1 from t1 where a = 1 group by a having r1>1 and r2=1;
select a, rand()*0+1 r2, sum(1) r1 from t1 where a = 1 group by a having r1>1 and r2=1;
# rand(100)*10 will be < 2 only for the first row (of 6)
select a, round(rand(100)*10) r2, sum(1) r1 from t1 where a = 1 group by a having r1>1 and r2<=2;
select a,sum(b) from t1 where a=1 group by c;
select a*sum(b) from t1 where a=1 group by c;
select sum(a)*sum(b) from t1 where a=1 group by c;
......
......@@ -295,6 +295,58 @@ CHARSET_INFO *Item::default_charset()
}
/*
Move SUM items out from item tree and replace with reference
SYNOPSIS
split_sum_func2()
thd Thread handler
ref_pointer_array Pointer to array of reference fields
fields All fields in select
ref Pointer to item
NOTES
This is from split_sum_func2() for items that should be split
All found SUM items are added FIRST in the fields list and
we replace the item with a reference.
thd->fatal_error() may be called if we are out of memory
*/
void Item::split_sum_func2(THD *thd, Item **ref_pointer_array,
List<Item> &fields, Item **ref)
{
if (type() != SUM_FUNC_ITEM && with_sum_func)
{
/* Will split complicated items and ignore simple ones */
split_sum_func(thd, ref_pointer_array, fields);
}
else if ((type() == SUM_FUNC_ITEM ||
(used_tables() & ~PARAM_TABLE_BIT)) &&
type() != REF_ITEM)
{
/*
Replace item with a reference so that we can easily calculate
it (in case of sum functions) or copy it (in case of fields)
The test above is to ensure we don't do a reference for things
that are constants (PARAM_TABLE_BIT is in effect a constant)
or already referenced (for example an item in HAVING)
*/
uint el= fields.elements;
Item *new_item;
ref_pointer_array[el]= this;
if (!(new_item= new Item_ref(ref_pointer_array + el, 0, name)))
return; // fatal_error is set
fields.push_front(this);
ref_pointer_array[el]= this;
thd->change_item_tree(ref, new_item);
}
}
/*
Aggregate two collations together taking
into account their coercibility (aka derivation):
......
......@@ -262,6 +262,9 @@ public:
virtual void update_used_tables() {}
virtual void split_sum_func(THD *thd, Item **ref_pointer_array,
List<Item> &fields) {}
/* Called for items that really have to be split */
void split_sum_func2(THD *thd, Item **ref_pointer_array, List<Item> &fields,
Item **ref);
virtual bool get_date(TIME *ltime,uint fuzzydate);
virtual bool get_time(TIME *ltime);
virtual bool get_date_result(TIME *ltime,uint fuzzydate)
......
......@@ -1982,16 +1982,6 @@ bool Item_cond::walk(Item_processor processor, byte *arg)
so that we can easily find and calculate them.
(Calculation done by update_sum_func() and copy_sum_funcs() in
sql_select.cc)
All found SUM items are added FIRST in the fields list and
we replace the item with a reference.
We also replace all functions without side effects (like RAND() or UDF's)
that uses columns as arguments.
For functions with side effects, we just remember any fields referred
by the function to ensure that we get a copy of the field value for the
first accepted row. This ensures that we can do things like
SELECT a*SUM(b) FROM t1 WHERE a=1
*/
void Item_cond::split_sum_func(THD *thd, Item **ref_pointer_array,
......@@ -1999,38 +1989,8 @@ void Item_cond::split_sum_func(THD *thd, Item **ref_pointer_array,
{
List_iterator<Item> li(list);
Item *item;
used_tables_cache=0;
const_item_cache=0;
while ((item=li++))
{
/* with_sum_func is set for items that contains a SUM expression */
if (item->type() != SUM_FUNC_ITEM &&
(item->with_sum_func ||
(item->used_tables() & PSEUDO_TABLE_BITS)))
item->split_sum_func(thd, ref_pointer_array, fields);
else if (item->type() == SUM_FUNC_ITEM ||
(item->used_tables() && item->type() != REF_ITEM))
{
/*
Replace item with a reference so that we can easily calculate
it (in case of sum functions) or copy it (in case of fields)
The test above is to ensure we don't do a reference for things
that are constants or are not yet calculated as in:
SELECT RAND() as r1, SUM(a) as r2 FROM t1 HAVING r1 > 1 AND r2 > 0
*/
Item **ref= li.ref();
uint el= fields.elements;
ref_pointer_array[el]= item;
Item *new_item= new Item_ref(ref_pointer_array + el, 0, item->name);
fields.push_front(item);
ref_pointer_array[el]= item;
thd->change_item_tree(ref, new_item);
}
item->update_used_tables();
used_tables_cache|=item->used_tables();
const_item_cache&=item->const_item();
}
while ((item= li++))
item->split_sum_func2(thd, ref_pointer_array, fields, li.ref());
}
......
......@@ -352,28 +352,14 @@ bool Item_func::walk (Item_processor processor, byte *argument)
}
/* See comments in Item_cmp_func::split_sum_func() */
void Item_func::split_sum_func(THD *thd, Item **ref_pointer_array,
List<Item> &fields)
{
Item **arg, **arg_end;
for (arg= args, arg_end= args+arg_count; arg != arg_end ; arg++)
{
Item *item=* arg;
if (item->type() != SUM_FUNC_ITEM &&
(item->with_sum_func ||
(item->used_tables() & PSEUDO_TABLE_BITS)))
item->split_sum_func(thd, ref_pointer_array, fields);
else if (item->type() == SUM_FUNC_ITEM ||
(item->used_tables() && item->type() != REF_ITEM))
{
uint el= fields.elements;
ref_pointer_array[el]= item;
Item *new_item= new Item_ref(ref_pointer_array + el, 0, item->name);
fields.push_front(item);
ref_pointer_array[el]= item;
thd->change_item_tree(arg, new_item);
}
}
(*arg)->split_sum_func2(thd, ref_pointer_array, fields, arg);
}
......
......@@ -90,25 +90,10 @@ void Item_row::split_sum_func(THD *thd, Item **ref_pointer_array,
{
Item **arg, **arg_end;
for (arg= items, arg_end= items+arg_count; arg != arg_end ; arg++)
{
Item *item= *arg;
if (item->type() != SUM_FUNC_ITEM &&
(item->with_sum_func ||
(item->used_tables() & PSEUDO_TABLE_BITS)))
item->split_sum_func(thd, ref_pointer_array, fields);
else if (item->type() == SUM_FUNC_ITEM ||
(item->used_tables() && item->type() != REF_ITEM))
{
uint el= fields.elements;
ref_pointer_array[el]=*arg;
Item *new_item= new Item_ref(ref_pointer_array + el, 0, (*arg)->name);
fields.push_front(*arg);
ref_pointer_array[el]= *arg;
thd->change_item_tree(arg, new_item);
}
}
(*arg)->split_sum_func2(thd, ref_pointer_array, fields, arg);
}
void Item_row::update_used_tables()
{
used_tables_cache= 0;
......
......@@ -1758,20 +1758,7 @@ String *Item_func_elt::val_str(String *str)
void Item_func_make_set::split_sum_func(THD *thd, Item **ref_pointer_array,
List<Item> &fields)
{
if (item->type() != SUM_FUNC_ITEM &&
(item->with_sum_func ||
(item->used_tables() & PSEUDO_TABLE_BITS)))
item->split_sum_func(thd, ref_pointer_array, fields);
else if (item->type() == SUM_FUNC_ITEM ||
(item->used_tables() && item->type() != REF_ITEM))
{
uint el= fields.elements;
ref_pointer_array[el]=item;
Item *new_item= new Item_ref(ref_pointer_array + el, 0, item->name);
fields.push_front(item);
ref_pointer_array[el]= item;
thd->change_item_tree(&item, new_item);
}
item->split_sum_func2(thd, ref_pointer_array, fields, &item);
Item_str_func::split_sum_func(thd, ref_pointer_array, fields);
}
......
......@@ -134,6 +134,15 @@ public:
if (!--elements)
last= &first;
}
inline void concat(base_list *list)
{
if (!list->is_empty())
{
*last= list->first;
last= list->last;
elements+= list->elements;
}
}
inline void *pop(void)
{
if (first == &end_of_list) return 0;
......
......@@ -8502,6 +8502,7 @@ setup_copy_fields(THD *thd, TMP_TABLE_PARAM *param,
res_selected_fields.empty();
res_all_fields.empty();
List_iterator_fast<Item> itr(res_all_fields);
List<Item> extra_funcs;
uint i, border= all_fields.elements - elements;
DBUG_ENTER("setup_copy_fields");
......@@ -8563,7 +8564,12 @@ setup_copy_fields(THD *thd, TMP_TABLE_PARAM *param,
*/
if (!(pos=new Item_copy_string(pos)))
goto err;
if (param->copy_funcs.push_back(pos))
if (i < border) // HAVING, ORDER and GROUP BY
{
if (extra_funcs.push_back(pos))
goto err;
}
else if (param->copy_funcs.push_back(pos))
goto err;
}
res_all_fields.push_back(pos);
......@@ -8575,6 +8581,12 @@ setup_copy_fields(THD *thd, TMP_TABLE_PARAM *param,
for (i= 0; i < border; i++)
itr++;
itr.sublist(res_selected_fields, elements);
/*
Put elements from HAVING, ORDER BY and GROUP BY last to ensure that any
reference used in these will resolve to a item that is already calculated
*/
param->copy_funcs.concat(&extra_funcs);
DBUG_RETURN(0);
err:
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment