Merge of fix for BUG#6976 continued: pulling in some Item_ref changes from 5.0

 * Added Item_ref::set_properties 
 * Item_ref::Item_ref now expects to get in *item either
     NULL - then fix_fields() will be called later  or 
     ptr to Item it will refer to - then an equivalent of fix_fields() call is performed


sql/item.cc:
  Merge of fix for BUG#6976 continued: pulling in some Item_ref changes from 5.0
   * Added Item_ref::set_properties 
   * Adjusted Item_ref::Item_ref calls to match new calling convention
sql/item.h:
  Merge of fix for BUG#6976 continued: pulling in some Item_ref changes from 5.0
   * Added Item_ref::set_properties 
   * Item_ref::Item_ref now expects to get in *item either
       NULL - then fix_fields() will be called later or
       ptr to Item it will refer to - then an equivalent of fix_fields() call is performed
sql/item_cmpfunc.cc:
  Merge of fix for BUG#6976 continued: pulling in some Item_ref changes from 5.0
   * Adjusted Item_ref::Item_ref calls to match new calling convention
sql/item_func.cc:
  Merge of fix for BUG#6976 continued: pulling in some Item_ref changes from 5.0
   * Added Item_ref::set_properties 
   * Adjusted Item_ref::Item_ref calls to match new calling convention
sql/item_row.cc:
  Merge of fix for BUG#6976 continued: pulling in some Item_ref changes from 5.0
   * Added Item_ref::set_properties 
   * Adjusted Item_ref::Item_ref calls to match new calling convention
sql/item_strfunc.cc:
  Merge of fix for BUG#6976 continued: pulling in some Item_ref changes from 5.0
   * Added Item_ref::set_properties 
   * Adjusted Item_ref::Item_ref calls to match new calling convention

6 files changed, 50 insertions, 32 deletions

diff --git a/sql/item.cc b/sql/item.cc
index ec637eb8bce..aa67219ab53 100644
--- a/sql/item.cc
+++ b/sql/item.cc
@@ -1488,16 +1488,24 @@ bool Item_field::fix_fields(THD *thd, TABLE_LIST *tables, Item **ref)
 		   "forward reference in item list");
 	  return -1;
 	}
+        /*
+          Here, a subset of actions performed by Item_ref::set_properties
+          is not enough. So we pass ptr to NULL into Item_[direct]_ref ctor,
+          so no initialization is performed, and call fix_fields() below.
+        */
+        Item *save= last->ref_pointer_array[counter];
+        last->ref_pointer_array[counter]= NULL;
 	Item_ref *rf= (place == IN_HAVING ?
                        new Item_ref(last->ref_pointer_array + counter,
                                     (char *)table_name,
-                                    (char *)field_name, this) :
+                                    (char *)field_name) :
                        new Item_direct_ref(last->ref_pointer_array + counter,
                                            (char *)table_name,
-                                           (char *)field_name, this));
+                                           (char *)field_name));
 	if (!rf)
 	  return 1;
         thd->change_item_tree(ref, rf);
+        last->ref_pointer_array[counter]= save;
 	/*
 	  rf is Item_ref => never substitute other items (in this case)
 	  during fix_fields() => we can use rf after fix_fields()
@@ -2220,19 +2228,24 @@ bool Item_ref::fix_fields(THD *thd,TABLE_LIST *tables, Item **reference)
 	      "forward reference in item list"));
     return 1;
   }
+
+  set_properties();
+
+  if (ref && (*ref)->check_cols(1))
+    return 1;
+  return 0;
+}
+
+void Item_ref::set_properties()
+{
   max_length= (*ref)->max_length;
   maybe_null= (*ref)->maybe_null;
   decimals=   (*ref)->decimals;
   collation.set((*ref)->collation);
   with_sum_func= (*ref)->with_sum_func;
   fixed= 1;
-
-  if (ref && (*ref)->check_cols(1))
-    return 1;
-  return 0;
 }
 
-
 void Item_ref::print(String *str)
 {
   if (ref && *ref)
diff --git a/sql/item.h b/sql/item.h
index 52a644677a6..2411904a00f 100644
--- a/sql/item.h
+++ b/sql/item.h
@@ -827,34 +827,35 @@ public:
 
 class Item_ref :public Item_ident
 {
+protected:
+  void set_properties();
 public:
   Field *result_field;			 /* Save result here */
   Item **ref;
   Item_ref(const char *db_par, const char *table_name_par,
            const char *field_name_par)
     :Item_ident(db_par, table_name_par, field_name_par), ref(0) {}
-  Item_ref(Item **item, const char *table_name_par, const char *field_name_par)
-    :Item_ident(NullS, table_name_par, field_name_par), ref(item) {}
-  
   /*
-    This constructor is used when processing GROUP BY and referred Item is 
-    available. We set all properties here because fix_fields() will not be 
-    called for the created Item_ref. (see BUG#6976) 
-    TODO check if we could get rid of *_name_par parameters and if we need to 
-         perform similar initialization for other ctors.
+    This constructor is used in two scenarios:
+    A) *item = NULL
+      No initialization is performed, fix_fields() call will be necessary.
+      
+    B) *item points to an Item this Item_ref will refer to. This is 
+      used for GROUP BY. fix_fields() will not be called in this case,
+      so we call set_properties to make this item "fixed". set_properties
+      performs a subset of action Item_ref::fix_fields does, and this subset
+      is enough for Item_ref's used in GROUP BY.
+    
     TODO we probably fix a superset of problems like in BUG#6658. Check this 
          with Bar, and if we have a more broader set of problems like this.
   */
