Fix for Bug#5034 "prepared "select 1 into @arg15", second

execute crashes server": we were deleting lex->result
after each execute, but prepared statements assumed that
it's left intact.
The fix adds cleanup() method to select_result hierarchy,
so that result objects can be reused.
Plus we now need to delete result objects more wisely.


mysql-test/r/ps.result:
  Test results fixed: test case for bug#5034
mysql-test/t/ps.test:
  A test case for bug#5034, few followups
sql/sql_class.cc:
  - fix warning in THD::THD
  - implementation of cleanup() for select_result hierarchy
  - select_export::send_eof was identical to 
    select_dump::send_eof: moved to the base class select_to_file.
  - Statement::end_statement() to end lex, free items, and
    delete possible select_result
sql/sql_class.h:
  - select_result::cleanup() declaration
  -
sql/sql_insert.cc:
  - implementation of select_insert::cleanup(): currently
    we always create a new instance of select_insert/
    select_create on each execute.
sql/sql_lex.cc:
  - with more complicated logic of freeing lex->result it's 
    easier to have it non-zero only if it points to a valid
    result.
sql/sql_lex.h:
  Now st_lex::st_lex is not empty.
sql/sql_parse.cc:
  mysql_execute_command():
  - delete select_result *result only if it was created in
    this function.
  - use end_statement() to cleanup lex and thd in the end of
    each statement.
  - no need to save THD::lock if this is explain. This save
    apparently left from times when derived tables were 
    materialized here, not in open_and_lock_tables.
sql/sql_prepare.cc:
  - call result->cleanup() in reset_stmt_for_execute
  - now Statement is responsible for freeing its lex->result.
sql/sql_select.cc:
  handle_select():
  - don't delete result, it might be needed
    for next executions
  - result is never null

mysql-test/r/ps.result

@@ -226,3 +226,18 @@ a	b
 execute stmt1;
 a	b
 deallocate prepare stmt1;
+drop table t1;
+prepare stmt1 from "select 1 into @var";
+execute stmt1;
+execute stmt1;
+prepare stmt1 from "create table t1 select 1 as i";
+execute stmt1;
+drop table t1;
+execute stmt1;
+prepare stmt1 from "insert into t1 select i from t1";
+execute stmt1;
+execute stmt1;
+prepare stmt1 from "select * from t1 into outfile 'f1.txt'";
+execute stmt1;
+deallocate prepare stmt1;
+drop table t1;

mysql-test/t/ps.test

@@ -209,7 +209,7 @@ drop table t1;
 
 #
 # Bug#4912 "mysqld crashs in case a statement is executed a second time":
-# negation elimination should and prepared statemens
+# negation elimination should work once and not break prepared statements
 # 
 
 create table t1(a varchar(2), b varchar(3));
@@ -217,4 +217,26 @@ prepare stmt1 from "select a, b from t1 where (not (a='aa' and b < 'zzz'))";
 execute stmt1;
 execute stmt1;
 deallocate prepare stmt1;
+drop table t1;
+
+#
+# Bug#5034 "prepared "select 1 into @arg15", second execute crashes
+# server".
+# Check that descendands of select_result can be reused in prepared 
+# statements or are correctly created and deleted on each execute
+#
 
+prepare stmt1 from "select 1 into @var";
+execute stmt1;
+execute stmt1;
+prepare stmt1 from "create table t1 select 1 as i";
+execute stmt1;
+drop table t1;
+execute stmt1;
+prepare stmt1 from "insert into t1 select i from t1";
+execute stmt1;
+execute stmt1;
+prepare stmt1 from "select * from t1 into outfile 'f1.txt'";
+execute stmt1;
+deallocate prepare stmt1;
+drop table t1;

sql/sql_class.cc

@@ -155,10 +155,10 @@ bool foreign_key_prefix(Key *a, Key *b)
 ** Thread specific functions
 ****************************************************************************/
 
-THD::THD():user_time(0), current_arena(this), is_fatal_error(0),
-	   last_insert_id_used(0),
+THD::THD():user_time(0), current_arena(this), global_read_lock(0),
+           is_fatal_error(0), last_insert_id_used(0),
            insert_id_used(0), rand_used(0), time_zone_used(0),
