Changed the fixes for the following bugs:

Bug #39022: completed
Bug #39653: reverted as invalid
Bug #45640: ameliorated, simplified, optimized
Bug #48483: completed
Bug #49324: improved
Bug #51242/52336: reverted, applied a real fix.

mysql-test/r/having.result

@@ -470,9 +470,10 @@ WHERE table2.f1 = 2
 GROUP BY table1.f1, table2.f2
 HAVING (table2.f2 = 8 AND table1.f1 >= 6);
 id	select_type	table	type	possible_keys	key	key_len	ref	rows	filtered	Extra
-1	SIMPLE	NULL	NULL	NULL	NULL	NULL	NULL	NULL	NULL	Impossible HAVING noticed after reading const tables
+1	SIMPLE	table2	const	PRIMARY	PRIMARY	4	const	1	100.00	Using filesort
+1	SIMPLE	table1	ALL	NULL	NULL	NULL	NULL	4	100.00	Using where
 Warnings:
-Note	1003	select `test`.`table1`.`f1` AS `f1`,'7' AS `f2` from `test`.`t1` `table1` join `test`.`t1` `table2` where ((`test`.`table1`.`f3` = '9')) group by `test`.`table1`.`f1`,'7' having 0
+Note	1003	select `test`.`table1`.`f1` AS `f1`,'7' AS `f2` from `test`.`t1` `table1` join `test`.`t1` `table2` where ((`test`.`table1`.`f3` = '9')) group by `test`.`table1`.`f1`,'7' having (('7' = 8) and (`test`.`table1`.`f1` >= 6))
 EXPLAIN EXTENDED
 SELECT table1.f1, table2.f2
 FROM t1 AS table1
@@ -481,9 +482,10 @@ WHERE table2.f1 = 2
 GROUP BY table1.f1, table2.f2
 HAVING (table2.f2 = 8);
 id	select_type	table	type	possible_keys	key	key_len	ref	rows	filtered	Extra
-1	SIMPLE	NULL	NULL	NULL	NULL	NULL	NULL	NULL	NULL	Impossible HAVING noticed after reading const tables
+1	SIMPLE	table2	const	PRIMARY	PRIMARY	4	const	1	100.00	Using filesort
+1	SIMPLE	table1	ALL	NULL	NULL	NULL	NULL	4	100.00	Using where
 Warnings:
-Note	1003	select `test`.`table1`.`f1` AS `f1`,'7' AS `f2` from `test`.`t1` `table1` join `test`.`t1` `table2` where ((`test`.`table1`.`f3` = '9')) group by `test`.`table1`.`f1`,'7' having 0
+Note	1003	select `test`.`table1`.`f1` AS `f1`,'7' AS `f2` from `test`.`t1` `table1` join `test`.`t1` `table2` where ((`test`.`table1`.`f3` = '9')) group by `test`.`table1`.`f1`,'7' having ('7' = 8)
 DROP TABLE t1;
 #
 # Bug#52336 Segfault / crash in 5.1 copy_fields (param=0x9872980) at sql_select.cc:15355

mysql-test/r/innodb_mysql.result

@@ -2296,28 +2296,6 @@ id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
 drop table t1,t2;
 #
 #
-# Bug #39653: find_shortest_key in sql_select.cc does not consider
-#             clustered primary keys
-#
-CREATE TABLE t1 (a INT PRIMARY KEY, b INT, c INT, d INT, e INT, f INT,
-KEY (b,c)) ENGINE=INNODB;
-INSERT INTO t1 VALUES (1,1,1,1,1,1), (2,2,2,2,2,2), (3,3,3,3,3,3),
-(4,4,4,4,4,4), (5,5,5,5,5,5), (6,6,6,6,6,6),
-(7,7,7,7,7,7), (8,8,8,8,8,8), (9,9,9,9,9,9),
-(11,11,11,11,11,11);
-EXPLAIN SELECT COUNT(*) FROM t1;
-id	1
-select_type	SIMPLE
-table	t1
-type	index
-possible_keys	NULL
-key	b
-key_len	10
-ref	NULL
-rows	10
-Extra	Using index
-DROP TABLE t1;
-#
 # Bug #49838: DROP INDEX and ADD UNIQUE INDEX for same index may
 #   corrupt definition at engine
 #

