Final solution for bug# 4302 "Ambiguos order by when renamed column is

identical to another in result"
According to SQL standard queries like 
"select t1.a as col from t1, t2 order by a" should return an error if
both tables contain field a.

mysql-test/r/order_by.result

@@ -680,6 +680,9 @@ order by col;
 ERROR 23000: Column 'col' in order clause is ambiguous
 select col1 from t1, t2 where t1.col1=t2.col2 order by col;
 ERROR 23000: Column 'col' in order clause is ambiguous
+select t1.col as t1_col, t2.col2 from t1, t2 where t1.col1=t2.col2
+order by col;
+ERROR 23000: Column 'col' in order clause is ambiguous
 select t1.col as t1_col, t2.col from t1, t2 where t1.col1=t2.col2
 order by col;
 t1_col	col
@@ -696,12 +699,6 @@ col	col2
 1	3
 2	2
 3	1
-select t1.col as t1_col, t2.col2 from t1, t2 where t1.col1=t2.col2
-order by col;
-t1_col	col2
-1	1
-2	2
-3	3
 select t2.col2, t2.col, t2.col from t2 order by col;
 col2	col	col
 3	1	1

mysql-test/t/order_by.test

@@ -472,13 +472,14 @@ select t1.col as c1, t2.col as c2 from t1, t2 where t1.col1=t2.col2
   order by col;
 --error 1052
 select col1 from t1, t2 where t1.col1=t2.col2 order by col;
+--error 1052
+select t1.col as t1_col, t2.col2 from t1, t2 where t1.col1=t2.col2
+  order by col;
 
 select t1.col as t1_col, t2.col from t1, t2 where t1.col1=t2.col2
   order by col;
 select col2 as c, col as c from t2 order by col;
 select col2 as col, col as col2 from t2 order by col; 
-select t1.col as t1_col, t2.col2 from t1, t2 where t1.col1=t2.col2
-  order by col;
 select t2.col2, t2.col, t2.col from t2 order by col;
 
 select t2.col2 as col from t2 order by t2.col;

sql/item.cc

@@ -1246,6 +1246,7 @@ bool Item_field::fix_fields(THD *thd, TABLE_LIST *tables, Item **ref)
       TABLE_LIST *table_list;
       Item **refer= (Item **)not_found_item;
       uint counter;
+      bool not_used;
       // 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)