-  Item_ref(Item **item, const char *table_name_par, 
-           const char *field_name_par, Item *src)
-    : Item_ident(NullS, table_name_par, field_name_par), ref(item)
+  Item_ref(Item **item, const char *table_name_par, const char *field_name_par)
+    :Item_ident(NullS, table_name_par, field_name_par), ref(item)
   {
-    collation.set(src->collation);
-    max_length= src->max_length;
-    decimals=	src->decimals;
-    with_sum_func= src->with_sum_func;
-    maybe_null= src->maybe_null;
+    if (*item)
+      set_properties();
   }
+
   /* Constructor need to process subselect with temporary tables (see Item) */
   Item_ref(THD *thd, Item_ref *item) :Item_ident(thd, item), ref(item->ref) {}
   enum Type type() const		{ return REF_ITEM; }
@@ -862,29 +863,34 @@ public:
   { return ref && (*ref)->eq(item, binary_cmp); }
   double val()
   {
+    DBUG_ASSERT(fixed);
     double tmp=(*ref)->val_result();
     null_value=(*ref)->null_value;
     return tmp;
   }
   longlong val_int()
   {
+    DBUG_ASSERT(fixed);
     longlong tmp=(*ref)->val_int_result();
     null_value=(*ref)->null_value;
     return tmp;
   }
   String *val_str(String* tmp)
   {
+    DBUG_ASSERT(fixed);
     tmp=(*ref)->str_result(tmp);
     null_value=(*ref)->null_value;
     return tmp;
   }
   bool is_null()
   {
+    DBUG_ASSERT(fixed);
     (void) (*ref)->val_int_result();
     return (*ref)->null_value;
   }
   bool get_date(TIME *ltime,uint fuzzydate)
   {
+    DBUG_ASSERT(fixed);
     return (null_value=(*ref)->get_date_result(ltime,fuzzydate));
   }
   bool send(Protocol *prot, String *tmp){ return (*ref)->send(prot, tmp); }
@@ -922,9 +928,6 @@ public:
     :Item_ref(item, table_name_par, field_name_par) {}
   /* Constructor need to process subselect with temporary tables (see Item) */
   Item_direct_ref(THD *thd, Item_direct_ref *item) : Item_ref(thd, item) {}
-  Item_direct_ref(Item **item, const char *table_name_par, 
-           const char *field_name_par, Item *src)
-    : Item_ref(item, table_name_par, field_name_par, src) {}
 
   double val()
   {
diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc
index 0b824178afd..26ed8f4e9c1 100644
--- a/sql/item_cmpfunc.cc
+++ b/sql/item_cmpfunc.cc
@@ -2022,8 +2022,8 @@ void Item_cond::split_sum_func(THD *thd, Item **ref_pointer_array,
     {
       Item **ref= li.ref();
       uint el= fields.elements;
-      Item *new_item= new Item_ref(ref_pointer_array + el, 0, item->name,
-                                   item);
+      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);
diff --git a/sql/item_func.cc b/sql/item_func.cc
index af53a771720..8220db40ecb 100644
--- a/sql/item_func.cc
+++ b/sql/item_func.cc
@@ -349,7 +349,8 @@ void Item_func::split_sum_func(THD *thd, Item **ref_pointer_array,
     else if (item->used_tables() || item->type() == SUM_FUNC_ITEM)
     {
       uint el= fields.elements;
-      Item *new_item= new Item_ref(ref_pointer_array + el, 0, item->name, item);
+      ref_pointer_array[el]= item;
+      Item *new_item= new Item_ref(ref_pointer_array + el, 0, item->name);
       new_item->collation.set(item->collation);
       fields.push_front(item);
       ref_pointer_array[el]= item;
diff --git a/sql/item_row.cc b/sql/item_row.cc
index b65b0b7b608..4e4957b980e 100644
--- a/sql/item_row.cc
+++ b/sql/item_row.cc
@@ -95,8 +95,8 @@ void Item_row::split_sum_func(THD *thd, Item **ref_pointer_array,
     else if ((*arg)->used_tables() || (*arg)->type() == SUM_FUNC_ITEM)
     {
       uint el= fields.elements;
-      Item *new_item= new Item_ref(ref_pointer_array + el, 0, (*arg)->name,
-                                   *arg);
+      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);
diff --git a/sql/item_strfunc.cc b/sql/item_strfunc.cc
index 82cbd1fed72..e1b063cd5e0 100644
--- a/sql/item_strfunc.cc
+++ b/sql/item_strfunc.cc
@@ -1748,7 +1748,8 @@ void Item_func_make_set::split_sum_func(THD *thd, Item **ref_pointer_array,
   else if (item->used_tables() || item->type() == SUM_FUNC_ITEM)
   {
     uint el= fields.elements;
-    Item *new_item= new Item_ref(ref_pointer_array + el, 0, item->name,item);
+    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);