mysql-test/t/innodb_mysql.test

@@ -560,23 +560,6 @@ drop table t1,t2;
 --echo #
 
 
---echo #
---echo # Bug #39653: find_shortest_key in sql_select.cc does not consider
---echo #             clustered primary keys
---echo #
-
-CREATE TABLE t1 (a INT PRIMARY KEY, b INT, c INT, d INT, e INT, f INT,
-                 KEY (b,c)) ENGINE=INNODB;
-
-INSERT INTO t1 VALUES (1,1,1,1,1,1), (2,2,2,2,2,2), (3,3,3,3,3,3),
-                      (4,4,4,4,4,4), (5,5,5,5,5,5), (6,6,6,6,6,6),
-		      (7,7,7,7,7,7), (8,8,8,8,8,8), (9,9,9,9,9,9),
-		      (11,11,11,11,11,11);
-
---query_vertical EXPLAIN SELECT COUNT(*) FROM t1
-
-DROP TABLE t1;
-
 --echo #
 --echo # Bug #49838: DROP INDEX and ADD UNIQUE INDEX for same index may
 --echo #   corrupt definition at engine

sql/filesort.cc

@@ -608,7 +608,9 @@ static ha_rows find_all_keys(SORTPARAM *param, SQL_SELECT *select,
     }
     if (error == 0)
       param->examined_rows++;
-    if (error == 0 && (!select || select->skip_record() == 0))
+   
+    int rc= 0;
+    if (error == 0 && (!select || select->skip_record(thd) > 0))
     {
       if (idx == param->keys)
       {
@@ -621,6 +623,7 @@ static ha_rows find_all_keys(SORTPARAM *param, SQL_SELECT *select,
     }
     else
       file->unlock_row();
+      
     /* It does not make sense to read more keys in case of a fatal error */
     if (thd->is_error())
       break;

sql/item.cc

@@ -4321,31 +4321,18 @@ bool Item_field::fix_fields(THD *thd, Item **reference)
               It's not an Item_field in the select list so we must make a new
               Item_ref to point to the Item in the select list and replace the
               Item_field created by the parser with the new Item_ref.
-
-              NOTE: If we are fixing an alias reference inside ORDER/GROUP BY
-              item tree, then we use new Item_ref as an intermediate value
-              to resolve referenced item only.
-              In this case the new Item_ref item is unused.
             */
             Item_ref *rf= new Item_ref(context, db_name,table_name,field_name);
             if (!rf)
               return 1;
-
-            bool save_group_fix_field= thd->lex->current_select->group_fix_field;
-            /*
-              No need for recursive resolving of aliases.
-            */
-            thd->lex->current_select->group_fix_field= 0;
-
             bool ret= rf->fix_fields(thd, (Item **) &rf) || rf->check_cols(1);
-            thd->lex->current_select->group_fix_field= save_group_fix_field;
             if (ret)
               return TRUE;
            
-            if (save_group_fix_field && alias_name_used)
-              thd->change_item_tree(reference, *rf->ref);
-            else
-              thd->change_item_tree(reference, rf);
+            SELECT_LEX *select= thd->lex->current_select;
+            thd->change_item_tree(reference,
+                                  select->parsing_place == IN_GROUP_BY && 
+				  alias_name_used  ?  *rf->ref : rf);
 
             return FALSE;
           }
@@ -6438,6 +6425,42 @@ bool Item_outer_ref::fix_fields(THD *thd, Item **reference)
 }
 
 