-           in_lock_tables(0), global_read_lock(0), bootstrap(0)
+           in_lock_tables(0), bootstrap(0)
 {
   host= user= priv_user= db= ip=0;
   host_or_ip= "connecting host";
@@ -703,6 +703,12 @@ void select_result::send_error(uint errcode,const char *err)
   ::send_error(thd, errcode, err);
 }
 
+
+void select_result::cleanup()
+{
+  /* do nothing */
+}
+
 static String default_line_term("\n",default_charset_info);
 static String default_escaped("\\",default_charset_info);
 static String default_field_term("\t",default_charset_info);
@@ -808,6 +814,32 @@ void select_to_file::send_error(uint errcode,const char *err)
 }
 
 
+bool select_to_file::send_eof()
+{
+  int error= test(end_io_cache(&cache));
+  if (my_close(file,MYF(MY_WME)))
+    error= 1;
+  if (!error)
+    ::send_ok(thd,row_count);
+  file= -1;
+  return error;
+}
+
+
+void select_to_file::cleanup()
+{
+  /* In case of error send_eof() may be not called: close the file here. */
+  if (file >= 0)
+  {
+    (void) end_io_cache(&cache);
+    (void) my_close(file,MYF(0));
+    file= -1;
+  }
+  path[0]= '\0';
+  row_count= 0;
+}
+
+
 select_to_file::~select_to_file()
 {
   if (file >= 0)
@@ -1058,18 +1090,6 @@ bool select_export::send_data(List<Item> &items)
 }
 
 
-bool select_export::send_eof()
-{
-  int error=test(end_io_cache(&cache));
-  if (my_close(file,MYF(MY_WME)))
-    error=1;
-  if (!error)
-    ::send_ok(thd,row_count);
-  file= -1;
-  return error;
-}
-
-
 /***************************************************************************
 ** Dump  of select to a binary file
 ***************************************************************************/
@@ -1123,18 +1143,6 @@ bool select_dump::send_data(List<Item> &items)
 }
 
 
-bool select_dump::send_eof()
-{
-  int error=test(end_io_cache(&cache));
-  if (my_close(file,MYF(MY_WME)))
-    error=1;
-  if (!error)
-    ::send_ok(thd,row_count);
-  file= -1;
-  return error;
-}
-
-
 select_subselect::select_subselect(Item_subselect *item_arg)
 {
   item= item_arg;
@@ -1301,6 +1309,13 @@ int select_dumpvar::prepare(List<Item> &list, SELECT_LEX_UNIT *u)
 }
 
 
+void select_dumpvar::cleanup()
+{
+  vars.empty();
+  row_count=0;
+}
+
+
 Item_arena::Item_arena(THD* thd)
   :free_list(0),
   state(INITIALIZED)
@@ -1405,6 +1420,21 @@ void Statement::restore_backup_statement(Statement *stmt, Statement *backup)
 }
 
 
+void Statement::end_statement()
+{
+  /* Cleanup SQL processing state to resuse this statement in next query. */
+  lex_end(lex);
+  delete lex->result;
+  lex->result= 0;
+  free_items(free_list);
+  free_list= 0;
+  /*
+    Don't free mem_root, as mem_root is freed in the end of dispatch_command
+    (once for any command).
+  */
+}
+
+
 void Item_arena::set_n_backup_item_arena(Item_arena *set, Item_arena *backup)
 {
   backup->set_item_arena(this);

sql/sql_class.h

@@ -547,6 +547,12 @@ class Statement: public Item_arena
   void restore_backup_statement(Statement *stmt, Statement *backup);
   /* return class type */
   virtual Type type() const;
+
+  /*
+    Cleanup statement parse state (parse tree, lex) after execution of
+    a non-prepared SQL statement.
+  */
+  void end_statement();
 };
 
 
@@ -1029,10 +1035,13 @@ class Disable_binlog {
   ~Disable_binlog();
 };
 
+
 /*
   Used to hold information about file and file structure in exchainge 
   via non-DB file (...INTO OUTFILE..., ...LOAD DATA...)
+  XXX: We never call destructor for objects of this class.
 */
