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.

4 files changed, 40 insertions, 21 deletions

diff --git a/sql/share/errmsg.txt b/sql/share/errmsg.txt
index 388f47bf96f..848cc422dc2 100644
--- a/sql/share/errmsg.txt
+++ b/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) für %s angegeben"
 	por "Desconhecido manipulador de declaração preparado (%.*s) determinado para %s"
-	spa "Desconocido preparado comando handler (%ld) dado para %s"
-	swe "Okänd 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 "Okänd 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 beschädigt 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."
diff --git a/sql/sql_prepare.cc b/sql/sql_prepare.cc
index 17c5f51f1e1..03b3df1bc22 100644
--- a/sql/sql_prepare.cc
+++ b/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;
 
   /* 
diff --git a/sql/sql_select.cc b/sql/sql_select.cc
index 47c7de6eba7..84f3bb92a7d 100644
--- a/sql/sql_select.cc
+++ b/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));
 }
 
 /*********************************************************************/
diff --git a/sql/sql_select.h b/sql/sql_select.h
index a27fbc60856..49ac58e913b 100644
--- a/sql/sql_select.h
+++ b/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