From: Date: October 7 2008 6:18pm Subject: bzr commit into mysql-5.0-bugteam branch (gshchepa:2689) Bug#38691 List-Archive: http://lists.mysql.com/commits/55611 X-Bug: 38691 Message-Id: <20081007161944.5BA29280E58@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_lex.cc sql/sql_lex.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 first_natural_join_processing 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_lex.cc Bug #38691: segfault/abort in ``UPDATE ...JOIN'' while ``FLUSH TABLES WITH READ LOCK'' Initialization of the new st_select_lex::first_natural_join_processing flag has been added. sql/sql_lex.h Bug #38691: segfault/abort in ``UPDATE ...JOIN'' while ``FLUSH TABLES WITH READ LOCK'' The st_select_lex::first_natural_join_processing flag has been added to skip unnecessary rebuilding of NATURAL/USING JOIN structures during table reopening after lock_tables failure. sql/sql_update.cc Bug #38691: segfault/abort in ``UPDATE ...JOIN'' while ``FLUSH TABLES WITH READ LOCK'' Extra cleanup calls have been added to reset Natural_join_column::table_field items. 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-07 16:18:17 +0000 @@ -99,3 +99,19 @@ 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; +# test altering of columns that multiupdate doesn't use +# normal mode +# PS mode +# test altering of columns that multiupdate uses +# normal mode +# PS mode +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-07 16:18:17 +0000 @@ -281,4 +281,123 @@ 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; + +--echo # test altering of columns that multiupdate doesn't use + +--echo # normal mode + +--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 +} + +--echo # 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 + + +--echo # test altering of columns that multiupdate uses + +--echo # normal mode + +--connection default + +--disable_query_log +let $i = 100; +while ($i) { + dec $i; + +--connection locker +--error 0,1060 + ALTER TABLE t2 ADD COLUMN a int(11) unsigned default NULL; + UPDATE t2 SET a=b; + +--connection writer +--send UPDATE t2 INNER JOIN (t1 JOIN t3 USING(a)) USING(a) SET a = NULL WHERE t1.b <> t2.b + +--connection locker +--error 0,1091 + ALTER TABLE t2 DROP COLUMN a; + +--connection writer +--error 0,1054 +--reap +} +--enable_query_log + +--echo # PS mode + +--disable_query_log +let $i = 100; +while ($i) { + dec $i; + +--connection locker +--error 0,1060 + ALTER TABLE t2 ADD COLUMN a int(11) unsigned default NULL; + UPDATE t2 SET a=b; + +--connection writer + PREPARE stmt FROM 'UPDATE t2 INNER JOIN (t1 JOIN t3 USING(a)) USING(a) SET a = NULL WHERE t1.b <> t2.b'; +--send EXECUTE stmt + +--connection locker +--error 0,1091 + ALTER TABLE t2 DROP COLUMN a; + +--connection writer +--error 0,1054 +--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-07 16:18:17 +0000 @@ -1758,14 +1758,16 @@ 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); - orig_table_name= thd->strdup(table_name); - orig_field_name= thd->strdup(field_name); + if (table_name) + orig_table_name= thd->strdup(table_name); + if (field_name) + orig_field_name= thd->strdup(field_name); /* We don't restore 'name' in cleanup because it's not changed during execution. Still we need it to point to persistent === 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-07 16:18:17 +0000 @@ -3617,8 +3617,21 @@ 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; + /* + This fix_fields is not necessary (initially this item is fixed by + the Item_field constructor; after reopen_tables the Item_func_eq + calls fix_fields on that item), it's just a check during table + reopening for columns that was dropped by the concurrent connection. + */ + if (!nj_col->table_field->fixed && + nj_col->table_field->fix_fields(thd, (Item **)&nj_col->table_field)) + { + DBUG_PRINT("info", ("column '%s' was dropped by the concurrent connection", + nj_col->table_field->name)); + DBUG_RETURN(NULL); + } + 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 +4463,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 +4484,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,9 +4976,15 @@ 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 && + context->select_lex->first_natural_join_processing) { + context->select_lex->first_natural_join_processing= FALSE; if (store_top_level_join_columns(thd, table_ref, left_neighbor, right_neighbor)) return TRUE; === modified file 'sql/sql_lex.cc' --- a/sql/sql_lex.cc 2008-07-14 21:41:30 +0000 +++ b/sql/sql_lex.cc 2008-10-07 16:18:17 +0000 @@ -1205,6 +1205,7 @@ void st_select_lex::init_query() subquery_in_having= explicit_limit= 0; is_item_list_lookup= 0; first_execution= 1; + first_natural_join_processing= 1; first_cond_optimization= 1; parsing_place= NO_MATTER; exclude_from_table_unique_test= no_wrap_view_item= FALSE; === modified file 'sql/sql_lex.h' --- a/sql/sql_lex.h 2008-08-11 16:10:00 +0000 +++ b/sql/sql_lex.h 2008-10-07 16:18:17 +0000 @@ -586,6 +586,7 @@ public: case of an error during prepare the PS is not created. */ bool first_execution; + bool first_natural_join_processing; bool first_cond_optimization; /* do not wrap view fields with Item_ref */ bool no_wrap_view_item; === 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-07 16:18:17 +0000 @@ -857,6 +857,8 @@ reopen_tables: if (!need_reopen) DBUG_RETURN(TRUE); + DBUG_PRINT("info", ("lock_tables failed, reopening")); + /* We have to reopen tables since some of them were altered or dropped during lock_tables() or something was done with their triggers. @@ -872,6 +874,14 @@ 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; } === modified file 'sql/table.cc' --- a/sql/table.cc 2007-10-10 13:26:02 +0000 +++ b/sql/table.cc 2008-10-07 16:18:17 +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-07 16:18:17 +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(); };