WL#1972 "Evaluate HAVING before SELECT select-list"

- Changed name resolution for GROUP BY so that derived columns do not shadow table columns
  from the FROM clause. As a result GROUP BY now is handled as a true ANSI extentsion.
- Issue a warning when HAVING is resolved into ambiguous columns, and prefer the columns from
  the GROUP BY clause over SELECT columns.


mysql-test/r/having.result:
  Correct result for updated GROUP BY name resolution.
sql/item.cc:
  - prefer GROUP columns, but if none is found use SELECT list
  - issue a waring when a field may be resolved ambiguously
  - more/fixed comments
sql/mysql_priv.h:
  More flexible find_field_in_tables().
sql/sp.cc:
  More flexible find_field_in_tables().
sql/sql_base.cc:
  More flexible find_field_in_tables().
sql/sql_help.cc:
  More flexible find_field_in_tables().
sql/sql_select.cc:
  - name resolution of GROUP/ORDER BY column references is differentiated:
    - GROUP BY is resolved in SELECT and FROM clauses
    - ORDER BY is resolved only in SELECT (as before)
  - more informative variable names
  - more comments

7 files changed, 171 insertions, 75 deletions

diff --git a/mysql-test/r/having.result b/mysql-test/r/having.result
index f312cc6659f..04f73792dd6 100644
--- a/mysql-test/r/having.result
+++ b/mysql-test/r/having.result
@@ -221,6 +221,9 @@ select count(*) from wl1972 group by s1 having s1 is null;
 count(*)
 select s1*0 as s1 from wl1972 group by s1 having s1 <> 0;
 s1
+0
+0
+0
 select s1*0 from wl1972 group by s1 having s1 = 0;
 s1*0
 select s1 from wl1972 group by 1 having 1 = 0;