@@ -1288,7 +1289,8 @@ bool Item_field::fix_fields(THD *thd, TABLE_LIST *tables, Item **ref)
           }
 	  if (sl->resolve_mode == SELECT_LEX::SELECT_MODE &&
 	      (refer= find_item_in_list(this, sl->item_list, &counter,
-					 REPORT_EXCEPT_NOT_FOUND)) !=
+                                        REPORT_EXCEPT_NOT_FOUND,
+                                        &not_used)) !=
 	       (Item **) not_found_item)
 	  {
 	    if (*refer && (*refer)->fixed) // Avoid crash in case of error
@@ -1889,6 +1891,7 @@ bool Item_ref::fix_fields(THD *thd,TABLE_LIST *tables, Item **reference)
 {
   DBUG_ASSERT(fixed == 0);
   uint counter;
+  bool not_used;
   if (!ref)
   {
     TABLE_LIST *where= 0, *table_list;
@@ -1908,13 +1911,13 @@ bool Item_ref::fix_fields(THD *thd,TABLE_LIST *tables, Item **reference)
 				  first_select()->linkage !=
 				  DERIVED_TABLE_TYPE) ? 
 				  REPORT_EXCEPT_NOT_FOUND :
-				  REPORT_ALL_ERRORS))) ==
+				  REPORT_ALL_ERRORS ), &not_used)) ==
 	(Item **)not_found_item)
     {
       upward_lookup= 1;
       Field *tmp= (Field*) not_found_field;
       /*
-	We can't find table field in table list of current select,
+	We can't find table field in select 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
@@ -1929,8 +1932,8 @@ bool Item_ref::fix_fields(THD *thd,TABLE_LIST *tables, Item **reference)
 	Item_subselect *prev_subselect_item= prev_unit->item;
 	if (sl->resolve_mode == SELECT_LEX::SELECT_MODE &&
 	    (ref= find_item_in_list(this, sl->item_list,
-				    &counter,
-				    REPORT_EXCEPT_NOT_FOUND)) !=
+                                    &counter, REPORT_EXCEPT_NOT_FOUND,
+                                    &not_used)) !=
 	   (Item **)not_found_item)
 	{
 	  if (*ref && (*ref)->fixed) // Avoid crash in case of error
@@ -1989,8 +1992,7 @@ bool Item_ref::fix_fields(THD *thd,TABLE_LIST *tables, Item **reference)
 	  // Call to report error
 	  find_item_in_list(this,
 			    *(thd->lex->current_select->get_item_list()),
-			    &counter,
-			    REPORT_ALL_ERRORS);
+			    &counter, REPORT_ALL_ERRORS, &not_used);
 	}
         ref= 0;
 	return 1;

sql/mysql_priv.h

@@ -705,7 +705,8 @@ 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);
+                          find_item_error_report_type report_error,
+                          bool *unaliased);
 bool get_key_map_from_key_list(key_map *map, TABLE *table,
                                List<String> *index_list);
 bool insert_fields(THD *thd,TABLE_LIST *tables,

sql/sql_base.cc

@@ -2082,14 +2082,17 @@ find_field_in_tables(THD *thd, Item_ident *item, TABLE_LIST *tables,
 				return not_found_item, report other errors,
 				return 0
       IGNORE_ERRORS		Do not report errors, return 0 if error
-      
+    unaliased                   Set to true if item is field which was found
+                                by original field name and not by its alias
+                                in item list. Set to false otherwise.
+
   RETURN VALUES
     0			Item is not found or item is not unique,
 			error message is reported
     not_found_item	Function was called with
 			report_error == REPORT_EXCEPT_NOT_FOUND and
 			item was not found. No error message was reported
-    found field 
+                        found field
 */
 
 // Special Item pointer for find_item_in_list returning
@@ -2098,7 +2101,7 @@ const Item **not_found_item= (const Item**) 0x1;
 
 Item **
 find_item_in_list(Item *find, List<Item> &items, uint *counter,
-		  find_item_error_report_type report_error)
+                  find_item_error_report_type report_error, bool *unaliased)
 {
   List_iterator<Item> li(items);
   Item **found=0, **found_unaliased= 0, *item;
@@ -2107,6 +2110,9 @@ find_item_in_list(Item *find, List<Item> &items, uint *counter,
   const char *table_name=0;
   bool found_unaliased_non_uniq= 0;
   uint unaliased_counter;
+
+  *unaliased= FALSE;
+
   if (find->type() == Item::FIELD_ITEM	|| find->type() == Item::REF_ITEM)
   {
     field_name= ((Item_ident*) find)->field_name;
@@ -2134,17 +2140,18 @@ find_item_in_list(Item *find, List<Item> &items, uint *counter,
         /*
           If table name is specified we should find field 'field_name' in
           table 'table_name'. According to SQL-standard we should ignore
-          aliases in this case. Note that we should prefer fields from the
-          select list over other fields from the tables participating in
-          this select in case of ambiguity.
+          aliases in this case.
+
+          Since we should NOT prefer fields from the select list over
+          other fields from the tables participating in this select in
+          case of ambiguity we have to do extra check outside this function.
 
           We use strcmp for table names and database names as these may be
-          case sensitive.
-          In cases where they are not case sensitive, they are always in lower
-          case.
+          case sensitive. In cases where they are not case sensitive, they
+          are always in lower case.
 
 	  item_field->field_name and item_field->table_name can be 0x0 if
-	  item is not fix fielded yet.
+	  item is not fix_field()'ed yet.
         */
         if (item_field->field_name && item_field->table_name &&
 	    !my_strcasecmp(system_charset_info, item_field->field_name,
@@ -2153,17 +2160,22 @@ find_item_in_list(Item *find, List<Item> &items, uint *counter,
             (!db_name || (item_field->db_name &&
                           !strcmp(item_field->db_name, db_name))))
         {
-          if (found)
+          if (found_unaliased)
           {
-            if ((*found)->eq(item, 0))
-              continue;                         // Same field twice
+            if ((*found_unaliased)->eq(item, 0))
+              continue;
+            /*
+              Two matching fields in select list.
+              We already can bail out because we are searching through
+              unaliased names only and will have duplicate error anyway.
+            */
             if (report_error != IGNORE_ERRORS)
               my_printf_error(ER_NON_UNIQ_ERROR, ER(ER_NON_UNIQ_ERROR),
                               MYF(0), find->full_name(), current_thd->where);
             return (Item**) 0;
           }
-          found= li.ref();
-          *counter= i;
+          found_unaliased= li.ref();
+          unaliased_counter= i;
           if (db_name)
             break;                              // Perfect match
         }
@@ -2235,6 +2247,7 @@ find_item_in_list(Item *find, List<Item> &items, uint *counter,
     {
       found= found_unaliased;
       *counter= unaliased_counter;
+      *unaliased= TRUE;
     }
   }
   if (found)

sql/sql_select.cc

@@ -7946,15 +7946,14 @@ find_order_in_list(THD *thd, Item **ref_pointer_array,
 		   TABLE_LIST *tables,ORDER *order, List<Item> &fields,
 		   List<Item> &all_fields)
 {
-  Item *itemptr=*order->item;
-  if (itemptr->type() == Item::INT_ITEM)
+  Item *it= *order->item;
+  if (it->type() == Item::INT_ITEM)
   {						/* Order by position */
-    uint count= (uint) itemptr->val_int();
+    uint count= (uint) it->val_int();
     if (!count || count > fields.elements)
     {
       my_printf_error(ER_BAD_FIELD_ERROR,ER(ER_BAD_FIELD_ERROR),
-		      MYF(0),itemptr->full_name(),
-	       thd->where);
+		      MYF(0), it->full_name(), thd->where);
       return 1;
     }
     order->item= ref_pointer_array + count-1;
@@ -7962,20 +7961,28 @@ find_order_in_list(THD *thd, Item **ref_pointer_array,
     return 0;
   }
   uint counter;
-  Item **item= find_item_in_list(itemptr, fields, &counter,
-                                 REPORT_EXCEPT_NOT_FOUND);
+  bool unaliased;
+  Item **item= find_item_in_list(it, fields, &counter,
+                                 REPORT_EXCEPT_NOT_FOUND, &unaliased);
   if (!item)
     return 1;
 
   if (item != (Item **)not_found_item)
   {
+    /*
+      If we have found field not by its alias in select list but by its
+      original field name, we should additionaly check if we have conflict
+      for this name (in case if we would perform lookup in all tables).
+    */
+    if (unaliased && !it->fixed && it->fix_fields(thd, tables, order->item))
+      return 1;
+
     order->item= ref_pointer_array + counter;
     order->in_field_list=1;
     return 0;
   }
 
   order->in_field_list=0;
-  Item *it= *order->item;
   /*
     We check it->fixed because Item_func_group_concat can put
     arguments for which fix_fields already was called.
@@ -8104,10 +8111,11 @@ setup_new_fields(THD *thd,TABLE_LIST *tables,List<Item> &fields,
 
   thd->set_query_id=1;				// Not really needed, but...
   uint counter;
+  bool not_used;
   for (; new_field ; new_field= new_field->next)
   {
     if ((item= find_item_in_list(*new_field->item, fields, &counter,
-				 IGNORE_ERRORS)))
+				 IGNORE_ERRORS, &not_used)))
       new_field->item=item;			/* Change to shared Item */
     else
     {