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

- post-review fix regarding Item_fields
- added test for the changed name resolution

mysql-test/r/having.result

@@ -245,6 +245,13 @@ col1
 10
 10
 10
+select sum(col1) from t1
+group by col_t1
+having (select col_t1 from t2 where col_t1 = col_t2 order by col_t2 limit 1);
+sum(col1)
+40
+20
+30
 select t1.col1 from t1
 where t1.col2 in 
 (select t2.col2 from t2 

mysql-test/t/having.test

@@ -211,6 +211,11 @@ where t1.col2 in
       (select t2.col2 from t2 
        group by t2.col1, t2.col2 having col_t1 <= 10);
 
+# Item_field must be resolved in the same way as Item_ref
+select sum(col1) from t1
+group by col_t1
+having (select col_t1 from t2 where col_t1 = col_t2 order by col_t2 limit 1);
+
 # nested queries with HAVING, inner having column resolved in outer FROM clause
 # the outer having column is not referenced in GROUP BY which results in an error
 --error 1054
@@ -256,7 +261,6 @@ group by col_t1
 having col_t1 in (select sum(t2.col1) from t2
                   group by t2.col2, t2.col1 having t2.col1 = col_t1);
 
-
 #
 # queries with joins and ambiguous column names
 #

sql/item.cc

@@ -1390,113 +1390,312 @@ static void mark_as_dependent(THD *thd, SELECT_LEX *last, SELECT_LEX *current,
 }
 
 
-bool Item_field::fix_fields(THD *thd, TABLE_LIST *tables, Item **ref)
+/*
+  Search a GROUP BY clause for a field with a certain name.
+
+  SYNOPSIS
+    find_field_in_group_list()
+    find_item  the item being searched for
+    group_list GROUP BY clause
+
+  DESCRIPTION
+    Search the GROUP BY list for a column named as find_item. When searching
+    preference is given to columns that are qualified with the same table (and
+    database) name as the one being searched for.
+
+  RETURN
+    - the found item on success
+    - NULL if find_item is not in group_list
+*/
+
+static Item** find_field_in_group_list(Item *find_item, ORDER *group_list)
+{
+  const char *db_name;
+  const char *table_name;
+  const char *field_name;
+  ORDER      *found_group= NULL;
+  int         found_match_degree= 0;
+  Item_field *cur_field;
+  int         cur_match_degree= 0;
+
+  if (find_item->type() == Item::FIELD_ITEM ||
+      find_item->type() == Item::REF_ITEM)
+  {
+    db_name=    ((Item_ident*) find_item)->db_name;
+    table_name= ((Item_ident*) find_item)->table_name;
+    field_name= ((Item_ident*) find_item)->field_name;
+  }
+  else
+    return NULL;
+
+  DBUG_ASSERT(field_name);
+
+  for (ORDER *cur_group= group_list ; cur_group ; cur_group= cur_group->next)
+  {
+    if ((*(cur_group->item))->type() == Item::FIELD_ITEM)
+    {
+      cur_field= (Item_field*) *cur_group->item;
+      cur_match_degree= 0;
+      
+      DBUG_ASSERT(cur_field->field_name);
+
+      if (!my_strcasecmp(system_charset_info,
+                         cur_field->field_name, field_name))
+        ++cur_match_degree;
+      else
+        continue;
+
+      if (cur_field->table_name && table_name)
+      {
+        /* If field_name is qualified by a table name. */
+        if (strcmp(cur_field->table_name, table_name))
+          /* Same field names, different tables. */
+          return NULL;
+
+        ++cur_match_degree;
+        if (cur_field->db_name && db_name)
+        {
+          /* If field_name is also qualified by a database name. */
+          if (strcmp(cur_field->db_name, db_name))
+            /* Same field names, different databases. */
+            return NULL;
+          ++cur_match_degree;
+        }
+      }
+
+      if (cur_match_degree > found_match_degree)
+      {
+        found_match_degree= cur_match_degree;
+        found_group= cur_group;
+      }
+      else if (found_group && (cur_match_degree == found_match_degree) &&
+               ! (*(found_group->item))->eq(cur_field, 0))
+      {
+        /*
+          If the current resolve candidate matches equally well as the current
+          best match, they must reference the same column, otherwise the field
+          is ambiguous.
+        */
+        my_printf_error(ER_NON_UNIQ_ERROR, ER(ER_NON_UNIQ_ERROR),
+                        MYF(0), find_item->full_name(), current_thd->where);
+        return NULL;
+      }
+    }
+  }
+
+  if (found_group)
+    return found_group->item;
+  else
+    return NULL;
+}
+
+
+/*
+  Resolve a column reference in a sub-select.
+
+  SYNOPSIS
+    resolve_ref_in_select_and_group()
+    thd     current thread
+    ref     column reference being resolved
+    select  the sub-select that ref is resolved against
+
+  DESCRIPTION
+    Resolve a column reference (usually inside a HAVING clause) against the
+    SELECT and GROUP BY clauses of the query described by 'select'. The name
+    resolution algorithm searches both the SELECT and GROUP BY clauses, and in
+    case of a name conflict prefers GROUP BY column names over SELECT names. If
+    both clauses contain different fields with the same names, a warning is
+    issued that name of 'ref' is ambiguous. 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.
+
+  NOTES
+    The resolution procedure is:
+    - Search for a column or derived column named col_ref_i [in table T_j]
+      in the SELECT clause of Q.
+    - 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.
+
+
+  RETURN
+    NULL - there was an error, and the error was already reported
+    not_found_item - the item was not resolved, no error was reported
+    resolved item - if the item was resolved
+*/
+
+static Item**
+resolve_ref_in_select_and_group(THD *thd, Item_ident *ref, SELECT_LEX *select)
+{
+  Item **group_by_ref= NULL;
+  Item **select_ref= NULL;
+  ORDER *group_list= (ORDER*) select->group_list.first;
+  bool ambiguous_fields= FALSE;
+  uint counter;
+
+  /*
+    Search for a column or derived column named as 'ref' in the SELECT
+    clause of the current select.
+  */
+  if (!(select_ref= find_item_in_list(ref, *(select->get_item_list()), &counter,
+                                      REPORT_EXCEPT_NOT_FOUND)))
+    return NULL; /* Some error occurred. */
+
+  /* If this is a non-aggregated field inside HAVING, search in GROUP BY. */
+  if (select->having_fix_field && !ref->with_sum_func && group_list)
+  {
+    group_by_ref= find_field_in_group_list(ref, group_list);
+    
+    /* Check if the fields found in SELECT and GROUP BY are the same field. */
+    if (group_by_ref && (select_ref != not_found_item) &&
+        !((*group_by_ref)->eq(*select_ref, 0)))
+    {
+      ambiguous_fields= TRUE;
+      push_warning_printf(thd, MYSQL_ERROR::WARN_LEVEL_WARN, ER_NON_UNIQ_ERROR,
+                          ER(ER_NON_UNIQ_ERROR), ref->full_name(),
+                          current_thd->where);
+
+    }
+  }
+
+  if (select_ref != not_found_item || group_by_ref)
+  {
+    if (select_ref != not_found_item && !ambiguous_fields)
+    {
+      if (*select_ref && !(*select_ref)->fixed)
+      {
+        my_error(ER_ILLEGAL_REFERENCE, MYF(0), ref->name,
+                 "forward reference in item list");
+        return NULL;
+      }
+      return (select->ref_pointer_array + counter);
+    }
+    else if (group_by_ref)
+      return group_by_ref;
+    else
+      DBUG_ASSERT(FALSE);
+  }
+  else
+    return (Item**) not_found_item;
+}
+
+
+bool Item_field::fix_fields(THD *thd, TABLE_LIST *tables, Item **reference)
 {
   DBUG_ASSERT(fixed == 0);
   if (!field)					// If field is not checked
   {
-    bool upward_lookup= 0;
-    Field *tmp= (Field *)not_found_field;
-    if ((tmp= find_field_in_tables(thd, this, tables, ref,
+    bool upward_lookup= FALSE;
+    Field *from_field= (Field *)not_found_field;
+    if ((from_field= find_field_in_tables(thd, this, tables, reference,
                                    IGNORE_EXCEPT_NON_UNIQUE,
                                    !any_privileges)) ==
 	not_found_field)
     {
-      /*
-	We can't find table field in table list of current select,
-	consequently we have to find it in outer subselect(s).
-	We can't join lists of outer & current select, because of scope
-	of view rules. For example if both tables (outer & current) have
-	field 'field' it is not mistake to refer to this field without
-	mention of table name, but if we join tables in one list it will
-	cause error ER_NON_UNIQ_ERROR in find_field_in_tables.
-      */
-      SELECT_LEX *last= 0;
 #ifdef EMBEDDED_LIBRARY
       thd->net.last_errno= 0;
 #endif
+      SELECT_LEX *last= 0;
       TABLE_LIST *table_list;
-      Item **refer= (Item **)not_found_item;
-      uint counter;
-      // Prevent using outer fields in subselects, that is not supported now
-      SELECT_LEX *cursel= (SELECT_LEX *) thd->lex->current_select;
-      if (cursel->master_unit()->first_select()->linkage != DERIVED_TABLE_TYPE)
+      Item **ref= (Item **) not_found_item;
+      SELECT_LEX *current_sel= (SELECT_LEX *) thd->lex->current_select;
+      /*
+        If there is an outer select, and it is not a derived table (which do
+        not support the use of outer fields for now), try to resolve this
+        reference in the outer select(s).
+      
+        We treat each subselect as a separate namespace, so that different
+        subselects may contain columns with the same names. The subselects are
+        searched starting from the innermost.
+      */
+      if (current_sel->master_unit()->first_select()->linkage !=
+          DERIVED_TABLE_TYPE)
       {
-	SELECT_LEX_UNIT *prev_unit= cursel->master_unit();
-	for (SELECT_LEX *sl= prev_unit->outer_select();
-	     sl;
-	     sl= (prev_unit= sl->master_unit())->outer_select())
+	SELECT_LEX_UNIT *prev_unit= current_sel->master_unit();
+        SELECT_LEX *outer_sel= prev_unit->outer_select();
+	for ( ; outer_sel ;
+              outer_sel= (prev_unit= outer_sel->master_unit())->outer_select())
 	{
-	  upward_lookup= 1;
-	  table_list= (last= sl)->get_table_list();
-	  if (sl->resolve_mode == SELECT_LEX::INSERT_MODE && table_list)
-	  {
+          last= outer_sel;
+	  Item_subselect *prev_subselect_item= prev_unit->item;
+	  upward_lookup= TRUE;
+
+          /* Search in the tables of the FROM clause of the outer select. */
+	  table_list= outer_sel->get_table_list();
+	  if (outer_sel->resolve_mode == SELECT_LEX::INSERT_MODE && table_list)
             /*
-              it is primary INSERT st_select_lex => skip first table
-              resolving
+              It is a primary INSERT st_select_lex => do not resolve against the
+              first table.
             */
 	    table_list= table_list->next_local;
-	  }
 
-	  Item_subselect *prev_subselect_item= prev_unit->item;
           enum_parsing_place place= prev_subselect_item->parsing_place;
           /*
-            check table fields only if subquery used somewhere out of HAVING
-            or SELECT list or outer SELECT do not use groupping (i.e. tables
-            are accessable)
+            Check table fields only if the subquery is used somewhere out of
+            HAVING or SELECT list, or the outer SELECT does not use grouping
+            (i.e. tables are accessible).
           */
-          if (((place != IN_HAVING &&
-                place != SELECT_LIST) ||
-               (sl->with_sum_func == 0 && sl->group_list.elements == 0)) &&
-              (tmp= find_field_in_tables(thd, this,
-					 table_list, ref,
-					 IGNORE_EXCEPT_NON_UNIQUE, 1)) !=
+          if (((place != IN_HAVING && place != SELECT_LIST) ||
+               (outer_sel->with_sum_func == 0 &&
+                outer_sel->group_list.elements == 0)) &&
+              (from_field= find_field_in_tables(thd, this, table_list,
+                                                reference,
+                                                IGNORE_EXCEPT_NON_UNIQUE,
+                                                TRUE)) !=
               not_found_field)
 	  {
-	    if (tmp)
+	    if (from_field)
             {
-              if (tmp != view_ref_found)
+              if (from_field != view_ref_found)
               {
-                prev_subselect_item->used_tables_cache|= tmp->table->map;
+                prev_subselect_item->used_tables_cache|= from_field->table->map;
                 prev_subselect_item->const_item_cache= 0;
               }
               else
               {
                 prev_subselect_item->used_tables_cache|=
-                  (*ref)->used_tables();
+                  (*reference)->used_tables();
                 prev_subselect_item->const_item_cache&=
-                  (*ref)->const_item();
+                  (*reference)->const_item();
               }
             }
 	    break;
 	  }
-	  if (sl->resolve_mode == SELECT_LEX::SELECT_MODE &&
-	      (refer= find_item_in_list(this, sl->item_list, &counter,
-					 REPORT_EXCEPT_NOT_FOUND)) !=
-	       (Item **) not_found_item)
+
+          /* Search in the SELECT and GROUP lists of the outer select. */
+	  if (outer_sel->resolve_mode == SELECT_LEX::SELECT_MODE)
 	  {
-	    if (*refer && (*refer)->fixed) // Avoid crash in case of error
-	    {
-	      prev_subselect_item->used_tables_cache|= (*refer)->used_tables();
-	      prev_subselect_item->const_item_cache&= (*refer)->const_item();
-	    }
-	    break;
+            if (!(ref= resolve_ref_in_select_and_group(thd, this, outer_sel)))
+              return TRUE; /* Some error occured (e.g. ambigous names). */
+            if (ref != not_found_item)
+            {
+              DBUG_ASSERT(*ref && (*ref)->fixed);
+              /*
+                Avoid crash in case of error.
+                TODO: what does this comment mean?
+              */
+              prev_subselect_item->used_tables_cache|= (*ref)->used_tables();
+	      prev_subselect_item->const_item_cache&= (*ref)->const_item();
+              break;
+            }
 	  }
 
 	  // Reference is not found => depend from outer (or just error)
 	  prev_subselect_item->used_tables_cache|= OUTER_REF_TABLE_BIT;
 	  prev_subselect_item->const_item_cache= 0;
 
-	  if (sl->master_unit()->first_select()->linkage ==
+	  if (outer_sel->master_unit()->first_select()->linkage ==
 	      DERIVED_TABLE_TYPE)
 	    break; // do not look over derived table
 	}
       }
-      if (!tmp)
-	return -1;
-      if (!refer)
-	return 1;
-      if (tmp == not_found_field && refer == (Item **)not_found_item)
+
+      DBUG_ASSERT(ref);
+      if (!from_field)
+	return TRUE;
+      if (ref == not_found_item && from_field == not_found_field)
       {
 	if (upward_lookup)
 	{
@@ -1507,59 +1706,54 @@ 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, REPORT_ALL_ERRORS, 1);
+	  find_field_in_tables(thd, this, tables, reference, REPORT_ALL_ERRORS,
+                               TRUE);
 	}
-	return -1;
+	return TRUE;
       }
-      else if (refer != (Item **)not_found_item)
+      else if (ref != not_found_item)
       {
-	if (!(*refer)->fixed)
-	{
-	  my_error(ER_ILLEGAL_REFERENCE, MYF(0), name,
-		   "forward reference in item list");
-	  return -1;
-	}
+        /* Should be checked in resolve_ref_in_select_and_group(). */
+        DBUG_ASSERT(*ref && (*ref)->fixed);
 
 	Item_ref *rf;
-	*ref= rf= new Item_ref(last->ref_pointer_array + counter,
-			       ref,
-			       (char *)table_name,
-			       (char *)field_name);
-	register_item_tree_changing(ref);
+	*reference= rf= new Item_ref(ref, reference, (char *) table_name,
+                                     (char *) field_name);
+	register_item_tree_changing(reference);
 	if (!rf)
-	  return 1;
+	  return TRUE;
 	/*
 	  rf is Item_ref => never substitute other items (in this case)
 	  during fix_fields() => we can use rf after fix_fields()
 	*/
-	if (rf->fix_fields(thd, tables, ref) || rf->check_cols(1))
-	  return 1;
+	if (rf->fix_fields(thd, tables, reference) || rf->check_cols(1))
+	  return TRUE;
 
-	mark_as_dependent(thd, last, cursel, rf);
-	return 0;
+	mark_as_dependent(thd, last, current_sel, rf);
+	return FALSE;
       }
       else
       {
-	mark_as_dependent(thd, last, cursel, this);
+	mark_as_dependent(thd, last, current_sel, this);
 	if (last->having_fix_field)
 	{
 	  Item_ref *rf;
-	  *ref= rf= new Item_ref(ref, *ref,
-				 (cached_table->db[0]?cached_table->db:0),
-				 (char *)cached_table->alias,
-				 (char *)field_name);
+	  *reference= rf= new Item_ref(reference, *reference,
+                                       (cached_table->db[0]?cached_table->db:0),
+                                       (char *)cached_table->alias,
+                                       (char *)field_name);
 	  if (!rf)
-	    return 1;
+	    return TRUE;
 	  /*
 	    rf is Item_ref => never substitute other items (in this case)
 	    during fix_fields() => we can use rf after fix_fields()
 	  */
-	  return rf->fix_fields(thd, tables, ref) ||  rf->check_cols(1);
+	  return rf->fix_fields(thd, tables, reference) ||  rf->check_cols(1);
 	}
       }
     }
-    else if (!tmp)
-      return -1;
+    else if (!from_field)
+      return TRUE;
 
     /*
       if it is not expression from merged VIEW we will set this field.
@@ -1573,8 +1767,8 @@ bool Item_field::fix_fields(THD *thd, TABLE_LIST *tables, Item **ref)
 
       Also we suppose that view can't be changed during PS/SP life.
     */
-    if (tmp != view_ref_found)
-      set_field(tmp);
+    if (from_field != view_ref_found)
+      set_field(from_field);
   }
   else if (thd->set_query_id && field->query_id != thd->query_id)
   {
@@ -1610,12 +1804,12 @@ bool Item_field::fix_fields(THD *thd, TABLE_LIST *tables, Item **ref)
                       thd->host_or_ip,
                       field_name,
                       tab);
-      return 1;
+      return TRUE;
     }
   }
 #endif
   fixed= 1;
-  return 0;
+  return FALSE;
 }
 
 void Item_field::cleanup()
@@ -2265,197 +2459,6 @@ bool Item_field::send(Protocol *protocol, String *buffer)
 }
 
 
-/*
-  Search a GROUP BY clause for a field with a certain name.
-
-  SYNOPSIS
-    find_field_in_group_list()
-    find_item  the item being searched for
-    group_list GROUP BY clause
-
-  DESCRIPTION
-    Search the GROUP BY list for a column named as find_item. When searching
-    preference is given to columns that are qualified with the same table (and
-    database) name as the one being searched for.
-
-  RETURN
-    - the found item on success
-    - NULL if find_item is not in group_list
-*/
-
-static Item** find_field_in_group_list(Item *find_item, ORDER *group_list)
-{
-  const char *db_name;
-  const char *table_name;
-  const char *field_name;
-  ORDER      *found_group= NULL;
-  int         found_match_degree= 0;
-  Item_field *cur_field;
-  int         cur_match_degree= 0;
-
-  if (find_item->type() == Item::FIELD_ITEM ||
-      find_item->type() == Item::REF_ITEM)
-  {
-    db_name=    ((Item_ident*) find_item)->db_name;
-    table_name= ((Item_ident*) find_item)->table_name;
-    field_name= ((Item_ident*) find_item)->field_name;
-  }
-  else
-    return NULL;
-
-  DBUG_ASSERT(field_name);
-
-  for (ORDER *cur_group= group_list ; cur_group ; cur_group= cur_group->next)
-  {
-    if ((*(cur_group->item))->type() == Item::FIELD_ITEM)
-    {
-      cur_field= (Item_field*) *cur_group->item;
-      cur_match_degree= 0;
-      
-      DBUG_ASSERT(cur_field->field_name);
-
-      if (!my_strcasecmp(system_charset_info,
-                         cur_field->field_name, field_name))
-        ++cur_match_degree;
-      else
-        continue;
-
-      if (cur_field->table_name && table_name)
-      {
-        /* If field_name is qualified by a table name. */
-        if (strcmp(cur_field->table_name, table_name))
-          /* Same field names, different tables. */
-          return NULL;
-
-        ++cur_match_degree;
-        if (cur_field->db_name && db_name)
-        {
-          /* If field_name is also qualified by a database name. */
-          if (strcmp(cur_field->db_name, db_name))
-            /* Same field names, different databases. */
-            return NULL;
-          ++cur_match_degree;
-        }
-      }
-
-      if (cur_match_degree > found_match_degree)
-      {
-        found_match_degree= cur_match_degree;
-        found_group= cur_group;
-      }
-      else if (found_group && (cur_match_degree == found_match_degree) &&
-               ! (*(found_group->item))->eq(cur_field, 0))
-      {
-        /*
-          If the current resolve candidate matches equally well as the current
-          best match, they must reference the same column, otherwise the field
-          is ambiguous.
-        */
-        my_printf_error(ER_NON_UNIQ_ERROR, ER(ER_NON_UNIQ_ERROR),
-                        MYF(0), find_item->full_name(), current_thd->where);
-        return NULL;
-      }
-    }
-  }
-
-  if (found_group)
-    return found_group->item;
-  else
-    return NULL;
-}
-
-
-/*
-  Resolve a column reference in a sub-select.
-
-  SYNOPSIS
-    resolve_ref_in_select_and_group()
-    thd     current thread
-    ref     column reference being resolved
-    select  the sub-select that ref is resolved against
-
-  DESCRIPTION
-    Resolve a column reference (usually inside a HAVING clause) against the
-    SELECT and GROUP BY clauses of the query described by 'select'. The name
-    resolution algorithm searches both the SELECT and GROUP BY clauses, and in
-    case of a name conflict prefers GROUP BY column names over SELECT names. If
-    both clauses contain different fields with the same names, a warning is
-    issued that name of 'ref' is ambiguous. 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.
-
-  NOTES
-    The resolution procedure is:
-    - Search for a column or derived column named col_ref_i [in table T_j]
-      in the SELECT clause of Q.
-    - 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.
-
-
-  RETURN
-    NULL - there was an error, and the error was already reported
-    not_found_item - the item was not resolved, no error was reported
-    resolved item - if the item was resolved
-*/
-static Item**
-resolve_ref_in_select_and_group(THD *thd, Item_ref *ref, SELECT_LEX *select)
-{
-  Item **group_by_ref= NULL;
-  Item **select_ref= NULL;
-  ORDER *group_list= (ORDER*) select->group_list.first;
-  bool ambiguous_fields= FALSE;
-  uint counter;
-
-  /*
-    Search for a column or derived column named as 'ref' in the SELECT
-    clause of the current select.
-  */
-  if (!(select_ref= find_item_in_list(ref, *(select->get_item_list()), &counter,
-                                      REPORT_EXCEPT_NOT_FOUND)))
-    return NULL; /* Some error occurred. */
-
-  /* If this is a non-aggregated field inside HAVING, search in GROUP BY. */
-  if (select->having_fix_field && !ref->with_sum_func && group_list)
-  {
-    group_by_ref= find_field_in_group_list(ref, group_list);
-    
-    /* Check if the fields found in SELECT and GROUP BY are the same field. */
-    if (group_by_ref && (select_ref != not_found_item) &&
-        !((*group_by_ref)->eq(*select_ref, 0)))
-    {
-      ambiguous_fields= TRUE;
-      push_warning_printf(thd, MYSQL_ERROR::WARN_LEVEL_WARN, ER_NON_UNIQ_ERROR,
-                          ER(ER_NON_UNIQ_ERROR), ref->full_name(),
-                          current_thd->where);
-
-    }
-  }
-
-  if (select_ref != not_found_item || group_by_ref)
-  {
-    if (select_ref != not_found_item && !ambiguous_fields)
-    {
-      if (*select_ref && !(*select_ref)->fixed)
-      {
-        my_error(ER_ILLEGAL_REFERENCE, MYF(0), ref->name,
-                 "forward reference in item list");
-        return NULL;
-      }
-      return (select->ref_pointer_array + counter);
-    }
-    else if (group_by_ref)
-      return group_by_ref;
-    else
-      DBUG_ASSERT(FALSE);
-  }
-  else
-    return (Item**) not_found_item;
-}
-
-
 /*
   Resolve the name of a column reference.
 
@@ -2539,7 +2542,7 @@ bool Item_ref::fix_fields(THD *thd, TABLE_LIST *tables, Item **reference)
                         DERIVED_TABLE_TYPE))
       {
         TABLE_LIST *table_list;
-        Field *tmp= (Field*) not_found_field;
+        Field *from_field= (Field*) not_found_field;
         SELECT_LEX *last= 0;
 
         for ( ; outer_sel ;
@@ -2569,14 +2572,16 @@ bool Item_ref::fix_fields(THD *thd, TABLE_LIST *tables, Item **reference)
           /* Search in the tables of the FROM clause of the outer select. */
           table_list= outer_sel->get_table_list();
           if (outer_sel->resolve_mode == SELECT_LEX::INSERT_MODE && table_list)
-          {
-            /* It is primary INSERT st_select_lex => skip the first table. */
+            /*
+              It is a primary INSERT st_select_lex => do not resolve against the
+              first table.
+            */
             table_list= table_list->next_local;
-          }
+
           enum_parsing_place place= prev_subselect_item->parsing_place;
           /*
             Check table fields only if the subquery is used somewhere out of
-            HAVING or SELECT list, or outer SELECT does not use grouping
+            HAVING or SELECT list, or the outer SELECT does not use grouping
             (i.e. tables are accessible).
             TODO: 
             Here we could first find the field anyway, and then test this
@@ -2588,13 +2593,15 @@ bool Item_ref::fix_fields(THD *thd, TABLE_LIST *tables, Item **reference)
                (!outer_sel->with_sum_func &&
                 outer_sel->group_list.elements == 0)))
           {
-            if ((tmp= find_field_in_tables(thd, this, table_list, reference,
-                                           IGNORE_EXCEPT_NON_UNIQUE, TRUE)) !=
+            if ((from_field= find_field_in_tables(thd, this, table_list,
+                                                  reference,
+                                                  IGNORE_EXCEPT_NON_UNIQUE,
+                                                  TRUE)) !=
                 not_found_field)
             {
-              if (tmp != view_ref_found)
+              if (from_field != view_ref_found)
               {
-                prev_subselect_item->used_tables_cache|= tmp->table->map;
+                prev_subselect_item->used_tables_cache|= from_field->table->map;
                 prev_subselect_item->const_item_cache= 0;
               }
               else
@@ -2618,21 +2625,21 @@ bool Item_ref::fix_fields(THD *thd, TABLE_LIST *tables, Item **reference)
         }
 
         DBUG_ASSERT(ref);
-        if (!tmp)
+        if (!from_field)
           return TRUE;
-        else if (ref == not_found_item && tmp == not_found_field)
+        if (ref == not_found_item && from_field == not_found_field)
         {
           my_printf_error(ER_BAD_FIELD_ERROR, ER(ER_BAD_FIELD_ERROR), MYF(0),
                           this->full_name(), current_thd->where);
           ref= 0;
           return TRUE;
         }
-        else if (tmp != not_found_field)
+        else if (from_field != not_found_field)
         {
           ref= 0; // To prevent "delete *ref;" on ~Item_ref() of this item
-          if (tmp != view_ref_found)
+          if (from_field != view_ref_found)
           {
-            Item_field* fld= new Item_field(tmp);
+            Item_field* fld= new Item_field(from_field);
             if (!((*reference)= fld))
               return TRUE;
             mark_as_dependent(thd, last, current_sel, fld);
@@ -2652,6 +2659,7 @@ bool Item_ref::fix_fields(THD *thd, TABLE_LIST *tables, Item **reference)
         }
         else
         {
+          /* Should be checked in resolve_ref_in_select_and_group(). */
           DBUG_ASSERT(*ref && (*ref)->fixed);
           mark_as_dependent(thd, last, current_sel, this);
         }