diff --git a/sql/item.cc b/sql/item.cc
index b500d84ec4a..738814bc353 100644
--- a/sql/item.cc
+++ b/sql/item.cc
@@ -1397,7 +1397,8 @@ bool Item_field::fix_fields(THD *thd, TABLE_LIST *tables, Item **ref)
   {
     bool upward_lookup= 0;
     Field *tmp= (Field *)not_found_field;
-    if ((tmp= find_field_in_tables(thd, this, tables, ref, 0,
+    if ((tmp= find_field_in_tables(thd, this, tables, ref,
+                                   IGNORE_EXCEPT_NON_UNIQUE,
                                    !any_privileges)) ==
 	not_found_field)
     {
@@ -1449,7 +1450,8 @@ bool Item_field::fix_fields(THD *thd, TABLE_LIST *tables, Item **ref)
                (sl->with_sum_func == 0 && sl->group_list.elements == 0)) &&
               (tmp= find_field_in_tables(thd, this,
 					 table_list, ref,
-					 0, 1)) != not_found_field)
+					 IGNORE_EXCEPT_NON_UNIQUE, 1)) !=
+              not_found_field)
 	  {
 	    if (tmp)
             {
@@ -1505,7 +1507,7 @@ bool Item_field::fix_fields(THD *thd, TABLE_LIST *tables, Item **ref)
 	else
 	{
 	  // Call to report error
-	  find_field_in_tables(thd, this, tables, ref, 1, 1);
+	  find_field_in_tables(thd, this, tables, ref, REPORT_ALL_ERRORS, 1);
 	}
 	return -1;
       }
@@ -2347,31 +2349,39 @@ static Item** find_field_in_group_list(Item *find_item, ORDER *group_list)
 
   SYNOPSIS
     Item_ref::fix_fields()
-    thd       [in]      Current thread
-    tables    [in]      The tables in the FROM clause
-    reference [in/out]  View column if this item was resolved to a view column
+    thd       [in]      current thread
+    tables    [in]      the tables in the FROM clause
+    reference [in/out]  view column if this item was resolved to a view column
 
   DESCRIPTION
-    The method resolves the column reference represented by this as an Item
-    present in either of: GROUP BY clause, SELECT clause, outer queries. It is
+    The method resolves the column reference represented by 'this' as a column
+    present in one of: GROUP BY clause, SELECT clause, outer queries. It is
     used for columns in the HAVING clause which are not under aggregate
     functions.
 
   NOTES
+    The general idea behind the name resolution algorithm is that it searches
+    both the SELECT and GROUP BY clauses, and in case of a name conflict
+    prefers GROUP BY column names over SELECT names. We extend ANSI SQL in that
+    when no GROUP BY column is found, then a HAVING name is resolved as a
+    possibly derived SELECT column.
+
     The name resolution algorithm used is:
+
       resolve_extended([T_j].col_ref_i)
       {
-        Search for a column named col_ref_i [in table T_j]
-        in the GROUP BY clause of Q.
-
         Search for a column or derived column named col_ref_i [in table T_j]
         in the SELECT list of Q.
 
-        if found different columns with the same name in GROUP BY and SELECT
-        issue an error.
+        Search for a column named col_ref_i [in table T_j]
+        in the GROUP BY clause of Q.
+
+        If found different columns with the same name in GROUP BY and SELECT
+        issue a warning and return the GROUP BY column,
+        otherwise return the found SELECT column.
 
-        // Lookup in outer queries.
-        if such a column is NOT found AND there are outer queries
+        if such a column is NOT found AND    // Lookup in outer queries.
+           there are outer queries
         {
           for each outer query Q_k beginning from the inner-most one
          {
@@ -2385,8 +2395,9 @@ static Item** find_field_in_group_list(Item *find_item, ORDER *group_list)
           }
         }
       }
-    This procedure treats GROUP BY and SELECT as one namespace for column
-    references in HAVING.
+
+    This procedure treats GROUP BY and SELECT clauses as one namespace for
+    column references in HAVING.
 
   RETURN
     TRUE  if error
@@ -2398,9 +2409,10 @@ bool Item_ref::fix_fields(THD *thd, TABLE_LIST *tables, Item **reference)
   DBUG_ASSERT(fixed == 0);
   uint counter;
   SELECT_LEX *current_sel= thd->lex->current_select;
-  List<Item> *search_namespace= current_sel->get_item_list();
+  List<Item> *select_fields= current_sel->get_item_list();
   bool is_having_field= current_sel->having_fix_field;
   Item **group_by_ref= NULL;
+  bool ambiguous_fields= FALSE;
 
   if (!ref)
   {
@@ -2413,7 +2425,7 @@ bool Item_ref::fix_fields(THD *thd, TABLE_LIST *tables, Item **reference)
       Search for a column or derived column named as 'this' in the SELECT
       clause of current_select.
     */
-    if (!(ref= find_item_in_list(this, *search_namespace, &counter,
+    if (!(ref= find_item_in_list(this, *select_fields, &counter,
                                  REPORT_EXCEPT_NOT_FOUND)))
       return TRUE; /* Some error occurred. */
 
@@ -2422,20 +2434,22 @@ bool Item_ref::fix_fields(THD *thd, TABLE_LIST *tables, Item **reference)
     {
       group_by_ref= find_field_in_group_list(this, (ORDER*)
                                              current_sel->group_list.first);
+
       /* Check if the fields found in SELECT and GROUP BY are the same field. */
       if (group_by_ref && ref != (Item **) not_found_item &&
           !((*group_by_ref)->eq(*ref, 0)))
       {
-        my_printf_error(ER_NON_UNIQ_ERROR, ER(ER_NON_UNIQ_ERROR),
-                        MYF(0), this->full_name(), current_thd->where);
-        return TRUE;
+        ambiguous_fields= TRUE;
+        push_warning_printf(thd, MYSQL_ERROR::WARN_LEVEL_WARN,
+                            ER_NON_UNIQ_ERROR, ER(ER_NON_UNIQ_ERROR),
+                            this->full_name(), current_thd->where);
+
       }
     }
 
-
     /*
       If we didn't find such a column in the current query, and if there is an
-      outer select, and this is not a derived table (which do not support the
+      outer select, and it is not a derived table (which do not support the
       use of outer fields for now), search the outer select(s) for a column
       named as 'this'.
     */
@@ -2494,7 +2508,8 @@ bool Item_ref::fix_fields(THD *thd, TABLE_LIST *tables, Item **reference)
               outer_sel->group_list.elements == 0)) &&
             (tmp= find_field_in_tables(thd, this,
 				       table_list, reference,
-				       0, 1)) != not_found_field)
+				       IGNORE_EXCEPT_NON_UNIQUE, TRUE)) !=
+            not_found_field)
 	{
           if (tmp)
           {
@@ -2538,7 +2553,7 @@ bool Item_ref::fix_fields(THD *thd, TABLE_LIST *tables, Item **reference)
 	else
 	{
 	  // Call to report error
-	  find_item_in_list(this, *search_namespace, &counter,
+	  find_item_in_list(this, *select_fields, &counter,
 			    REPORT_ALL_ERRORS);
 	}
         ref= 0;
@@ -2588,7 +2603,7 @@ bool Item_ref::fix_fields(THD *thd, TABLE_LIST *tables, Item **reference)
     }
     else
     {
-      if (ref != (Item **) not_found_item)
+      if (ref != (Item **) not_found_item && !ambiguous_fields)
         ref= current_sel->ref_pointer_array + counter;
       else if (group_by_ref)
         ref= group_by_ref;
diff --git a/sql/mysql_priv.h b/sql/mysql_priv.h
index d8916149b77..8c27e0130a1 100644
--- a/sql/mysql_priv.h
+++ b/sql/mysql_priv.h
@@ -650,8 +650,13 @@ void execute_init_command(THD *thd, sys_var_str *init_command_var,
 			  rw_lock_t *var_mutex);
 extern const Field *not_found_field;
 extern const Field *view_ref_found;
+
+enum find_item_error_report_type {REPORT_ALL_ERRORS, REPORT_EXCEPT_NOT_FOUND,
+				  IGNORE_ERRORS, REPORT_EXCEPT_NON_UNIQUE,
+                                  IGNORE_EXCEPT_NON_UNIQUE};
 Field *find_field_in_tables(THD *thd, Item_ident *item, TABLE_LIST *tables,
-			    Item **ref, bool report_error,
+			    Item **ref,
+                            find_item_error_report_type report_error,
                             bool check_privileges);
 Field *
 find_field_in_table(THD *thd, TABLE_LIST *table_list,
@@ -765,8 +770,6 @@ TABLE *unlink_open_table(THD *thd,TABLE *list,TABLE *find);
 SQL_SELECT *make_select(TABLE *head, table_map const_tables,
 			table_map read_tables, COND *conds, int *error,
                         bool allow_null_cond= false);
-enum find_item_error_report_type {REPORT_ALL_ERRORS, REPORT_EXCEPT_NOT_FOUND,
-				  IGNORE_ERRORS};
 extern const Item **not_found_item;
 Item ** find_item_in_list(Item *item, List<Item> &items, uint *counter,
 			  find_item_error_report_type report_error);
diff --git a/sql/sp.cc b/sql/sp.cc
index e444a412760..323c1acd525 100644
--- a/sql/sp.cc
+++ b/sql/sp.cc
@@ -615,7 +615,7 @@ db_show_routine_status(THD *thd, int type, const char *wild)
       Item_field *field= new Item_field("mysql", "proc",
 					used_field->field_name);
       if (!(used_field->field= find_field_in_tables(thd, field, &tables, 
-						    0, TRUE, 1)))
+						    0, REPORT_ALL_ERRORS, 1)))
       {
 	res= SP_INTERNAL_ERROR;
 	goto err_case1;
diff --git a/sql/sql_base.cc b/sql/sql_base.cc
index 4f273fbd0c4..a8a44205b64 100644
--- a/sql/sql_base.cc
+++ b/sql/sql_base.cc
@@ -2173,25 +2173,33 @@ Field *find_field_in_real_table(THD *thd, TABLE *table,
     find_field_in_tables()
     thd			Pointer to current thread structure
     item		Field item that should be found
-    tables		Tables for scanning
-    ref			if view field is found, pointer to view item will
-			be returned via this parameter
-    report_error	If FALSE then do not report error if item not found
-			and return not_found_field
+    tables		Tables to be searched for item
+    ref			If 'item' is resolved to a view field, ref is set to
+                        point to the found view field
+    report_error	Degree of error reporting:
+                        - IGNORE_ERRORS then do not report any error
+                        - IGNORE_EXCEPT_NON_UNIQUE report only non-unique
+                          fields, suppress all other errors
+                        - REPORT_EXCEPT_NON_UNIQUE report all other errors
+                          except when non-unique fields were found
+                        - REPORT_ALL_ERRORS
     check_privileges    need to check privileges
 
   RETURN VALUES
-    0			Field is not found or field is not unique- error
-			message is reported
-    not_found_field	Function was called with report_error == FALSE and
-			field was not found. no error message reported.
-    view_ref_found	view field is found, item passed through ref parameter
-    found field
+    0			No field was found, or the found field is not unique, or
+                        there are no sufficient access priviligaes for the
+                        found field, or the field is qualified with non-existing
+                        table.
+    not_found_field	The function was called with report_error ==
+                        (IGNORE_ERRORS || IGNORE_EXCEPT_NON_UNIQUE) and a
+			field was not found.
+    view_ref_found	View field is found, item passed through ref parameter
+    found field         If a item was resolved to some field
 */
 
 Field *
 find_field_in_tables(THD *thd, Item_ident *item, TABLE_LIST *tables,
-		     Item **ref, bool report_error,
+		     Item **ref, find_item_error_report_type report_error,
                      bool check_privileges)
 {
   Field *found=0;
@@ -2268,8 +2276,10 @@ find_field_in_tables(THD *thd, Item_ident *item, TABLE_LIST *tables,
 	    return find;
 	  if (found)
 	  {
-	    my_printf_error(ER_NON_UNIQ_ERROR,ER(ER_NON_UNIQ_ERROR),MYF(0),
-			    item->full_name(),thd->where);
+            if (report_error == REPORT_ALL_ERRORS ||
+                report_error == IGNORE_EXCEPT_NON_UNIQUE)
+              my_printf_error(ER_NON_UNIQ_ERROR,ER(ER_NON_UNIQ_ERROR),MYF(0),
+                              item->full_name(),thd->where);
 	    return (Field*) 0;
 	  }
 	  found=find;
@@ -2278,7 +2288,8 @@ find_field_in_tables(THD *thd, Item_ident *item, TABLE_LIST *tables,
     }
     if (found)
       return found;
-    if (!found_table && report_error)
+    if (!found_table && (report_error == REPORT_ALL_ERRORS ||
+                         report_error == REPORT_EXCEPT_NON_UNIQUE))
     {
       char buff[NAME_LEN*2+1];
       if (db && db[0])
@@ -2286,28 +2297,30 @@ find_field_in_tables(THD *thd, Item_ident *item, TABLE_LIST *tables,
 	strxnmov(buff,sizeof(buff)-1,db,".",table_name,NullS);
 	table_name=buff;
       }
-      if (report_error)
-      {
+      if (report_error == REPORT_ALL_ERRORS ||
+          report_error == REPORT_EXCEPT_NON_UNIQUE)
 	my_printf_error(ER_UNKNOWN_TABLE, ER(ER_UNKNOWN_TABLE), MYF(0),
 			table_name, thd->where);
-      }
       else
 	return (Field*) not_found_field;
     }
     else
-      if (report_error)
+      if (report_error == REPORT_ALL_ERRORS ||
+          report_error == REPORT_EXCEPT_NON_UNIQUE)
 	my_printf_error(ER_BAD_FIELD_ERROR,ER(ER_BAD_FIELD_ERROR),MYF(0),
 			item->full_name(),thd->where);
       else
 	return (Field*) not_found_field;
     return (Field*) 0;
   }
+
   bool allow_rowid= tables && !tables->next_local;	// Only one table
   for (; tables ; tables= tables->next_local)
   {
     if (!tables->table)
     {
-      if (report_error)
+      if (report_error == REPORT_ALL_ERRORS ||
+          report_error == REPORT_EXCEPT_NON_UNIQUE)
 	my_printf_error(ER_BAD_FIELD_ERROR,ER(ER_BAD_FIELD_ERROR),MYF(0),
 			item->full_name(),thd->where);
       return (Field*) not_found_field;
@@ -2332,8 +2345,10 @@ find_field_in_tables(THD *thd, Item_ident *item, TABLE_LIST *tables,
       {
 	if (!thd->where)			// Returns first found
 	  break;
-	my_printf_error(ER_NON_UNIQ_ERROR,ER(ER_NON_UNIQ_ERROR),MYF(0),
-			name,thd->where);
+        if (report_error == REPORT_ALL_ERRORS ||
+            report_error == IGNORE_EXCEPT_NON_UNIQUE)
+          my_printf_error(ER_NON_UNIQ_ERROR,ER(ER_NON_UNIQ_ERROR),MYF(0),
+                          name,thd->where);
 	return (Field*) 0;
       }
       found=field;
@@ -2341,7 +2356,8 @@ find_field_in_tables(THD *thd, Item_ident *item, TABLE_LIST *tables,
   }
   if (found)
     return found;
-  if (report_error)
+  if (report_error == REPORT_ALL_ERRORS ||
+      report_error == REPORT_EXCEPT_NON_UNIQUE)
     my_printf_error(ER_BAD_FIELD_ERROR, ER(ER_BAD_FIELD_ERROR),
 		    MYF(0), item->full_name(), thd->where);
   else
@@ -2377,7 +2393,7 @@ find_field_in_tables(THD *thd, Item_ident *item, TABLE_LIST *tables,
     found field 
 */
 
-// Special Item pointer for find_item_in_list returning
+/* Special Item pointer to serve as a return value from find_item_in_list(). */
 const Item **not_found_item= (const Item**) 0x1;
 
 
diff --git a/sql/sql_help.cc b/sql/sql_help.cc
index cba74c93a6a..62ee708b695 100644
--- a/sql/sql_help.cc
+++ b/sql/sql_help.cc
@@ -88,7 +88,7 @@ static bool init_fields(THD *thd, TABLE_LIST *tables,
     Item_field *field= new Item_field("mysql", find_fields->table_name,
                                       find_fields->field_name);
     if (!(find_fields->field= find_field_in_tables(thd, field, tables,
-						   0, TRUE, 1)))
+						   0, REPORT_ALL_ERRORS, 1)))
       DBUG_RETURN(1);
   }
   DBUG_RETURN(0);
diff --git a/sql/sql_select.cc b/sql/sql_select.cc
index 9ba191a3f3a..99d076bd335 100644
--- a/sql/sql_select.cc
+++ b/sql/sql_select.cc
@@ -11115,24 +11115,53 @@ cp_buffer_from_ref(TABLE_REF *ref)
 *****************************************************************************/
 
 /*
-  Find order/group item in requested columns and change the item to point at
-  it. If item doesn't exists, add it first in the field list
-  Return 0 if ok.
+  Resolve an ORDER BY or GROUP BY column reference.
+
+  SYNOPSIS
+    find_order_in_list()
+    thd		      [in]     Pointer to current thread structure
+    ref_pointer_array [in/out] All select, group and order by fields
+    tables            [in]     List of tables to search in (usually FROM clause)
+    order             [in]     Column reference to be resolved
+    fields            [in]     List of fields to search in (usually SELECT list)
+    all_fields        [in/out] All select, group and order by fields
+    is_group_field    [in]     True if order is a GROUP field, false if
+                               ORDER by field
+
+  DESCRIPTION
+    Given a column reference (represented by 'order') from a GROUP BY or ORDER
+    BY clause, find the actual column it represents. If the column being
+    resolved is from the GROUP BY clause, the procedure searches the SELECT
+    list 'fields' and the columns in the FROM list 'tables'. If 'order' is from
+    the ORDER BY clause, only the SELECT list is being searched.
+
+    If 'order' is resolved to an Item, then order->item is set to the found
+    Item. If there is no item for the found column (that is, it was resolved
+    into a table field), order->item is 'fixed' and is added to all_fields and
+    ref_pointer_array.
+
+  RETURN
+    0 if ok
+    1 if error occurred
 */
 
 static int
-find_order_in_list(THD *thd, Item **ref_pointer_array,
-		   TABLE_LIST *tables,ORDER *order, List<Item> &fields,
-		   List<Item> &all_fields)
+find_order_in_list(THD *thd, Item **ref_pointer_array, TABLE_LIST *tables,
+                   ORDER *order, List<Item> &fields, List<Item> &all_fields,
+                   bool is_group_field)
 {
-  Item *itemptr=*order->item;
-  if (itemptr->type() == Item::INT_ITEM)
+  Item *order_item=*order->item; /* The item from the GROUP/ORDER caluse. */
+  Item::Type order_item_type;
+  Item **select_item; /* The corresponding item from the SELECT clause. */
+  Field *from_field;  /* The corresponding field from the FROM clause. */
+
+  if (order_item->type() == Item::INT_ITEM)
   {						/* Order by position */
-    uint count= (uint) itemptr->val_int();
+    uint count= (uint) order_item->val_int();
     if (!count || count > fields.elements)
     {
       my_printf_error(ER_BAD_FIELD_ERROR,ER(ER_BAD_FIELD_ERROR),
-		      MYF(0),itemptr->full_name(),
+		      MYF(0),order_item->full_name(),
 	       thd->where);
       return 1;
     }
@@ -11142,17 +11171,47 @@ find_order_in_list(THD *thd, Item **ref_pointer_array,
     order->counter_used= 1;
     return 0;
   }
+  /* Lookup the current GROUP/ORDER field in the SELECT clause. */
   uint counter;
-  Item **item= find_item_in_list(itemptr, fields, &counter,
+  select_item= find_item_in_list(order_item, fields, &counter,
                                  REPORT_EXCEPT_NOT_FOUND);
-  if (!item)
-    return 1;
+  if (!select_item)
+    return 1; /* Some error occured. */
+
 
-  if (item != (Item **)not_found_item)
+  /* Check whether the resolved field is not ambiguos. */
+  if (select_item != not_found_item)
   {
-    order->item= ref_pointer_array + counter;
-    order->in_field_list=1;
-    return 0;
+    /* Lookup the current GROUP field in the FROM clause. */
+    order_item_type= order_item->type();
+    if (is_group_field &&
+        order_item_type == Item::FIELD_ITEM || order_item_type == Item::REF_ITEM)
+    {
+      Item **view_ref= NULL;
+      from_field= find_field_in_tables(thd, (Item_ident*) order_item, tables,
+                                       view_ref, IGNORE_ERRORS, TRUE);
+      if(!from_field)
+       from_field= (Field*) not_found_field;
+    }
+    else
+      from_field= (Field*) not_found_field;
+
+    if (from_field == not_found_field ||
+        from_field && from_field != view_ref_found &&
+        (*select_item)->type() == Item::FIELD_ITEM &&
+        ((Item_field*) (*select_item))->field->eq(from_field))
+      /*
+        If there is no such field in the FROM clause, or it is the same field as
+        the one found in the SELECT clause, then use the Item created for the
+        SELECT field. As a result if there was a derived field that 'shadowed'
+        a table field with the same name, the table field will be chosen over
+        the derived field.
+      */
+    {
+      order->item= ref_pointer_array + counter;
+      order->in_field_list=1;
+      return 0;
+    }
   }
 
   order->in_field_list=0;
@@ -11187,7 +11246,7 @@ int setup_order(THD *thd, Item **ref_pointer_array, TABLE_LIST *tables,
   for (; order; order=order->next)
   {
     if (find_order_in_list(thd, ref_pointer_array, tables, order, fields,
-			   all_fields))
+			   all_fields, FALSE))
       return 1;
   }
   return 0;
@@ -11239,7 +11298,7 @@ setup_group(THD *thd, Item **ref_pointer_array, TABLE_LIST *tables,
   for (; order; order=order->next)
   {
     if (find_order_in_list(thd, ref_pointer_array, tables, order, fields,
-			   all_fields))
+			   all_fields, TRUE))
       return 1;
     (*order->item)->marker=1;		/* Mark found */
     if ((*order->item)->with_sum_func)