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"

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;

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;
 

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));
 }
 
 /*********************************************************************/

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 @@ class Cursor: public Sql_alloc, public Query_arena
   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);

tests/mysql_client_test.c

@@ -13477,6 +13477,74 @@ static void test_bug10794()
 }
 
 
+/* Bug#11172: cursors, crash on a fetch from a datetime column */
+
+static void test_bug11172()
+{
+  MYSQL_STMT *stmt;
+  MYSQL_BIND bind_in[1], bind_out[2];
+  MYSQL_TIME hired;
+  int rc;
+  const char *stmt_text;
+  int i= 0, id;
+  ulong type;
+
+  myheader("test_bug11172");
+
+  mysql_query(mysql, "drop table if exists t1");
+  mysql_query(mysql, "create table t1 (id integer not null primary key,"
+                                      "hired date not null)");
+  rc= mysql_query(mysql,
+                  "insert into t1 (id, hired) values (1, '1933-08-24'), "
+                  "(2, '1965-01-01'), (3, '1949-08-17'), (4, '1945-07-07'), "
+                  "(5, '1941-05-15'), (6, '1978-09-15'), (7, '1936-03-28')");
+  myquery(rc);
+  stmt= mysql_stmt_init(mysql);
+  stmt_text= "SELECT id, hired FROM t1 WHERE hired=?";
+  rc= mysql_stmt_prepare(stmt, stmt_text, strlen(stmt_text));
+  check_execute(stmt, rc);
+
+  type= (ulong) CURSOR_TYPE_READ_ONLY;
+  mysql_stmt_attr_set(stmt, STMT_ATTR_CURSOR_TYPE, (const void*) &type);
+
+  bzero(bind_in, sizeof(bind_in));
+  bzero(bind_out, sizeof(bind_out));
+  bzero(&hired, sizeof(hired));
+  hired.year= 1965;
+  hired.month= 1;
+  hired.day= 1;
+  bind_in[0].buffer_type= MYSQL_TYPE_DATE;
+  bind_in[0].buffer= (void*) &hired;
+  bind_in[0].buffer_length= sizeof(hired);
+  bind_out[0].buffer_type= MYSQL_TYPE_LONG;
+  bind_out[0].buffer= (void*) &id;
+  bind_out[1]= bind_in[0];
+
+  for (i= 0; i < 3; i++)
+  {
+    rc= mysql_stmt_bind_param(stmt, bind_in);
+    check_execute(stmt, rc);
+    rc= mysql_stmt_bind_result(stmt, bind_out);
+    check_execute(stmt, rc);
+    rc= mysql_stmt_execute(stmt);
+    check_execute(stmt, rc);
+    while ((rc= mysql_stmt_fetch(stmt)) == 0)
+    {
+      if (!opt_silent)
+        printf("fetched data %d:%d-%d-%d\n", id,
+               hired.year, hired.month, hired.day);
+    }
+    DIE_UNLESS(rc == MYSQL_NO_DATA);
+    mysql_stmt_free_result(stmt) || mysql_stmt_reset(stmt);
+  }
+  mysql_stmt_close(stmt);
+  mysql_rollback(mysql);
+  mysql_rollback(mysql);
+
+  rc= mysql_query(mysql, "drop table t1");
+  myquery(rc);
+}
+
 /*
   Read and parse arguments and MySQL options from my.cnf
 */
@@ -13714,6 +13782,7 @@ static struct my_tests_st my_tests[]= {
   { "test_bug9992", test_bug9992 },
   { "test_bug10736", test_bug10736 },
   { "test_bug10794", test_bug10794 },
+  { "test_bug11172", test_bug11172 },
   { 0, 0 }
 };