A fix and a test case for Bug#11172 "mysql_stmt_attr_set

CURSOR_TYPE_READ_ONLY date/datetime filter server crash".
The fix adds support for Item_change_list in cursors (proper rollback
of the modified item tree). 


sql/sql_class.cc:
  No need to call fatal_error() twice.
sql/sql_prepare.cc:
  - implement proper cleanup of the prepared statement in mysql_stmt_reset
    if there is a cursor.
  - take into account thd->change_list when fetching data through a
    cursor.
sql/sql_select.cc:
  - take into account thd->change_list when fetching data from a cursor:
    grab it when we open a cursor, and rollback the changes to the parsed
    tree when we close it.
sql/sql_select.h:
  - Cursor::change_list added
tests/mysql_client_test.c:
  - a test case for Bug#11172 "mysql_stmt_attr_set CURSOR_TYPE_READ_ONLY date/datetime
   filter server crash"

4 files changed, 44 insertions, 21 deletions

diff --git a/sql/sql_class.cc b/sql/sql_class.cc
index 20f48da9283..0eac72b8fdc 100644
--- a/sql/sql_class.cc
+++ b/sql/sql_class.cc
@@ -784,7 +784,10 @@ void THD::nocheck_register_item_tree_change(Item **place, Item *old_value,
   void *change_mem= alloc_root(runtime_memroot, sizeof(*change));
   if (change_mem == 0)
   {
-    fatal_error();
+    /*
+      OOM, thd->fatal_error() is called by the error handler of the
+      memroot. Just return.
+    */
     return;
   }
   change= new (change_mem) Item_change_record;
diff --git a/sql/sql_prepare.cc b/sql/sql_prepare.cc
index c97cb037f15..7c9e0bc8a08 100644
--- a/sql/sql_prepare.cc
+++ b/sql/sql_prepare.cc
@@ -2203,13 +2203,15 @@ void mysql_stmt_fetch(THD *thd, char *packet, uint packet_length)
   ulong num_rows= uint4korr(packet+4);
   Prepared_statement *stmt;
   Statement stmt_backup;
+  Cursor *cursor;
   DBUG_ENTER("mysql_stmt_fetch");
 
   statistic_increment(thd->status_var.com_stmt_fetch, &LOCK_status);
   if (!(stmt= find_prepared_statement(thd, stmt_id, "mysql_stmt_fetch")))
     DBUG_VOID_RETURN;
 
-  if (!stmt->cursor || !stmt->cursor->is_open())
+  cursor= stmt->cursor;
+  if (!cursor || !cursor->is_open())
   {
     my_error(ER_STMT_HAS_NO_OPEN_CURSOR, MYF(0), stmt_id);
     DBUG_VOID_RETURN;
@@ -2222,22 +2224,27 @@ void mysql_stmt_fetch(THD *thd, char *packet, uint packet_length)
     my_pthread_setprio(pthread_self(), QUERY_PRIOR);
 
   thd->protocol= &thd->protocol_prep;           // Switch to binary protocol
-  stmt->cursor->fetch(num_rows);
+  cursor->fetch(num_rows);
   thd->protocol= &thd->protocol_simple;         // Use normal protocol
 
   if (!(specialflag & SPECIAL_NO_PRIOR))
     my_pthread_setprio(pthread_self(), WAIT_PRIOR);
 
-  thd->restore_backup_statement(stmt, &stmt_backup);
-  thd->current_arena= thd;
-
-  if (!stmt->cursor->is_open())
+  if (!cursor->is_open())
   {
     /* We're done with the fetch: reset PS for next execution */
     cleanup_stmt_and_thd_after_use(stmt, thd);
     reset_stmt_params(stmt);
+    /*
+      Must be the last, as some momory is still needed for
+      the previous calls.
+    */
+    free_root(cursor->mem_root, MYF(0));
   }
 
+  thd->restore_backup_statement(stmt, &stmt_backup);
+  thd->current_arena= thd;
+
   DBUG_VOID_RETURN;
 }
 
@@ -2264,14 +2271,21 @@ void mysql_stmt_reset(THD *thd, char *packet)
   /* There is always space for 4 bytes in buffer */
   ulong stmt_id= uint4korr(packet);
   Prepared_statement *stmt;
+  Cursor *cursor;
   DBUG_ENTER("mysql_stmt_reset");
 
   statistic_increment(thd->status_var.com_stmt_reset, &LOCK_status);
   if (!(stmt= find_prepared_statement(thd, stmt_id, "mysql_stmt_reset")))
     DBUG_VOID_RETURN;
 
-  if (stmt->cursor && stmt->cursor->is_open())
-    stmt->cursor->close();
+  cursor= stmt->cursor;
+  if (cursor && cursor->is_open())
+  {
+    thd->change_list= cursor->change_list;
+    cursor->close(FALSE);
+    cleanup_stmt_and_thd_after_use(stmt, thd);
+    free_root(cursor->mem_root, MYF(0));
+  }
 
   stmt->state= Query_arena::PREPARED;
 
diff --git a/sql/sql_select.cc b/sql/sql_select.cc
index cf8d7b1b83c..42b21a94794 100644
--- a/sql/sql_select.cc
+++ b/sql/sql_select.cc
@@ -1737,6 +1737,7 @@ Cursor::init_from_thd(THD *thd)
   lock=           thd->lock;
   query_id=       thd->query_id;
   free_list=	  thd->free_list;
+  change_list=    thd->change_list;
   reset_thd(thd);
   /*
     XXX: thd->locked_tables is not changed.
@@ -1753,6 +1754,7 @@ Cursor::reset_thd(THD *thd)
   thd->open_tables= 0;
   thd->lock= 0;
   thd->free_list= 0;
+  thd->change_list.empty();
 }
 
 
@@ -1826,6 +1828,7 @@ Cursor::fetch(ulong num_rows)
   thd->open_tables= open_tables;
   thd->lock= lock;
   thd->query_id= query_id;
+  thd->change_list= change_list;
   /* save references to memory, allocated during fetch */
   thd->set_n_backup_item_arena(this, &backup_arena);
 
@@ -1842,10 +1845,8 @@ Cursor::fetch(ulong num_rows)
 #ifdef USING_TRANSACTIONS
     ha_release_temporary_latches(thd);
 #endif
-
+  /* Grab free_list here to correctly free it in close */
   thd->restore_backup_item_arena(this, &backup_arena);
-  DBUG_ASSERT(thd->free_list == 0);
-  reset_thd(thd);
 
   if (error == NESTED_LOOP_CURSOR_LIMIT)
   {
@@ -1853,10 +1854,12 @@ Cursor::fetch(ulong num_rows)
     thd->server_status|= SERVER_STATUS_CURSOR_EXISTS;
     ::send_eof(thd);
     thd->server_status&= ~SERVER_STATUS_CURSOR_EXISTS;
+    change_list= thd->change_list;
+    reset_thd(thd);
   }
   else
   {
-    close();
+    close(TRUE);
     if (error == NESTED_LOOP_OK)
     {
       thd->server_status|= SERVER_STATUS_LAST_ROW_SENT;
@@ -1871,7 +1874,7 @@ Cursor::fetch(ulong num_rows)
 
 
 void
-Cursor::close()
+Cursor::close(bool is_active)
 {
   THD *thd= join->thd;
   DBUG_ENTER("Cursor::close");
@@ -1884,6 +1887,10 @@ Cursor::close()
     (void) unit->cleanup();
   else
     (void) join->select_lex->cleanup();
+
+  if (is_active)
+    close_thread_tables(thd);
+  else
   {
     /* XXX: Another hack: closing tables used in the cursor */
     DBUG_ASSERT(lock || open_tables || derived_tables);
@@ -1903,11 +1910,7 @@ Cursor::close()
   join= 0;
   unit= 0;
   free_items();
-  /*
-    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));
+  change_list.empty();
   DBUG_VOID_RETURN;
 }
 
@@ -1915,7 +1918,8 @@ Cursor::close()
 Cursor::~Cursor()
 {
   if (is_open())
-    close();
+    close(FALSE);
+  free_root(mem_root, MYF(0));
 }
 
 /*********************************************************************/
diff --git a/sql/sql_select.h b/sql/sql_select.h
index 6d33ae2f978..ac3e8898cc6 100644
--- a/sql/sql_select.h
+++ b/sql/sql_select.h
@@ -390,6 +390,7 @@ class Cursor: public Sql_alloc, public Query_arena
   /* List of items created during execution */
   query_id_t query_id;
 public:
+  Item_change_list change_list;
   select_send result;
 
   /* Temporary implementation as now we replace THD state by value */
@@ -402,7 +403,8 @@ public:
   void fetch(ulong num_rows);
   void reset() { join= 0; }
   bool is_open() const { return join != 0; }
-  void close();
+
+  void close(bool is_active);
 
   void set_unit(SELECT_LEX_UNIT *unit_arg) { unit= unit_arg; }
   Cursor(THD *thd);