+
 class sql_exchange :public Sql_alloc
 {
 public:
@@ -1042,7 +1051,6 @@ class sql_exchange :public Sql_alloc
   bool dumpfile;
   ulong skip_lines;
   sql_exchange(char *name,bool dumpfile_flag);
-  ~sql_exchange() {}
 };
 
 #include "log_event.h"
@@ -1073,6 +1081,11 @@ class select_result :public Sql_alloc {
   virtual void send_error(uint errcode,const char *err);
   virtual bool send_eof()=0;
   virtual void abort() {}
+  /*
+    Cleanup instance of this class for next execution of a prepared
+    statement/stored procedure.
+  */
+  virtual void cleanup();
 };
 
 
@@ -1099,6 +1112,8 @@ class select_to_file :public select_result {
   ~select_to_file();
   bool send_fields(List<Item> &list, uint flag) { return 0; }
   void send_error(uint errcode,const char *err);
+  bool send_eof();
+  void cleanup();
 };
 
 
@@ -1111,7 +1126,6 @@ class select_export :public select_to_file {
   ~select_export();
   int prepare(List<Item> &list, SELECT_LEX_UNIT *u);
   bool send_data(List<Item> &items);
-  bool send_eof();
 };
 
 
@@ -1120,7 +1134,6 @@ class select_dump :public select_to_file {
   select_dump(sql_exchange *ex) :select_to_file(ex) {}
   int prepare(List<Item> &list, SELECT_LEX_UNIT *u);
   bool send_data(List<Item> &items);
-  bool send_eof();
 };
 
 
@@ -1145,6 +1158,8 @@ class select_insert :public select_result {
   bool send_data(List<Item> &items);
   void send_error(uint errcode,const char *err);
   bool send_eof();
+  /* not implemented: select_insert is never re-used in prepared statements */
+  void cleanup();
 };
 
 
@@ -1445,4 +1460,5 @@ class select_dumpvar :public select_result {
   bool send_fields(List<Item> &list, uint flag) {return 0;}
   bool send_data(List<Item> &items);
   bool send_eof();
+  void cleanup();
 };

sql/sql_insert.cc

@@ -1465,6 +1465,13 @@ select_insert::prepare(List<Item> &values, SELECT_LEX_UNIT *u)
   DBUG_RETURN(0);
 }
 
+
+void select_insert::cleanup()
+{
+  /* select_insert/select_create are never re-used in prepared statement */
+  DBUG_ASSERT(0);
+}
+
 select_insert::~select_insert()
 {
   if (table)

sql/sql_lex.cc

@@ -1653,6 +1653,11 @@ void st_select_lex::print_limit(THD *thd, String *str)
 }
 
 
+st_lex::st_lex()
+  :result(0)
+{}
+
+
 /*
   Unlink first table from global table list and first table from outer select
   list (lex->select_lex)

sql/sql_lex.h

@@ -639,7 +639,7 @@ typedef struct st_lex
     list of those tables after they are opened.
   */
   TABLE_LIST *time_zone_tables_used;
-  st_lex() {}
+  st_lex();
   inline void uncacheable(uint8 cause)
   {
     safe_to_cache_query= 0;

sql/sql_parse.cc

@@ -1967,8 +1967,6 @@ mysql_execute_command(THD *thd)
 	else
 	  thd->send_explain_fields(result);
 	res= mysql_explain_union(thd, &thd->lex->unit, result);
-	MYSQL_LOCK *save_lock= thd->lock;
-	thd->lock= (MYSQL_LOCK *)0;
 	if (lex->describe & DESCRIBE_EXTENDED)
 	{
 	  char buff[1024];
@@ -1980,20 +1978,19 @@ mysql_execute_command(THD *thd)
 		       ER_YES, str.ptr());
 	}
 	result->send_eof();
-	thd->lock= save_lock;
+        delete result;
       }
       else
       {
-	if (!result)
-	{
-	  if (!(result=new select_send()))
+	if (!result && !(result= new select_send()))
 	{
 	  res= -1;
 	  break;
 	}
-	}
 	query_cache_store_query(thd, tables);
-	res=handle_select(thd, lex, result);
+	res= handle_select(thd, lex, result);
+        if (result != lex->result)
+          delete result;
       }
     }
     break;