+/**
+  Mark references from inner selects used in group by clause
+
+  The method is used by the walk method when called for the expressions
+  from the group by clause. The callsare  occurred in the function
+  fix_inner_refs invoked by JOIN::prepare.
+  The parameter passed to Item_outer_ref::check_inner_refs_processor
+  is the iterator over the list of inner references from the subselects
+  of the select to be prepared. The function marks those references
+  from this list whose occurrences are encountered in the group by 
+  expressions passed to the walk method.  
+ 
+  @param arg  pointer to the iterator over a list of inner references
+
+  @return
+    FALSE always
+*/
+
+bool Item_outer_ref::check_inner_refs_processor(uchar *arg)
+{
+  List_iterator_fast<Item_outer_ref> *it=
+    ((List_iterator_fast<Item_outer_ref> *) arg);
+  Item_outer_ref *ref;
+  while ((ref= (*it)++))
+  {
+    if (ref == this)
+    {
+      ref->found_in_group_by= 1;
+      break;
+    }
+  }
+  (*it).rewind();
+  return FALSE;
+}
+
+
 /**
   Compare two view column references for equality.
 

sql/item.h

@@ -905,7 +905,6 @@ class Item {
   virtual bool change_context_processor(uchar *context) { return 0; }
   virtual bool reset_query_id_processor(uchar *query_id_arg) { return 0; }
   virtual bool is_expensive_processor(uchar *arg) { return 0; }
-  virtual bool find_item_processor(uchar *arg) { return this == (void *) arg; }
   virtual bool register_field_in_read_map(uchar *arg) { return 0; }
   virtual bool enumerate_field_refs_processor(uchar *arg) { return 0; }
   virtual bool mark_as_eliminated_processor(uchar *arg) { return 0; }
@@ -998,6 +997,8 @@ class Item {
     return FALSE;
   }
 
+  virtual bool check_inner_refs_processor(uchar *arg) { return FALSE; }
+
   /*
     For SP local variable returns pointer to Item representing its
     current value and pointer to current Item otherwise.
@@ -2450,12 +2451,13 @@ class Item_outer_ref :public Item_direct_ref
     of the outer select.
   */
   bool found_in_select_list;
+  bool found_in_group_by;
   Item_outer_ref(Name_resolution_context *context_arg,
                  Item_field *outer_field_arg)
     :Item_direct_ref(context_arg, 0, outer_field_arg->table_name,
                      outer_field_arg->field_name),
     outer_ref(outer_field_arg), in_sum_func(0),
-    found_in_select_list(0)
+    found_in_select_list(0), found_in_group_by(0)
   {
     ref= &outer_ref;
     set_properties();
@@ -2466,7 +2468,7 @@ class Item_outer_ref :public Item_direct_ref
                  bool alias_name_used_arg)
     :Item_direct_ref(context_arg, item, table_name_arg, field_name_arg,
                      alias_name_used_arg),
-    outer_ref(0), in_sum_func(0), found_in_select_list(1)
+    outer_ref(0), in_sum_func(0), found_in_select_list(1), found_in_group_by(0)
   {}
   void save_in_result_field(bool no_conversions)
   {
@@ -2478,6 +2480,7 @@ class Item_outer_ref :public Item_direct_ref
     return (*ref)->const_item() ? 0 : OUTER_REF_TABLE_BIT;
   }
   virtual Ref_Type ref_type() { return OUTER_REF; }
+  bool check_inner_refs_processor(uchar * arg); 
 };
 
 

sql/mysql_priv.h

@@ -640,7 +640,8 @@ enum enum_parsing_place
   IN_HAVING,
   SELECT_LIST,
   IN_WHERE,
-  IN_ON
+  IN_ON,
+  IN_GROUP_BY
 };
 
 struct st_table;
@@ -1196,7 +1197,7 @@ int setup_group(THD *thd, Item **ref_pointer_array, TABLE_LIST *tables,
 		List<Item> &fields, List<Item> &all_fields, ORDER *order,
 		bool *hidden_group_fields);
 bool fix_inner_refs(THD *thd, List<Item> &all_fields, SELECT_LEX *select,
-                   Item **ref_pointer_array, ORDER *group_list= NULL);
+                   Item **ref_pointer_array);
 
 bool handle_select(THD *thd, LEX *lex, select_result *result,
                    ulong setup_tables_done_option);

sql/opt_range.h

@@ -718,7 +718,19 @@ class SQL_SELECT :public Sql_alloc {
     tmp.set_all();
     return test_quick_select(thd, tmp, 0, limit, force_quick_range) < 0;
   }
