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()


BitKeeper/etc/ignore:
  added support-files/ndb-config-2-node.ini
mysql-test/r/group_by.result:
  More complicated test to assure that rand() is only calulated once
mysql-test/r/user_var.result:
  Back to old results :(  (ok but not perfect)
mysql-test/t/group_by.test:
  More complicated test to assure that rand() is only calulated once
sql/item.cc:
  Better bugfix for "HAVING when refering to RAND()"
  This will ensure that when refering to things like RAND() in HAVING through an alias we will not recalculate that rand() value in the HAVING part but use the value in the row
  Generalize split_sum_func()
sql/item.h:
  Better bugfix for "HAVING when refering to RAND()"
  T
sql/item_cmpfunc.cc:
  Better bugfix for "HAVING when refering to RAND()"
  Use generalized split_sum_func2() function
sql/item_func.cc:
  Better bugfix for "HAVING when refering to RAND()"
  Use generalized split_sum_func2() function
sql/item_row.cc:
  Better bugfix for "HAVING when refering to RAND()"
  Use generalized split_sum_func2() function
sql/item_strfunc.cc:
  Better bugfix for "HAVING when refering to RAND()"
  Use generalized split_sum_func2() function
sql/sql_list.h:
  Add functions to concatenate lists
sql/sql_select.cc:
  Better bugfix for "HAVING when refering to RAND()"
  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.

8 files changed, 89 insertions, 95 deletions

diff --git a/sql/item.cc b/sql/item.cc
index d61d628e8fa..17d41fda677 100644
--- a/sql/item.cc
+++ b/sql/item.cc
@@ -296,6 +296,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):
 
diff --git a/sql/item.h b/sql/item.h
index e0de7452eec..a8b892292d3 100644
--- a/sql/item.h
+++ b/sql/item.h
@@ -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)
diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc
index 2b9a612da18..a827f316ec1 100644
--- a/sql/item_cmpfunc.cc
+++ b/sql/item_cmpfunc.cc
@@ -1969,10 +1969,10 @@ bool Item_cond::walk(Item_processor processor, byte *arg)
   Move SUM items out from item tree and replace with reference
 
   SYNOPSIS
-  split_sum_func()
-  thd			Thread handler
-  ref_pointer_array	Pointer to array of reference fields
-  fields		All fields in select
+    split_sum_func()
+    thd			Thread handler
+    ref_pointer_array	Pointer to array of reference fields
+    fields		All fields in select
 
   NOTES
    This function is run on all expression (SELECT list, WHERE, HAVING etc)
@@ -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());
 }
 
 
diff --git a/sql/item_func.cc b/sql/item_func.cc
index d5d0ac8cd49..34c8732a9f2 100644
--- a/sql/item_func.cc
+++ b/sql/item_func.cc
@@ -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);
 }
 
 
diff --git a/sql/item_row.cc b/sql/item_row.cc
index 0ace0fc0451..12d202a1699 100644
--- a/sql/item_row.cc
+++ b/sql/item_row.cc
@@ -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;
diff --git a/sql/item_strfunc.cc b/sql/item_strfunc.cc
index b22b65eddd0..a92c4e94c87 100644
--- a/sql/item_strfunc.cc
+++ b/sql/item_strfunc.cc
@@ -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);
 }
 
diff --git a/sql/sql_list.h b/sql/sql_list.h
index 38c5af2a4f7..a607b31d60c 100644
--- a/sql/sql_list.h
+++ b/sql/sql_list.h
@@ -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;
diff --git a/sql/sql_select.cc b/sql/sql_select.cc
index 8353ca9333d..7bb60fe8e79 100644
--- a/sql/sql_select.cc
+++ b/sql/sql_select.cc
@@ -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: