A fix and test case for Bug#9478 "mysql_stmt_attr_set mysql_stmt_execute"

(crash on attempt to re-execute a statement with an open cursor) + 
post-review fixes.

include/errmsg.h

@@ -95,6 +95,7 @@ extern const char *client_errors[];	/* Error messages */
 #define CR_FETCH_CANCELED                       2050
 #define CR_NO_DATA                              2051
 #define CR_NO_STMT_METADATA                     2052
-#define CR_ERROR_LAST  /*Copy last error nr:*/  2052
+#define CR_NO_RESULT_SET                        2053
+#define CR_ERROR_LAST  /*Copy last error nr:*/  2053
 /* Add error numbers before CR_ERROR_LAST and change it accordingly. */
 

libmysql/errmsg.c

@@ -80,6 +80,7 @@ const char *client_errors[]=
   "Row retrieval was canceled by mysql_stmt_close() call",
   "Attempt to read column without prior row fetch",
   "Prepared statement contains no metadata",
+  "Attempt to read a row while there is no result set associated with the statement"
   ""
 };
 
@@ -141,6 +142,7 @@ const char *client_errors[]=
   "Row retrieval was canceled by mysql_stmt_close() call",
   "Attempt to read column without prior row fetch",
   "Prepared statement contains no metadata",
+  "Attempt to read a row while there is no result set associated with the statement"
   ""
 };
 
@@ -200,6 +202,7 @@ const char *client_errors[]=
   "Row retrieval was canceled by mysql_stmt_close() call",
   "Attempt to read column without prior row fetch",
   "Prepared statement contains no metadata",
+  "Attempt to read a row while there is no result set associated with the statement"
   ""
 };
 #endif

libmysql/libmysql.c

@@ -1724,6 +1724,13 @@ static int stmt_read_row_no_data(MYSQL_STMT *stmt, unsigned char **row);
 static void stmt_update_metadata(MYSQL_STMT *stmt, MYSQL_ROWS *data);
 static my_bool setup_one_fetch_function(MYSQL_BIND *bind, MYSQL_FIELD *field);
 
+/* Auxilary function used to reset statement handle. */
+
+#define RESET_SERVER_SIDE 1
+#define RESET_LONG_DATA 2
+
+static my_bool reset_stmt_handle(MYSQL_STMT *stmt, uint flags);
+
 /*
   Maximum sizes of MYSQL_TYPE_DATE, MYSQL_TYPE_TIME, MYSQL_TYPE_DATETIME
   values stored in network buffer.
@@ -2019,7 +2026,8 @@ mysql_stmt_prepare(MYSQL_STMT *stmt, const char *query, ulong length)
     /* This is second prepare with another statement */
     char buff[MYSQL_STMT_HEADER];               /* 4 bytes - stmt id */
 
-    mysql_stmt_free_result(stmt);
+    if (reset_stmt_handle(stmt, RESET_LONG_DATA))
+      DBUG_RETURN(1);
     /*
       These members must be reset for API to
       function in case of error or misuse.
@@ -2702,12 +2710,8 @@ static int
 stmt_read_row_no_data(MYSQL_STMT *stmt  __attribute__((unused)),
                       unsigned char **row  __attribute__((unused)))
 {
-  if ((int) stmt->state < (int) MYSQL_STMT_PREPARE_DONE)
-  {
-    set_stmt_error(stmt, CR_NO_PREPARE_STMT, unknown_sqlstate);
-    return 1;
-  }
-  return MYSQL_NO_DATA;
+  set_stmt_error(stmt, CR_NO_RESULT_SET, unknown_sqlstate);
+  return 1;
 }
 
 
@@ -2817,7 +2821,8 @@ int STDCALL mysql_stmt_execute(MYSQL_STMT *stmt)
     DBUG_RETURN(1);
   }
 
-  mysql_stmt_free_result(stmt);
+  if (reset_stmt_handle(stmt, 0))
+    DBUG_RETURN(1);
   /*
     No need to check for stmt->state: if the statement wasn't
     prepared we'll get 'unknown statement handler' error from server.
@@ -4805,16 +4810,21 @@ my_ulonglong STDCALL mysql_stmt_num_rows(MYSQL_STMT *stmt)
   DBUG_RETURN(stmt->result.rows);
 }
 
-my_bool STDCALL mysql_stmt_free_result(MYSQL_STMT *stmt)
-{
-  MYSQL_DATA *result= &stmt->result;
-  DBUG_ENTER("mysql_stmt_free_result");
 
-  DBUG_ASSERT(stmt != 0);
+/*
+  Free the client side memory buffers, reset long data state
+  on client if necessary, and reset the server side statement if
+  this has been requested.
+*/
 
+static my_bool reset_stmt_handle(MYSQL_STMT *stmt, uint flags)
+{
+  /* If statement hasn't been prepared there is nothing to reset */
   if ((int) stmt->state > (int) MYSQL_STMT_INIT_DONE)
   {
     MYSQL *mysql= stmt->mysql;
+    MYSQL_DATA *result= &stmt->result;
+    my_bool has_cursor= stmt->read_row_func == stmt_read_row_from_cursor;
 
     if (result->data)
     {
@@ -4824,23 +4834,58 @@ my_bool STDCALL mysql_stmt_free_result(MYSQL_STMT *stmt)
       result->rows= 0;
       stmt->data_cursor= NULL;
     }
-
-    if (mysql && stmt->field_count &&
-        (int) stmt->state > (int) MYSQL_STMT_PREPARE_DONE)
+    if (flags & RESET_LONG_DATA)
     {
-      if (mysql->unbuffered_fetch_owner == &stmt->unbuffered_fetch_cancelled)
-        mysql->unbuffered_fetch_owner= 0;
-      if (mysql->status != MYSQL_STATUS_READY)
+      MYSQL_BIND *param= stmt->params, *param_end= param + stmt->param_count;
+      /* Clear long_data_used flags */
+      for (; param < param_end; param++)
+        param->long_data_used= 0;
+    }
+    stmt->read_row_func= stmt_read_row_no_data;
+    if (mysql)
+    {
+      if ((int) stmt->state > (int) MYSQL_STMT_PREPARE_DONE)
       {
-        /* There is a result set and it belongs to this statement */
-        (*mysql->methods->flush_use_result)(mysql);
-        mysql->status= MYSQL_STATUS_READY;
+        if (mysql->unbuffered_fetch_owner == &stmt->unbuffered_fetch_cancelled)
+          mysql->unbuffered_fetch_owner= 0;
+        if (stmt->field_count && mysql->status != MYSQL_STATUS_READY)
+        {
+          /* There is a result set and it belongs to this statement */
+          (*mysql->methods->flush_use_result)(mysql);
+          if (mysql->unbuffered_fetch_owner)
+            *mysql->unbuffered_fetch_owner= TRUE;
+          mysql->status= MYSQL_STATUS_READY;
+        }
+      }
+      if (has_cursor || (flags & RESET_SERVER_SIDE))
+      {
+        /*
+          Reset the server side statement and close the server side
+          cursor if it exists.
+        */
+        char buff[MYSQL_STMT_HEADER]; /* packet header: 4 bytes for stmt id */
+        int4store(buff, stmt->stmt_id);
+        if ((*mysql->methods->advanced_command)(mysql, COM_RESET_STMT, buff,
+                                                sizeof(buff), 0, 0, 0))
+        {
+          set_stmt_errmsg(stmt, mysql->net.last_error, mysql->net.last_errno,
+                          mysql->net.sqlstate);
+          stmt->state= MYSQL_STMT_INIT_DONE;
+          return 1;
+        }
       }
     }
     stmt->state= MYSQL_STMT_PREPARE_DONE;
-    stmt->read_row_func= stmt_read_row_no_data;
   }
-  DBUG_RETURN(0);
+  return 0;
+}
+
+my_bool STDCALL mysql_stmt_free_result(MYSQL_STMT *stmt)
+{
+  DBUG_ENTER("mysql_stmt_free_result");
+
+  /* Free the client side and close the server side cursor if there is one */
+  DBUG_RETURN(reset_stmt_handle(stmt, RESET_LONG_DATA));
 }
 
 /********************************************************************
@@ -4913,33 +4958,10 @@ my_bool STDCALL mysql_stmt_close(MYSQL_STMT *stmt)
 
 my_bool STDCALL mysql_stmt_reset(MYSQL_STMT *stmt)
 {
-  char buff[MYSQL_STMT_HEADER]; /* packet header: 4 bytes for stmt id */
-  MYSQL *mysql;
-  MYSQL_BIND *param, *param_end;
   DBUG_ENTER("mysql_stmt_reset");
   DBUG_ASSERT(stmt != 0);
-
-  /* If statement hasnt been prepared there is nothing to reset */
-  if ((int) stmt->state < (int) MYSQL_STMT_PREPARE_DONE)
-    DBUG_RETURN(0);
-
-  mysql= stmt->mysql->last_used_con;
-  int4store(buff, stmt->stmt_id);		/* Send stmt id to server */
-  if ((*mysql->methods->advanced_command)(mysql, COM_RESET_STMT, buff,
-                                          sizeof(buff), 0, 0, 0))
-  {
-    set_stmt_errmsg(stmt, mysql->net.last_error, mysql->net.last_errno,
-                    mysql->net.sqlstate);
-    DBUG_RETURN(1);
-  }
-
-  /* Clear long_data_used for next call (as we do in mysql_stmt_execute() */
-  for (param= stmt->params, param_end= param + stmt->param_count;
-       param < param_end;
-       param++)
-    param->long_data_used= 0;
-
-  DBUG_RETURN(0);
+  /* Reset the client and server sides of the prepared statement */
+  DBUG_RETURN(reset_stmt_handle(stmt, RESET_SERVER_SIDE | RESET_LONG_DATA));
 }
 
 /*

sql-common/client.c

@@ -864,6 +864,8 @@ mysql_free_result(MYSQL_RES *result)
       {
         (*mysql->methods->flush_use_result)(mysql);
         mysql->status=MYSQL_STATUS_READY;
+        if (mysql->unbuffered_fetch_owner)
+          *mysql->unbuffered_fetch_owner= TRUE;
       }
     }
     free_rows(result->data);

sql/share/errmsg.txt

@@ -4766,13 +4766,13 @@ ER_SUBQUERY_NO_1_ROW 21000
 	swe "Subquery returnerade mer n 1 rad"
 	ukr "  ¦ i 1 "
 ER_UNKNOWN_STMT_HANDLER  
-	dan "Unknown prepared statement handler (%ld) given to %s"
+	dan "Unknown prepared statement handler (%.*s) given to %s"
 	eng "Unknown prepared statement handler (%.*s) given to %s"
 	ger "Unbekannter Prepared-Statement-Handler (%.*s) fr %s angegeben"
 	por "Desconhecido manipulador de declarao preparado (%.*s) determinado para %s"
-	spa "Desconocido preparado comando handler (%ld) dado para %s"
-	swe "Oknd PREPARED STATEMENT id (%ld) var given till %s"
-	ukr "Unknown prepared statement handler (%ld) given to %s"
+	spa "Desconocido preparado comando handler (%.*s) dado para %s"
+	swe "Oknd PREPARED STATEMENT id (%.*s) var given till %s"
+	ukr "Unknown prepared statement handler (%.*s) given to %s"
 ER_CORRUPT_HELP_DB  
 	eng "Help database is corrupt or does not exist"
 	ger "Die Hilfe-Datenbank ist beschdigt oder existiert nicht"
@@ -5352,3 +5352,7 @@ ER_BINLOG_UNSAFE_ROUTINE
 	eng "This routine is declared to be non-deterministic and to modify data and binary logging is enabled (you *might* want to use the less safe log_bin_trust_routine_creators variable)"
 ER_BINLOG_CREATE_ROUTINE_NEED_SUPER
 	eng "You do not have SUPER privilege and binary logging is enabled (you *might* want to use the less safe log_bin_trust_routine_creators variable)"
+ER_EXEC_STMT_WITH_OPEN_CURSOR
+	eng "You can't execute a prepared statement which has an open cursor associated with it. Reset the statement to re-execute it."
+ER_STMT_HAS_NO_OPEN_CURSOR
+	eng "The statement (%d) has no open cursor."

sql/sql_prepare.cc

@@ -135,7 +135,8 @@ find_prepared_statement(THD *thd, ulong id, const char *where)
   if (stmt == 0 || stmt->type() != Item_arena::PREPARED_STATEMENT)
   {
     char llbuf[22];
-    my_error(ER_UNKNOWN_STMT_HANDLER, MYF(0), 22, llstr(id, llbuf), where);
+    my_error(ER_UNKNOWN_STMT_HANDLER, MYF(0), sizeof(llbuf), llstr(id, llbuf),
+             where);
     return 0;
   }
   return (Prepared_statement *) stmt;
@@ -1969,7 +1970,7 @@ void mysql_stmt_execute(THD *thd, char *packet, uint packet_length)
 {
   ulong stmt_id= uint4korr(packet);
   ulong flags= (ulong) ((uchar) packet[4]);
-  Cursor *cursor= 0;
+  Cursor *cursor;
   /*
     Query text for binary log, or empty string if the query is not put into
     binary log.
@@ -1995,6 +1996,13 @@ void mysql_stmt_execute(THD *thd, char *packet, uint packet_length)
     DBUG_VOID_RETURN;
   }
 
+  cursor= stmt->cursor;
+  if (cursor && cursor->is_open())
+  {
+    my_error(ER_EXEC_STMT_WITH_OPEN_CURSOR, MYF(0));
+    DBUG_VOID_RETURN;
+  }
+
   DBUG_ASSERT(thd->free_list == NULL);
   mysql_reset_thd_for_next_command(thd);
   if (flags & (ulong) CURSOR_TYPE_READ_ONLY)
@@ -2013,7 +2021,7 @@ void mysql_stmt_execute(THD *thd, char *packet, uint packet_length)
     else
     {
       DBUG_PRINT("info",("Using READ_ONLY cursor"));
-      if (!stmt->cursor &&
+      if (!cursor &&
           !(cursor= stmt->cursor= new (&stmt->main_mem_root) Cursor()))
         DBUG_VOID_RETURN;
       /* If lex->result is set, mysql_execute_command will use it */
