From 0efa6f86be8e7e29be6a85d0935ffbae69df7057 Mon Sep 17 00:00:00 2001 From: "konstantin@mysql.com" <> Date: Fri, 1 Jul 2005 15:47:45 +0400 Subject: [PATCH] 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 | 5 ++- sql/sql_prepare.cc | 30 ++++++++++++----- sql/sql_select.cc | 26 ++++++++------- sql/sql_select.h | 4 ++- tests/mysql_client_test.c | 69 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 113 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); diff --git a/tests/mysql_client_test.c b/tests/mysql_client_test.c index fa70168d0ef..e57d00e5a91 100644 --- a/tests/mysql_client_test.c +++ b/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 } };