From: Date: October 7 2008 12:48am Subject: bzr commit into mysql-5.0-bugteam branch (gshchepa:2689) Bug#38691 List-Archive: http://lists.mysql.com/commits/55494 X-Bug: 38691 Message-Id: <20081006225040.AE4DD40CD42@localhost.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7BIT #At file:///work/bzr/5.0-5.1.29-rc-38691/ 2689 Gleb Shchepa 2008-10-07 Bug #38691: segfault/abort in ``UPDATE ...JOIN'' while ``FLUSH TABLES WITH READ LOCK'' Concurrent execution of 1) multitable update with a NATURAL/USING join and 2) a such query as "FLUSH TABLES WITH READ LOCK" or "ALTER TABLE" of updating table led to a server crash. The mysql_multi_update_prepare() function call is optimized to lock updating tables only, so it postpones locking to the last, and if locking fails, it does cleanup of modified syntax structures and repeats a query analysis. However, that cleanup procedure was incomplete for NATURAL/USING join syntax data: 1) some Field_item items pointed into freed table structures, and 2) the TABLE_LIST::join_columns fields was not reset. Major change: short-living Field *Natural_join_column::table_field has been replaced with long-living Item*. modified: mysql-test/r/lock_multi.result mysql-test/t/lock_multi.test sql/item.cc sql/sql_base.cc sql/sql_class.cc sql/sql_class.h sql/sql_update.cc sql/table.cc sql/table.h per-file messages: mysql-test/r/lock_multi.result Added test case for bug #38691. mysql-test/t/lock_multi.test Added test case for bug #38691. sql/item.cc Bug #38691: segfault/abort in ``UPDATE ...JOIN'' while ``FLUSH TABLES WITH READ LOCK'' The Item_field constructor has been modified to allocate and copy original database/table/field names always (not during PS preparation/1st execution only), because an initialization of Item_field items with a pointer to short-living Field structures is a common practice. sql/sql_base.cc Bug #38691: segfault/abort in ``UPDATE ...JOIN'' while ``FLUSH TABLES WITH READ LOCK'' 1) Type adjustment for Natural_join_column::table_field (Field to Item_field); 2) The setup_natural_join_row_types function has been updated to take into account new thd->multiupdate_reopen flag to skip unnecessary reinitialization of Natural_join_column::join_columns during table reopening after lock_tables() failure (like the 'first_execution' flag for PS). sql/sql_class.cc Bug #38691: segfault/abort in ``UPDATE ...JOIN'' while ``FLUSH TABLES WITH READ LOCK'' Initialization of new THD::multiupdate_reopen has been added. sql/sql_class.h Bug #38691: segfault/abort in ``UPDATE ...JOIN'' while ``FLUSH TABLES WITH READ LOCK'' The THD::multiupdate_reopen flag has been added to mark table reopening state for multitable update. sql/sql_update.cc Bug #38691: segfault/abort in ``UPDATE ...JOIN'' while ``FLUSH TABLES WITH READ LOCK'' 1) Extra cleanup calls have been added to reset Natural_join_column::table_field items. 2) Setting/resetting on the THD::multiupdate_reopen flag has been added. sql/table.cc Bug #38691: segfault/abort in ``UPDATE ...JOIN'' while ``FLUSH TABLES WITH READ LOCK'' Type adjustment for Natural_join_column::table_field (Field to Item_field); sql/table.h Bug #38691: segfault/abort in ``UPDATE ...JOIN'' while ``FLUSH TABLES WITH READ LOCK'' Type of the Natural_join_column::table_field field has been changed from Field that points into short-living TABLE memory to long-living Item_field that can be linked to (fixed) reopened table. === modified file 'mysql-test/r/lock_multi.result' --- a/mysql-test/r/lock_multi.result 2007-11-28 12:18:01 +0000 +++ b/mysql-test/r/lock_multi.result 2008-10-06 22:48:46 +0000 @@ -99,3 +99,13 @@ kill query ERROR 70100: Query execution was interrupted unlock tables; drop table t1; +CREATE TABLE t1 ( +a int(11) unsigned default NULL, +b varchar(255) default NULL, +UNIQUE KEY a (a), +KEY b (b) +); +INSERT INTO t1 VALUES (1, 1), (2, 2), (3, 3); +CREATE TABLE t2 SELECT * FROM t1; +CREATE TABLE t3 SELECT * FROM t1; +DROP TABLE t1, t2, t3; === modified file 'mysql-test/t/lock_multi.test' --- a/mysql-test/t/lock_multi.test 2007-11-28 12:18:01 +0000 +++ b/mysql-test/t/lock_multi.test 2008-10-06 22:48:46 +0000 @@ -281,4 +281,63 @@ unlock tables; connection default; drop table t1; +# +# Bug #38691: segfault/abort in ``UPDATE ...JOIN'' while +# ``FLUSH TABLES WITH READ LOCK'' +# + +--connection default +CREATE TABLE t1 ( + a int(11) unsigned default NULL, + b varchar(255) default NULL, + UNIQUE KEY a (a), + KEY b (b) +); + +INSERT INTO t1 VALUES (1, 1), (2, 2), (3, 3); +CREATE TABLE t2 SELECT * FROM t1; +CREATE TABLE t3 SELECT * FROM t1; + +--disable_query_log +let $i = 100; +while ($i) { +--dec $i + +--connection writer + send UPDATE t2 INNER JOIN (t1 JOIN t3 USING(a)) USING(a) + SET a = NULL WHERE t1.b <> t2.b; + +--connection locker + ALTER TABLE t2 ADD COLUMN (c INT); + ALTER TABLE t2 DROP COLUMN c; + +--connection writer +--reap +} + +# check PS mode: + +--connection writer +PREPARE stmt FROM 'UPDATE t2 INNER JOIN (t1 JOIN t3 USING(a)) USING(a) + SET a = NULL WHERE t1.b <> t2.b'; + +let $i = 100; +while ($i) { +--dec $i + +--connection writer +--send EXECUTE stmt + +--connection locker + ALTER TABLE t2 ADD COLUMN (c INT); + ALTER TABLE t2 DROP COLUMN c; + +--connection writer +--reap +} +--enable_query_log + +--connection default +DROP TABLE t1, t2, t3; + # End of 5.0 tests === modified file 'sql/item.cc' --- a/sql/item.cc 2008-10-01 09:48:47 +0000 +++ b/sql/item.cc 2008-10-06 22:48:46 +0000 @@ -1758,9 +1758,9 @@ Item_field::Item_field(THD *thd, Name_re We need to copy db_name, table_name and field_name because they must be allocated in the statement memory, not in table memory (the table structure can go away and pop up again between subsequent executions - of a prepared statement). + of a prepared statement or after the close_tables_for_reopen() call + in mysql_multi_update_prepare()). */ - if (thd->stmt_arena->is_stmt_prepare_or_first_sp_execute()) { if (db_name) orig_db_name= thd->strdup(db_name); === modified file 'sql/sql_base.cc' --- a/sql/sql_base.cc 2008-03-21 15:23:17 +0000 +++ b/sql/sql_base.cc 2008-10-06 22:48:46 +0000 @@ -3617,8 +3617,8 @@ find_field_in_natural_join(THD *thd, TAB { /* This is a base table. */ DBUG_ASSERT(nj_col->view_field == NULL); - DBUG_ASSERT(nj_col->table_ref->table == nj_col->table_field->table); - found_field= nj_col->table_field; + DBUG_ASSERT(nj_col->table_ref->table == nj_col->table_field->field->table); + found_field= nj_col->table_field->field; update_field_dependencies(thd, found_field, nj_col->table_ref->table); } @@ -4450,7 +4450,7 @@ mark_common_columns(THD *thd, TABLE_LIST const char *field_name_1; /* true if field_name_1 is a member of using_fields */ bool is_using_column_1; - if (!(nj_col_1= it_1.get_or_create_column_ref(leaf_1))) + if (!(nj_col_1= it_1.get_or_create_column_ref(thd, leaf_1))) goto err; field_name_1= nj_col_1->name(); is_using_column_1= using_fields && @@ -4471,7 +4471,7 @@ mark_common_columns(THD *thd, TABLE_LIST { Natural_join_column *cur_nj_col_2; const char *cur_field_name_2; - if (!(cur_nj_col_2= it_2.get_or_create_column_ref(leaf_2))) + if (!(cur_nj_col_2= it_2.get_or_create_column_ref(thd, leaf_2))) goto err; cur_field_name_2= cur_nj_col_2->name(); DBUG_PRINT ("info", ("cur_field_name_2=%s.%s", @@ -4963,8 +4963,12 @@ static bool setup_natural_join_row_types { table_ref= left_neighbor; left_neighbor= table_ref_it++; - /* For stored procedures do not redo work if already done. */ - if (context->select_lex->first_execution) + /* + Do not redo work if already done: + 1) for stored procedures, + 2) for multitable update after lock failure and table reopening. + */ + if (context->select_lex->first_execution && !thd->multiupdate_reopen) { if (store_top_level_join_columns(thd, table_ref, left_neighbor, right_neighbor)) === modified file 'sql/sql_class.cc' --- a/sql/sql_class.cc 2008-09-17 06:34:00 +0000 +++ b/sql/sql_class.cc 2008-10-06 22:48:46 +0000 @@ -178,7 +178,7 @@ THD::THD() last_insert_id_used(0), last_insert_id_used_bin_log(0), insert_id_used(0), clear_next_insert_id(0), in_lock_tables(0), bootstrap(0), derived_tables_processing(FALSE), spcont(NULL), - m_parser_state(NULL) + m_parser_state(NULL), multiupdate_reopen(0) { ulong tmp; === modified file 'sql/sql_class.h' --- a/sql/sql_class.h 2008-09-17 06:34:00 +0000 +++ b/sql/sql_class.h 2008-10-06 22:48:46 +0000 @@ -1581,6 +1581,12 @@ public: */ Parser_state *m_parser_state; + /* + TRUE if mysql_multi_update_prepare is reopening tables after lock_table + failure. For use in setup_natural_join_row_types only. + */ + bool multiupdate_reopen; + THD(); ~THD(); === modified file 'sql/sql_update.cc' --- a/sql/sql_update.cc 2008-07-15 14:13:21 +0000 +++ b/sql/sql_update.cc 2008-10-06 22:48:46 +0000 @@ -857,6 +857,9 @@ reopen_tables: if (!need_reopen) DBUG_RETURN(TRUE); + DBUG_PRINT("info", ("lock_tables failed, reopening")); + thd->multiupdate_reopen= TRUE; + /* We have to reopen tables since some of them were altered or dropped during lock_tables() or something was done with their triggers. @@ -872,10 +875,20 @@ reopen_tables: for (TABLE_LIST *tbl= table_list; tbl; tbl= tbl->next_global) tbl->cleanup_items(); + /* + Also we need to cleanup Natural_join_column::table_field items. + To not to traverse a join tree we will cleanup whole + thd->free_list (in PS execution mode that list may not contain + items from 'fields' list, so the cleanup above is necessary to. + */ + cleanup_items(thd->free_list); + close_tables_for_reopen(thd, &table_list); goto reopen_tables; } + thd->multiupdate_reopen= FALSE; + /* Check that we are not using table that we are updating, but we should skip all tables of UPDATE SELECT itself === modified file 'sql/table.cc' --- a/sql/table.cc 2007-10-10 13:26:02 +0000 +++ b/sql/table.cc 2008-10-06 22:48:46 +0000 @@ -2191,7 +2191,7 @@ TABLE_LIST *TABLE_LIST::find_underlying_ } /* - cleunup items belonged to view fields translation table + cleanup items belonged to view fields translation table SYNOPSIS TABLE_LIST::cleanup_items() @@ -2637,10 +2637,10 @@ Natural_join_column::Natural_join_column } -Natural_join_column::Natural_join_column(Field *field_param, +Natural_join_column::Natural_join_column(Item_field *field_param, TABLE_LIST *tab) { - DBUG_ASSERT(tab->table == field_param->table); + DBUG_ASSERT(tab->table == field_param->field->table); table_field= field_param; view_field= NULL; table_ref= tab; @@ -2668,7 +2668,7 @@ Item *Natural_join_column::create_item(T return create_view_field(thd, table_ref, &view_field->item, view_field->name); } - return new Item_field(thd, &thd->lex->current_select->context, table_field); + return table_field; } @@ -2679,7 +2679,7 @@ Field *Natural_join_column::field() DBUG_ASSERT(table_field == NULL); return NULL; } - return table_field; + return table_field->field; } @@ -2811,7 +2811,7 @@ void Field_iterator_natural_join::next() cur_column_ref= column_ref_it++; DBUG_ASSERT(!cur_column_ref || ! cur_column_ref->table_field || cur_column_ref->table_ref->table == - cur_column_ref->table_field->table); + cur_column_ref->table_field->field->table); } @@ -2975,7 +2975,7 @@ GRANT_INFO *Field_iterator_table_ref::gr */ Natural_join_column * -Field_iterator_table_ref::get_or_create_column_ref(TABLE_LIST *parent_table_ref) +Field_iterator_table_ref::get_or_create_column_ref(THD *thd, TABLE_LIST *parent_table_ref) { Natural_join_column *nj_col; bool is_created= TRUE; @@ -2988,7 +2988,11 @@ Field_iterator_table_ref::get_or_create_ { /* The field belongs to a stored table. */ Field *tmp_field= table_field_it.field(); - nj_col= new Natural_join_column(tmp_field, table_ref); + Item_field *tmp_item= + new Item_field(thd, &thd->lex->current_select->context, tmp_field); + if (!tmp_item) + return NULL; + nj_col= new Natural_join_column(tmp_item, table_ref); field_count= table_ref->table->s->fields; } else if (field_it == &view_field_it) @@ -3012,7 +3016,7 @@ Field_iterator_table_ref::get_or_create_ DBUG_ASSERT(nj_col); } DBUG_ASSERT(!nj_col->table_field || - nj_col->table_ref->table == nj_col->table_field->table); + nj_col->table_ref->table == nj_col->table_field->field->table); /* If the natural join column was just created add it to the list of @@ -3077,7 +3081,7 @@ Field_iterator_table_ref::get_natural_co nj_col= natural_join_it.column_ref(); DBUG_ASSERT(nj_col && (!nj_col->table_field || - nj_col->table_ref->table == nj_col->table_field->table)); + nj_col->table_ref->table == nj_col->table_field->field->table)); return nj_col; } === modified file 'sql/table.h' --- a/sql/table.h 2007-09-24 12:34:10 +0000 +++ b/sql/table.h 2008-10-06 22:48:46 +0000 @@ -18,6 +18,7 @@ class Item; /* Needed by ORDER */ class Item_subselect; +class Item_field; class GRANT_TABLE; class st_select_lex_unit; class st_select_lex; @@ -469,7 +470,7 @@ class Natural_join_column: public Sql_al { public: Field_translator *view_field; /* Column reference of merge view. */ - Field *table_field; /* Column reference of table or temp view. */ + Item_field *table_field; /* Column reference of table or temp view. */ TABLE_LIST *table_ref; /* Original base table/view reference. */ /* True if a common join column of two NATURAL/USING join operands. Notice @@ -481,7 +482,7 @@ public: bool is_common; public: Natural_join_column(Field_translator *field_param, TABLE_LIST *tab); - Natural_join_column(Field *field_param, TABLE_LIST *tab); + Natural_join_column(Item_field *field_param, TABLE_LIST *tab); const char *name(); Item *create_item(THD *thd); Field *field(); @@ -899,7 +900,7 @@ public: GRANT_INFO *grant(); Item *create_item(THD *thd) { return field_it->create_item(thd); } Field *field() { return field_it->field(); } - Natural_join_column *get_or_create_column_ref(TABLE_LIST *parent_table_ref); + Natural_join_column *get_or_create_column_ref(THD *thd, TABLE_LIST *parent_table_ref); Natural_join_column *get_natural_column_ref(); };