@@ -2208,13 +2216,15 @@ void mysql_stmt_fetch(THD *thd, char *packet, uint packet_length)
   Statement *stmt;
   DBUG_ENTER("mysql_stmt_fetch");
 
-  if (!(stmt= thd->stmt_map.find(stmt_id)) ||
-      !stmt->cursor ||
-      !stmt->cursor->is_open())
+  if (!(stmt= find_prepared_statement(thd, stmt_id, "mysql_stmt_fetch")))
+    DBUG_VOID_RETURN;
+
+  if (!stmt->cursor || !stmt->cursor->is_open())
   {
-    my_error(ER_UNKNOWN_STMT_HANDLER, MYF(0), stmt_id, "fetch");
+    my_error(ER_STMT_HAS_NO_OPEN_CURSOR, MYF(0));
     DBUG_VOID_RETURN;
   }
+
   thd->current_arena= stmt;
   thd->set_n_backup_statement(stmt, &thd->stmt_backup);
   stmt->cursor->init_thd(thd);
@@ -2266,6 +2276,9 @@ void mysql_stmt_reset(THD *thd, char *packet)
   if (!(stmt= find_prepared_statement(thd, stmt_id, "mysql_stmt_reset")))
     DBUG_VOID_RETURN;
 
+  if (stmt->cursor && stmt->cursor->is_open())
+    stmt->cursor->close();
+
   stmt->state= Item_arena::PREPARED;
 
   /* 

sql/sql_select.cc

@@ -1742,6 +1742,7 @@ Cursor::init_from_thd(THD *thd)
   /*
     XXX: thd->locked_tables is not changed.
     What problems can we have with it if cursor is open?
+    TODO: must be fixed because of the prelocked mode.
   */
   /*
     TODO: grab thd->free_list here?
@@ -1871,10 +1872,6 @@ Cursor::fetch(ulong num_rows)
     }
     else if (error != NESTED_LOOP_KILLED)
       my_message(ER_OUT_OF_RESOURCES, ER(ER_OUT_OF_RESOURCES), MYF(0));
-    /* free cursor memory */
-    free_items(free_list);
-    free_list= 0;
-    free_root(&main_mem_root, MYF(0));
   }
 }
 
@@ -1914,6 +1911,13 @@ Cursor::close()
   }
   join= 0;
   unit= 0;
+  free_items(free_list);
+  free_list= 0;
+  /*
+    Must be last, as some memory might be allocated for free purposes,
+    like in free_tmp_table() (TODO: fix this issue)
+  */
+  free_root(mem_root, MYF(0));
   DBUG_VOID_RETURN;
 }
 
@@ -1922,12 +1926,6 @@ Cursor::~Cursor()
 {
   if (is_open())
     close();
-  free_items(free_list);
-  /*
-    Must be last, as some memory might be allocated for free purposes,
-    like in free_tmp_table() (TODO: fix this issue)
-  */
-  free_root(&main_mem_root, MYF(0));
 }
 
 /*********************************************************************/

sql/sql_select.h

@@ -364,6 +364,10 @@ class JOIN :public Sql_alloc
 /*
   Server-side cursor (now stands only for basic read-only cursor)
   See class implementation in sql_select.cc
+  A cursor has its own runtime state - list of used items and memory root of
+  used memory - which is different from Prepared statement runtime: it must
+  be different at least for the purpose of reusing the same prepared
+  statement for many cursors.
 */
 
 class Cursor: public Sql_alloc, public Item_arena