-  inline bool skip_record() { return cond ? cond->val_int() == 0 : 0; }
+  /* 
+    RETURN
+      0   if record must be skipped <-> (cond && cond->val_int() == 0)
+     -1   if error
+      1   otherwise
+  */   
+  inline int skip_record(THD *thd)
+  {
+    int rc= test(!cond || cond->val_int());
+    if (thd->is_error())
+      rc= -1;
+    return rc;
+  }
   int test_quick_select(THD *thd, key_map keys, table_map prev_tables,
 			ha_rows limit, bool force_quick_range);
 };

sql/sql_delete.cc

@@ -305,9 +305,8 @@ bool mysql_delete(THD *thd, TABLE_LIST *table_list, COND *conds,
 	 ! thd->is_error())
   {
     // thd->is_error() is tested to disallow delete row on error
-    if (!(select && select->skip_record())&& ! thd->is_error() )
+    if (!select || select->skip_record(thd) > 0)
     {
-
       if (triggers_applicable &&
           table->triggers->process_triggers(thd, TRG_EVENT_DELETE,
                                             TRG_ACTION_BEFORE, FALSE))

sql/sql_lex.cc

@@ -1603,7 +1603,6 @@ void st_select_lex::init_query()
   having= prep_having= where= prep_where= 0;
   olap= UNSPECIFIED_OLAP_TYPE;
   having_fix_field= 0;
-  group_fix_field= 0;
   context.select_lex= this;
   context.init();
   /*

sql/sql_lex.h

@@ -647,8 +647,6 @@ class st_select_lex: public st_select_lex_node
   bool  braces;   	/* SELECT ... UNION (SELECT ... ) <- this braces */
   /* TRUE when having fix field called in processing of this SELECT */
   bool having_fix_field;
-  /* TRUE when GROUP BY fix field called in processing of this SELECT */
-  bool group_fix_field;
   /* List of references to fields referenced from inner selects */
   List<Item_outer_ref> inner_refs_list;
   /* Number of Item_sum-derived objects in this SELECT */

sql/sql_select.cc

@@ -317,8 +317,8 @@ bool handle_select(THD *thd, LEX *lex, select_result *result,
       function is aggregated in the select where the outer field was
       resolved or in some more inner select then the Item_direct_ref
       class should be used.
-      Also it should be used if we are grouping by a subquery containing
-      the outer field.
+      It used used also if we are grouping by a subquery that refers
+      this outer field.
     The resolution is done here and not at the fix_fields() stage as
     it can be done only after sum functions are fixed and pulled up to
     selects where they are have to be aggregated.
@@ -335,13 +335,25 @@ bool handle_select(THD *thd, LEX *lex, select_result *result,
 
 bool
 fix_inner_refs(THD *thd, List<Item> &all_fields, SELECT_LEX *select,
-                 Item **ref_pointer_array, ORDER *group_list)
+                 Item **ref_pointer_array)
 {
   Item_outer_ref *ref;
   bool res= FALSE;
   bool direct_ref= FALSE;
 
-  List_iterator<Item_outer_ref> ref_it(select->inner_refs_list);
+  /*
+    Mark the references from  the inner_refs_list that are occurred in
+    the group by expressions. Those references will contain direct
+    references to the referred fields. The markers are set in 
+    the found_in_group_by field of the references from the list.
+  */
+  List_iterator_fast <Item_outer_ref> ref_it(select->inner_refs_list);
+  for (ORDER *group= select->join->group_list; group;  group= group->next)
+  {
+    (*group->item)->walk(&Item::check_inner_refs_processor,
+                         TRUE, (uchar *) &ref_it);
+  } 
+    
   while ((ref= ref_it++))
   {
     Item *item= ref->outer_ref;
@@ -385,22 +397,9 @@ fix_inner_refs(THD *thd, List<Item> &all_fields, SELECT_LEX *select,
         }
       }
     }
-    else
-    {
-      /*
-        Check if GROUP BY item trees contain the outer ref:
-        in this case we have to use Item_direct_ref instead of Item_ref.
-      */
-      for (ORDER *group= group_list; group; group= group->next)
-      {
-        if ((*group->item)->walk(&Item::find_item_processor, TRUE,
-                                 (uchar *) ref))
-        {
+    else if (ref->found_in_group_by)
       direct_ref= TRUE;
-          break;
-        }
-      }
-    }
+
     new_ref= direct_ref ?
               new Item_direct_ref(ref->context, item_ref, ref->table_name,
                           ref->field_name, ref->alias_name_used) :
@@ -607,8 +606,7 @@ JOIN::prepare(Item ***rref_pointer_array,
   }
 
   if (select_lex->inner_refs_list.elements &&
-      fix_inner_refs(thd, all_fields, select_lex, ref_pointer_array,
-                     group_list))
+      fix_inner_refs(thd, all_fields, select_lex, ref_pointer_array))
     DBUG_RETURN(-1);
 
   if (group_list)
@@ -1123,31 +1121,6 @@ JOIN::optimize()
   {
     conds=new Item_int((longlong) 0,1);	// Always false
   }
-
-  /*
-    It's necessary to check const part of HAVING cond as
-    there is a chance that some cond parts may become
-    const items after make_join_statisctics(for example
-    when Item is a reference to cost table field from
-    outer join).
-    This check is performed only for those conditions
-    which do not use aggregate functions. In such case
-    temporary table may not be used and const condition
-    elements may be lost during further having
-    condition transformation in JOIN::exec.
-  */
-  if (having && const_table_map)
-  {
-    having->update_used_tables();
-    having= remove_eq_conds(thd, having, &having_value);
-    if (having_value == Item::COND_FALSE)
-    {
-      having= new Item_int((longlong) 0,1);
-      zero_result_cause= "Impossible HAVING noticed after reading const tables";
-      DBUG_RETURN(0);
-    }
-  }
-
   if (make_join_select(this, select, conds))
   {
     zero_result_cause=
@@ -2210,7 +2183,7 @@ JOIN::exec()
 
       Item* sort_table_cond= make_cond_for_table(curr_join->tmp_having,
 						 used_tables,
-						 used_tables);
+						 (table_map) 0);
       if (sort_table_cond)
       {
 	if (!curr_table->select)
@@ -8943,7 +8916,8 @@ simplify_joins(JOIN *join, List<TABLE_LIST> *join_list, COND *conds, bool top)
           For example it might happen if RAND() function
           is used in JOIN ON clause.
 	*/  
-        if (!((prev_table->on_expr->used_tables() & ~RAND_TABLE_BIT) &
+        if (!((prev_table->on_expr->used_tables() &
+               ~(OUTER_REF_TABLE_BIT | RAND_TABLE_BIT)) &
               ~prev_used_tables))
           prev_table->dep_tables|= used_tables;
       }
@@ -11867,40 +11841,24 @@ flush_cached_records(JOIN *join,JOIN_TAB *join_tab,bool skip_last)
       join->thd->send_kill_message();
       return NESTED_LOOP_KILLED; // Aborted by user /* purecov: inspected */
     }
+    int err= 0;
     SQL_SELECT *select=join_tab->select;
-    if (rc == NESTED_LOOP_OK)
+    if (rc == NESTED_LOOP_OK &&
+        (!join_tab->cache.select ||
+         (err= join_tab->cache.select->skip_record(join->thd)) != 0 ))
     {
-      bool consider_record= !join_tab->cache.select || 
-        !join_tab->cache.select->skip_record();
-
-      /*
-        Check for error: skip_record() can execute code by calling
-        Item_subselect::val_*. We need to check for errors (if any)
-        after such call.
-      */
-      if (join->thd->is_error())
-      {
-        reset_cache_write(&join_tab->cache);
+      if (err < 0)
         return NESTED_LOOP_ERROR;
-      }
-
-      if (consider_record)  
-      {
-        uint i;
+      rc= NESTED_LOOP_OK;
       reset_cache_read(&join_tab->cache);
-        for (i=(join_tab->cache.records- (skip_last ? 1 : 0)) ; i-- > 0 ;)
+      for (uint i= (join_tab->cache.records- (skip_last ? 1 : 0)) ; i-- > 0 ;)
       {
 	read_cached_record(join_tab);
-          if (!select || !select->skip_record())
+        err= 0;
+	if (!select || (err= select->skip_record(join->thd)) != 0)
         {
-            /*
-              Check for error: skip_record() can execute code by calling
-              Item_subselect::val_*. We need to check for errors (if any)
-              after such call.
-              */
-            if (join->thd->is_error())
-              rc= NESTED_LOOP_ERROR;
-            else
+          if (err < 0)
+            return NESTED_LOOP_ERROR;
           rc= (join_tab->next_select)(join,join_tab+1,0);
 	  if (rc != NESTED_LOOP_OK && rc != NESTED_LOOP_NO_MORE_ROWS)
           {
@@ -11910,7 +11868,9 @@ flush_cached_records(JOIN *join,JOIN_TAB *join_tab,bool skip_last)
         }
       }
     }
-    }
+      
+    rc= NESTED_LOOP_OK;
+
   } while (!(error=info->read_record(info)));
 
   if (skip_last)
@@ -13280,35 +13240,12 @@ static int test_if_order_by_key(ORDER *order, TABLE *table, uint idx,
 
 uint find_shortest_key(TABLE *table, const key_map *usable_keys)
 {
+  uint min_length= (uint) ~0;
   uint best= MAX_KEY;
-  uint usable_clustered_pk= (table->file->primary_key_is_clustered() &&
-                             table->s->primary_key != MAX_KEY &&
-                             usable_keys->is_set(table->s->primary_key)) ?
-                            table->s->primary_key : MAX_KEY;
   if (!usable_keys->is_clear_all())
   {
-    uint min_length= (uint) ~0;
     for (uint nr=0; nr < table->s->keys ; nr++)
     {
-      /*
-       As far as 
-       1) clustered primary key entry data set is a set of all record
-          fields (key fields and not key fields) and
-       2) secondary index entry data is a union of its key fields and
-          primary key fields (at least InnoDB and its derivatives don't
-          duplicate primary key fields there, even if the primary and
-          the secondary keys have a common subset of key fields),
-       then secondary index entry data is always a subset of primary key
-       entry, and the PK is always longer.
-       Unfortunately, key_info[nr].key_length doesn't show the length
-       of key/pointer pair but a sum of key field lengths only, thus
-       we can't estimate index IO volume comparing only this key_length
-       value of seconday keys and clustered PK.
-       So, try secondary keys first, and choose PK only if there are no
-       usable secondary covering keys:
-      */
-      if (nr == usable_clustered_pk)
-        continue;
       if (usable_keys->is_set(nr))
       {
         if (table->key_info[nr].key_length < min_length)
@@ -13319,7 +13256,7 @@ uint find_shortest_key(TABLE *table, const key_map *usable_keys)
       }
     }
   }
-  return best != MAX_KEY ? best : usable_clustered_pk;
+  return best;
 }
 
 /**
@@ -13768,9 +13705,47 @@ test_if_skip_sort_order(JOIN_TAB *tab,ORDER *order,ha_rows select_limit,
               key (e.g. as in Innodb). 
               See Bug #28591 for details.
             */  
-            rec_per_key= used_key_parts &&
-                         used_key_parts <= keyinfo->key_parts ?
-                         keyinfo->rec_per_key[used_key_parts-1] : 1;
+             uint used_index_parts= keyinfo->key_parts;
+             uint used_pk_parts= 0;
+             if (used_key_parts > used_index_parts)
+               used_pk_parts= used_key_parts-used_index_parts;
+             rec_per_key= keyinfo->rec_per_key[used_key_parts-1];
+             /* Take into account the selectivity of the used pk prefix */
+             if (used_pk_parts)
+	     {
+               KEY *pkinfo=tab->table->key_info+table->s->primary_key;
+               /*
+                 If the values of of records per key for the prefixes
+                 of the primary key are considered unknown we assume
+                 they are equal to 1.
+	       */
+               if (used_key_parts == pkinfo->key_parts ||
+                   pkinfo->rec_per_key[0] == 0)
+                 rec_per_key= 1;                 
+               if (rec_per_key > 1)
+	       {
+                 rec_per_key*= pkinfo->rec_per_key[used_pk_parts-1];
+                 rec_per_key/= pkinfo->rec_per_key[0];
+                 /* 
+                   The value of rec_per_key for the extended key has
+                   to be adjusted accordingly if some components of
+                   the secondary key are included in the primary key.
+	         */
+                  for(uint i= 0; i < used_pk_parts; i++)
+	         {
+		   if (pkinfo->key_part[i].field->key_start.is_set(nr))
+		   {
+                     /* 
+                       We presume here that for any index rec_per_key[i] != 0
+                       if rec_per_key[0] != 0.
+		     */
+                     DBUG_ASSERT(pkinfo->rec_per_key[i]);
+                     rec_per_key*= pkinfo->rec_per_key[i-1];
+                     rec_per_key/= pkinfo->rec_per_key[i];
+                   }
+	         }
+               }    
+             }
              set_if_bigger(rec_per_key, 1);
             /*
               With a grouping query each group containing on average
@@ -14958,29 +14933,11 @@ find_order_in_list(THD *thd, Item **ref_pointer_array, TABLE_LIST *tables,
 
     We check order_item->fixed because Item_func_group_concat can put
     arguments for which fix_fields already was called.    
-    
-    group_fix_field= TRUE is to resolve aliases from the SELECT list
-    without creating of Item_ref-s: JOIN::exec() wraps aliased items
-    in SELECT list with Item_copy items. To re-evaluate such a tree
-    that includes Item_copy items we have to refresh Item_copy caches,
-    but:
-      - filesort() never refresh Item_copy items,
-      - end_send_group() checks every record for group boundary by the
-        test_if_group_changed function that obtain data from these
-        Item_copy items, but the copy_fields function that
-        refreshes Item copy items is called after group boundaries only -
-        that is a vicious circle.
-    So we prevent inclusion of Item_copy items.
-  */
-  bool save_group_fix_field= thd->lex->current_select->group_fix_field;
-  if (is_group_field)
-    thd->lex->current_select->group_fix_field= TRUE;
-  bool ret= (!order_item->fixed &&
+  */
+  if (!order_item->fixed &&
       (order_item->fix_fields(thd, order->item) ||
        (order_item= *order->item)->check_cols(1) ||
-       thd->is_fatal_error));
-  thd->lex->current_select->group_fix_field= save_group_fix_field;
-  if (ret)
+       thd->is_fatal_error))
     return TRUE; /* Wrong field. */
 
   uint el= all_fields.elements;
@@ -15052,6 +15009,8 @@ setup_group(THD *thd, Item **ref_pointer_array, TABLE_LIST *tables,
   uint org_fields=all_fields.elements;
 
   thd->where="group statement";
+  enum_parsing_place save_place= thd->lex->current_select->parsing_place;
+  thd->lex->current_select->parsing_place= IN_GROUP_BY;
   for (ord= order; ord; ord= ord->next)
   {
     if (find_order_in_list(thd, ref_pointer_array, tables, ord, fields,
@@ -15064,6 +15023,8 @@ setup_group(THD *thd, Item **ref_pointer_array, TABLE_LIST *tables,
       return 1;
     }
   }
+  thd->lex->current_select->parsing_place= save_place;
+
   if (thd->variables.sql_mode & MODE_ONLY_FULL_GROUP_BY)
   {
     /*

sql/sql_update.cc

@@ -469,9 +469,10 @@ int mysql_update(THD *thd,
       thd_proc_info(thd, "Searching rows for update");
       ha_rows tmp_limit= limit;
 
-      while (!(error=info.read_record(&info)) && !thd->killed)
+      while (!(error=info.read_record(&info)) && 
+             !thd->killed && !thd->is_error())
       {
-	if (!(select && select->skip_record()))
+	if (!select || select->skip_record(thd) > 0)
 	{
           if (table->file->was_semi_consistent_read())
 	    continue;  /* repeat the read of the same row if it still exists */
@@ -577,7 +578,7 @@ int mysql_update(THD *thd,
 
   while (!(error=info.read_record(&info)) && !thd->killed)
   {
-    if (!(select && select->skip_record()))
+    if (!select || select->skip_record(thd) > 0)
     {
       if (table->file->was_semi_consistent_read())
         continue;  /* repeat the read of the same row if it still exists */