@@ -2708,10 +2705,10 @@ mysql_execute_command(THD *thd)
     }
 
 
-    if (!(res=open_and_lock_tables(thd, tables)))
-    {
-      if ((result=new select_insert(tables->table,&lex->field_list,
+    if (!(res= open_and_lock_tables(thd, tables)) &&
+        (result= new select_insert(tables->table, &lex->field_list,
                                    lex->duplicates)))
+    {
       /* Skip first table, which is the table we are inserting in */
       lex->select_lex.table_list.first= (byte*) first_local_table->next;
       /*
@@ -2719,10 +2716,11 @@ mysql_execute_command(THD *thd)
         and item_list belong to SELECT
       */
       lex->select_lex.resolve_mode= SELECT_LEX::SELECT_MODE;
-	res=handle_select(thd,lex,result);
+      res= handle_select(thd, lex, result);
       /* revert changes for SP */
       lex->select_lex.table_list.first= (byte*) first_local_table;
       lex->select_lex.resolve_mode= SELECT_LEX::INSERT_MODE;
+      delete result;
       if (thd->net.report_error)
         res= -1;
     }
@@ -3904,8 +3902,8 @@ mysql_init_select(LEX *lex)
   select_lex->select_limit= HA_POS_ERROR;
   if (select_lex == &lex->select_lex)
   {
+    DBUG_ASSERT(lex->result == 0);
     lex->exchange= 0;
-    lex->result= 0;
     lex->proc_list.first= 0;
   }
 }
@@ -4047,9 +4045,7 @@ void mysql_parse(THD *thd, char *inBuf, uint length)
       query_cache_abort(&thd->net);
     }
     thd->proc_info="freeing items";
-    free_items(thd->free_list);  /* Free strings used by items */
-    thd->free_list= 0;
-    lex_end(lex);
+    thd->end_statement();
   }
   DBUG_VOID_RETURN;
 }
@@ -4074,10 +4070,7 @@ bool mysql_test_parse_for_slave(THD *thd, char *inBuf, uint length)
   if (!yyparse((void*) thd) && ! thd->is_fatal_error &&
       all_tables_not_ok(thd,(TABLE_LIST*) lex->select_lex.table_list.first))
     error= 1;                /* Ignore question */
-  free_items(thd->free_list);  /* Free strings used by items */
-  thd->free_list= 0;
-  lex_end(lex);
-
+  thd->end_statement();
   return error;
 }
 #endif

sql/sql_prepare.cc

@@ -1630,7 +1630,8 @@ int mysql_stmt_prepare(THD *thd, char *packet, uint packet_length,
 static void reset_stmt_for_execute(Prepared_statement *stmt)
 {
   THD *thd= stmt->thd;
-  SELECT_LEX *sl= stmt->lex->all_selects_list;
+  LEX *lex= stmt->lex;
+  SELECT_LEX *sl= lex->all_selects_list;
 
   for (; sl; sl= sl->next_select_in_list())
   {
@@ -1678,7 +1679,9 @@ static void reset_stmt_for_execute(Prepared_statement *stmt)
       unit->reinit_exec_mechanism();
     }
   }
-  stmt->lex->current_select= &stmt->lex->select_lex;
+  lex->current_select= &lex->select_lex;
+  if (lex->result)
+    lex->result->cleanup();
 }
 
 
@@ -2053,6 +2056,7 @@ void Prepared_statement::setup_set_params()
 Prepared_statement::~Prepared_statement()
 {
   free_items(free_list);
+  delete lex->result;
 }
 
 

sql/sql_select.cc

@@ -198,17 +198,11 @@ int handle_select(THD *thd, LEX *lex, select_result *result)
   if (thd->net.report_error)
     res= 1;
   if (res)
-  {
-    if (result)
   {
     result->send_error(0, NullS);
     result->abort();
-    }
-    else
-      send_error(thd, 0, NullS);
     res= 1;					// Error sent to client
   }
-  delete result;
   DBUG_RETURN(res);
 }