fixed brocken of client_test

fixed aggregate functions in PS (BUG#3360)


mysql-test/r/union.result:
  4.1 more correct error
sql/item_cmpfunc.h:
  and_conds do not make fix fields
sql/item_sum.cc:
  storing/restoring argument of aggregate function for prepared statements
  restoring order list of group_concat for safety
sql/item_sum.h:
  storing/restoring argument of aggregate function for prepared statements
  layout fix
sql/mysql_priv.h:
  just declaration
sql/sql_base.cc:
  fix_fields() have to be called with temporary memory pool active
sql/sql_parse.cc:
  removed hack with item pointer storing
sql/sql_prepare.cc:
  debug output added
  removed hack with item pointer storing
sql/sql_select.cc:
  fix_fields now should be called separately
sql/sql_union.cc:
  removed wrong merged check from 4.0 (4.1 have its own protection)
sql/table.h:
  removed hack with item pointer storing
tests/client_test.c:
  new test fo PS

mysql-test/r/union.result

@@ -435,7 +435,7 @@ drop temporary table t1;
 create table t1 select a from t1 union select a from t2;
 ERROR HY000: You can't specify target table 't1' for update in FROM clause
 select a from t1 union select a from t2 order by t2.a;
-ERROR 42S22: Unknown column 't2.a' in 'ORDER BY'
+ERROR 42S02: Unknown table 't2' in order clause
 drop table t1,t2;
 select length(version()) > 1 as `*` UNION select 2;
 *

sql/item_cmpfunc.h

@@ -991,14 +991,11 @@ class Item_cond_xor :public Item_cond
 
 /* Some usefull inline functions */
 
-inline Item *and_conds(THD *thd, Item *a, Item *b, TABLE_LIST *tables)
+inline Item *and_conds(Item *a, Item *b)
 {
   if (!b) return a;
   if (!a) return b;
-  Item *cond= new Item_cond_and(a,b);
-  if (cond)
-    cond->fix_fields(thd, tables, &cond);
-  return cond;
+  return new Item_cond_and(a, b);
 }
 
 Item *and_expressions(Item *a, Item *b, Item **org_item);

sql/item_sum.cc

@@ -24,8 +24,8 @@
 #include "mysql_priv.h"
 
 Item_sum::Item_sum(List<Item> &list)
+  :args_copy(0), arg_count(list.elements)
 {
-  arg_count=list.elements;
   if ((args=(Item**) sql_alloc(sizeof(Item*)*arg_count)))
   {
     uint i=0;
@@ -43,18 +43,51 @@ Item_sum::Item_sum(List<Item> &list)
 
 // Constructor used in processing select with temporary tebles
 Item_sum::Item_sum(THD *thd, Item_sum *item):
-  Item_result_field(thd, item), quick_group(item->quick_group)
+  Item_result_field(thd, item), arg_count(item->arg_count),
+  quick_group(item->quick_group)
 {
-  arg_count= item->arg_count;
   if (arg_count <= 2)
     args=tmp_args;
   else
-    if (!(args=(Item**) sql_alloc(sizeof(Item*)*arg_count)))
+    if (!(args= (Item**) thd->alloc(sizeof(Item*)*arg_count)))
       return;
-  for (uint i= 0; i < arg_count; i++)
-    args[i]= item->args[i];
+  memcpy(args, item->args, sizeof(Item*)*arg_count);
+  if (item->args_copy != 0)
+    save_args(thd);
+  else
+    args_copy= 0;
+}
+
+
+/*
+  Save copy of arguments if we are prepare prepared statement
+  (arguments can be rewritten in get_tmp_table_item())
+
+  SYNOPSIS
+    Item_sum::save_args_for_prepared_statements()
+    thd		- thread handler
+
+  RETURN
+    0 - OK
+    1 - Error
+*/
+bool Item_sum::save_args_for_prepared_statements(THD *thd)
+{
+  if (thd->current_statement)
+    return save_args(thd->current_statement);
+  return 0;
 }
 
+
+bool Item_sum::save_args(Statement* stmt)
+{
+  if (!(args_copy= (Item**) stmt->alloc(sizeof(Item*)*arg_count)))
+    return 1;
+  memcpy(args_copy, args, sizeof(Item*)*arg_count);
+  return 0;
+}
+
+
 void Item_sum::mark_as_sum_func()
 {
   current_thd->lex->current_select->with_sum_func= 1;
@@ -62,6 +95,17 @@ void Item_sum::mark_as_sum_func()
 }
 
 
+void Item_sum::cleanup()
+{
+  DBUG_ENTER("Item_sum::cleanup");
+  Item_result_field::cleanup();
+  if (args_copy != 0)
+    memcpy(args, args_copy, sizeof(Item*)*arg_count);
+  result_field=0;
+  DBUG_VOID_RETURN;
+}
+
+
 void Item_sum::make_field(Send_field *tmp_field)
 {
   if (args[0]->type() == Item::FIELD_ITEM && keep_field_type())
@@ -165,6 +209,10 @@ bool
 Item_sum_num::fix_fields(THD *thd, TABLE_LIST *tables, Item **ref)
 {
   DBUG_ASSERT(fixed == 0);
+
+  if (save_args_for_prepared_statements(thd))
+    return 1;
+  
   if (!thd->allow_sum_func)
   {
     my_error(ER_INVALID_GROUP_FUNC_USE,MYF(0));
@@ -195,6 +243,10 @@ bool
 Item_sum_hybrid::fix_fields(THD *thd, TABLE_LIST *tables, Item **ref)
 {
   DBUG_ASSERT(fixed == 0);
+
+  if (save_args_for_prepared_statements(thd))
+    return 1;
+
   Item *item= args[0];
   if (!thd->allow_sum_func)
   {
@@ -1131,15 +1183,6 @@ void Item_sum_count_distinct::cleanup()
 }
 
 
-bool Item_sum_count_distinct::fix_fields(THD *thd, TABLE_LIST *tables,
-					 Item **ref)
-{
-  DBUG_ASSERT(fixed == 0);
-  if (Item_sum_num::fix_fields(thd, tables, ref))
-    return 1;
-  return 0;
-}
-
 /* This is used by rollup to create a separate usable copy of the function */
 
 void Item_sum_count_distinct::make_unique()
@@ -1705,6 +1748,11 @@ void Item_func_group_concat::cleanup()
 {
   DBUG_ENTER("Item_func_group_concat::cleanup");
   Item_sum::cleanup();
+
+  /* fix order list */
+  for (uint i= 0; i < arg_count_order ; i++)
+    order[i]->item= &order[i]->item_ptr;
+
   /*
     Free table and tree if they belong to this item (if item have not pointer
     to original item from which was made copy => it own its objects )
@@ -1821,6 +1869,10 @@ bool
 Item_func_group_concat::fix_fields(THD *thd, TABLE_LIST *tables, Item **ref)
 {
   DBUG_ASSERT(fixed == 0);
+
+  if (save_args_for_prepared_statements(thd))
+    return 1;
+
   uint i;			/* for loop variable */ 
 
   if (!thd->allow_sum_func)

sql/item_sum.h

@@ -32,40 +32,32 @@ class Item_sum :public Item_result_field
     UDF_SUM_FUNC, GROUP_CONCAT_FUNC
   };
 
-  Item **args,*tmp_args[2];
+  Item **args, *tmp_args[2];
+  Item **args_copy;			/* copy of arguments for PS */
   uint arg_count;
   bool quick_group;			/* If incremental update of fields */
 
   void mark_as_sum_func();
-  Item_sum() : arg_count(0),quick_group(1) 
+  Item_sum() :args_copy(0), arg_count(0), quick_group(1) 
   {
     mark_as_sum_func();
   }
-  Item_sum(Item *a) :quick_group(1)
+  Item_sum(Item *a)
+    :args(tmp_args), args_copy(0), arg_count(1), quick_group(1)
   {
-    arg_count=1;
-    args=tmp_args;
     args[0]=a;
     mark_as_sum_func();
   }
-  Item_sum( Item *a, Item *b ) :quick_group(1)
+  Item_sum( Item *a, Item *b )
+    :args(tmp_args), args_copy(0),  arg_count(2), quick_group(1)
   {
-    arg_count=2;
-    args=tmp_args;
     args[0]=a; args[1]=b;
     mark_as_sum_func();
   }
   Item_sum(List<Item> &list);
   //Copy constructor, need to perform subselects with temporary tables
   Item_sum(THD *thd, Item_sum *item);
-  void cleanup()
-  {
-    DBUG_ENTER("Item_sum::cleanup");
-    Item_result_field::cleanup();
-    result_field=0;
-    DBUG_VOID_RETURN;
-  }
-
+  void cleanup();
   enum Type type() const { return SUM_FUNC_ITEM; }
   virtual enum Sumfunctype sum_func () const=0;
   inline bool reset() { clear(); return add(); };
@@ -100,6 +92,8 @@ class Item_sum :public Item_result_field
   virtual bool setup(THD *thd) {return 0;}
   virtual void make_unique() {}
   Item *get_tmp_table_item(THD *thd);
+  bool save_args_for_prepared_statements(THD *);
+  bool save_args(Statement* stmt);
 
   bool walk (Item_processor processor, byte *argument);
 };
@@ -190,7 +184,6 @@ class Item_sum_count_distinct :public Item_sum_int
 {
   TABLE *table;
   table_map used_table_cache;
-  bool fix_fields(THD *thd, TABLE_LIST *tables, Item **ref);
   uint32 *field_lengths;
   TMP_TABLE_PARAM *tmp_table_param;
   TREE tree_base;
@@ -236,7 +229,8 @@ class Item_sum_count_distinct :public Item_sum_int
   Item_sum_count_distinct(THD *thd, Item_sum_count_distinct *item)
     :Item_sum_int(thd, item), table(item->table),
      used_table_cache(item->used_table_cache),
-     field_lengths(item->field_lengths), tmp_table_param(item->tmp_table_param),
+     field_lengths(item->field_lengths),
+     tmp_table_param(item->tmp_table_param),
      tree(item->tree), original(item), key_length(item->key_length),
      max_elements_in_tree(item->max_elements_in_tree),
      rec_offset(item->rec_offset), use_tree(item->use_tree),

sql/mysql_priv.h

@@ -285,6 +285,7 @@ void debug_sync_point(const char* lock_name, uint lock_timeout);
 
 struct st_table;
 class THD;
+class Statement;
 
 /* Struct to handle simple linked lists */
 

sql/sql_base.cc

@@ -2403,15 +2403,13 @@ int setup_conds(THD *thd,TABLE_LIST *tables,COND **conds)
   {
     if (table->on_expr)
     {
-      if (stmt)
-	thd->set_n_backup_item_arena(stmt, &backup);
       /* Make a join an a expression */
       thd->where="on clause";
       
       if (!table->on_expr->fixed &&
 	  table->on_expr->fix_fields(thd, tables, &table->on_expr) ||
 	  table->on_expr->check_cols(1))
-	goto err;
+	DBUG_RETURN(1);
       thd->lex->current_select->cond_count++;
 
       /*
@@ -2423,12 +2421,16 @@ int setup_conds(THD *thd,TABLE_LIST *tables,COND **conds)
 	   !(specialflag & SPECIAL_NO_NEW_FUNC)))
       {
 	table->outer_join= 0;
-	if (!(*conds= and_conds(thd, *conds, table->on_expr, tables)))
-	  goto err;
+	if (stmt)
+	  thd->set_n_backup_item_arena(stmt, &backup);
+	*conds= and_conds(*conds, table->on_expr);
 	table->on_expr=0;
+	if (stmt)
+	  thd->restore_backup_item_arena(stmt, &backup);
+	if ((*conds) && !(*conds)->fixed &&
+	    (*conds)->fix_fields(thd, tables, conds))
+	  DBUG_RETURN(1);
       }
-      if (stmt)
-	thd->restore_backup_item_arena(stmt, &backup);
     }
     if (table->natural_join)
     {
@@ -2467,20 +2469,28 @@ int setup_conds(THD *thd,TABLE_LIST *tables,COND **conds)
 
       if (!table->outer_join)			// Not left join
       {
-	if (!(*conds= and_conds(thd, *conds, cond_and, tables)) ||
-	    (*conds && !(*conds)->fixed &&
-	     (*conds)->fix_fields(thd, tables, conds)))
-	  goto err;
+	*conds= and_conds(*conds, cond_and);
+	// fix_fields() should be made with temporary memory pool
+	if (stmt)
+	  thd->restore_backup_item_arena(stmt, &backup);
+	if (*conds && !(*conds)->fixed)
+	{
+	  if ((*conds)->fix_fields(thd, tables, conds))
+	    DBUG_RETURN(1);
+	}
       }
       else
       {
-	table->on_expr= and_conds(thd, table->on_expr, cond_and, tables);
-	if (table->on_expr && !table->on_expr->fixed &&
-	    table->on_expr->fix_fields(thd, tables, &table->on_expr))
-	  goto err;
+	table->on_expr= and_conds(table->on_expr, cond_and);
+	// fix_fields() should be made with temporary memory pool
+	if (stmt)
+	  thd->restore_backup_item_arena(stmt, &backup);
+	if (table->on_expr && !table->on_expr->fixed)
+	{
+	  if (table->on_expr->fix_fields(thd, tables, &table->on_expr))
+	   DBUG_RETURN(1);
+	}
       }
-      if (stmt)
-	thd->restore_backup_item_arena(stmt, &backup);
     }
   }
 

sql/sql_parse.cc

@@ -4389,11 +4389,10 @@ bool add_to_list(THD *thd, SQL_LIST &list,Item *item,bool asc)
   ORDER *order;
   Item	**item_ptr;
   DBUG_ENTER("add_to_list");
-  if (!(order = (ORDER *) thd->alloc(sizeof(ORDER)+sizeof(Item*))))
+  if (!(order = (ORDER *) thd->alloc(sizeof(ORDER))))
     DBUG_RETURN(1);
-  item_ptr = (Item**) (order+1);
-  *item_ptr=item;
-  order->item= item_ptr;
+  order->item_ptr= item;
+  order->item= &order->item_ptr;
   order->asc = asc;
   order->free_me=0;
   order->used=0;

sql/sql_prepare.cc

@@ -987,6 +987,8 @@ void mysql_stmt_prepare(THD *thd, char *packet, uint packet_length)
   int error;
   DBUG_ENTER("mysql_stmt_prepare");
 
+  DBUG_PRINT("pquery", ("%s", packet));
+
   if (stmt == 0)
   {
     send_error(thd, ER_OUT_OF_RESOURCES);
@@ -1076,11 +1078,11 @@ static void reset_stmt_for_execute(Prepared_statement *stmt)
     DBUG_ASSERT(sl->join == 0);
     ORDER *order;
     /* Fix GROUP list */
-    for (order=(ORDER *)sl->group_list.first ; order ; order=order->next)
-      order->item= (Item **)(order+1);
+    for (order= (ORDER *)sl->group_list.first; order; order= order->next)
+      order->item= &order->item_ptr;
     /* Fix ORDER list */
-    for (order=(ORDER *)sl->order_list.first ; order ; order=order->next)
-      order->item= (Item **)(order+1);
+    for (order= (ORDER *)sl->order_list.first; order; order= order->next)
+      order->item= &order->item_ptr;
 
     /*
       TODO: When the new table structure is ready, then have a status bit 
@@ -1131,6 +1133,8 @@ void mysql_stmt_execute(THD *thd, char *packet, uint packet_length)
   if (!(stmt= find_prepared_statement(thd, stmt_id, "execute", SEND_ERROR)))
     DBUG_VOID_RETURN;
 
+  DBUG_PRINT("equery:", ("%s", stmt->query));
+
   /* Check if we got an error when sending long data */
   if (stmt->get_longdata_error)
   {

sql/sql_select.cc

@@ -3536,7 +3536,9 @@ make_join_select(JOIN *join,SQL_SELECT *select,COND *cond)
 	  {
 	    /* Join with outer join condition */
 	    COND *orig_cond=sel->cond;
-	    sel->cond=and_conds(join->thd, sel->cond, tab->on_expr, 0);
+	    sel->cond= and_conds(sel->cond, tab->on_expr);
+	    if (sel->cond && !sel->cond->fixed)
+	      sel->cond->fix_fields(join->thd, 0, &sel->cond);
 	    if (sel->test_quick_select(join->thd, tab->keys,
 				       used_tables & ~ current_map,
 				       (join->select_options &

sql/sql_union.cc

@@ -208,19 +208,6 @@ int st_select_lex_unit::prepare(THD *thd_arg, select_result *sel_result,
     }
   }
 
-  for (tmp_order= (ORDER*) global_parameters->order_list.first;
-       tmp_order ;
-       tmp_order= tmp_order->next)
-  {
-    Item *item= *tmp_order->item;
-    if (((item->type() == Item::FIELD_ITEM) &&
-	 ((class Item_field*) item)->table_name))
-    {
-      my_error(ER_BAD_FIELD_ERROR,MYF(0),item->full_name(),"ORDER BY");
-      DBUG_RETURN(-1);
-    }
-  }
-
   item_list.empty();
   // it is not single select
   if (first_select->next_select())

sql/table.h

@@ -26,6 +26,7 @@ class st_select_lex_unit;
 typedef struct st_order {
   struct st_order *next;
   Item	 **item;			/* Point at item in select fields */
+  Item	 *item_ptr;			/* Storage for initial item */
   bool	 asc;				/* true if ascending */
   bool	 free_me;			/* true if item isn't shared  */
   bool	 in_field_list;			/* true if in select field list */

tests/client_test.c

@@ -8531,6 +8531,52 @@ static void test_on()
 }
 
 
+static void test_selecttmp()
+{
+  MYSQL_STMT *stmt;
+  int rc, i;
+  const char *query= "select a,(select count(distinct t1.b) as sum from t1,t2 where t1.a=t2.a and t2.b > 0 and t1.a <= t3.b group by t1.a order by sum limit 1) from t3";
+
+  myheader("test_select_tmp");
+  
+  rc = mysql_query(mysql, "DROP TABLE IF EXISTS t1,t2,t3");
+  myquery(rc);
+  
+  rc= mysql_query(mysql,"CREATE TABLE t1 (a int , b int);");
+  myquery(rc);
+
+  rc= mysql_query(mysql,"create table t2 (a int, b int);");
+  myquery(rc);
+
+  rc= mysql_query(mysql,"create table t3 (a int, b int);");
+  myquery(rc);
+
+  rc= mysql_query(mysql,
+		  "insert into t1 values (0,100),(1,2), (1,3), (2,2), (2,7), \
+(2,-1), (3,10);");
+  myquery(rc);
+  rc= mysql_query(mysql,
+		  "insert into t2 values (0,0), (1,1), (2,1), (3,1), (4,1);");
+  myquery(rc);
+  rc= mysql_query(mysql,
+		  "insert into t3 values (3,3), (2,2), (1,1);");
+  myquery(rc);
+
+  stmt= mysql_prepare(mysql, query, strlen(query));
+  mystmt_init(stmt);
+  for (i= 0; i < 3; i++)
+  {
+    rc= mysql_execute(stmt);
+    mystmt(stmt, rc);
+    assert(3 == my_process_stmt_result(stmt));
+  }
+  mysql_stmt_close(stmt);
+
+  rc= mysql_query(mysql, "DROP TABLE t1,t2,t3");
+  myquery(rc);
+}
+
+
 /*
   Read and parse arguments and MySQL options from my.cnf
 */
@@ -8791,6 +8837,7 @@ int main(int argc, char **argv)
     test_union();	    /* test union with prepared statements */
     test_bug3117();	    /* BUG#3117: LAST_INSERT_ID() */
     test_on();		    /* ... join ... on(), BUG#2794 */
+    test_selecttmp();	    /* temporary table used in select execution */
 
 
     end_time= time((time_t *)0);