From: Date: September 16 2008 8:16pm Subject: bzr commit into mysql-5.0 branch (gshchepa:2674) Bug#38691 List-Archive: http://lists.mysql.com/commits/54225 X-Bug: 38691 Message-Id: <20080916182145.BFCFB28117A@localhost.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7BIT #At file:///work/bzr/5.0-bugteam-38691/ 2674 Gleb Shchepa 2008-09-16 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. modified: mysql-test/r/lock_multi.result mysql-test/t/lock_multi.test sql/item.cc sql/sql_update.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 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_update.cc The mysql_multi_update_prepare() function has been modified to: 1) cleanup all items used in a query, 2) walk over all TABLE_LIST structures of a query and reset all NATURAL/USING join-related data for re-execution of query preparation after failed attempt to lock updating tables. sql/table.h The TABLE_LIST::cleanup_join_columns() function has been added to cleanup all NATURAL/USING join-related data (currently it simply resets that data). === 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-09-16 18:16:26 +0000 @@ -99,3 +99,18 @@ kill query ERROR 70100: Query execution was interrupted unlock tables; drop table t1; +# +# Bug #38691: segfault/abort in ``UPDATE ...JOIN'' while +# ``FLUSH TABLES WITH READ LOCK'' +# +CREATE TABLE t1 ( +knr int(11) unsigned default NULL, +em varchar(255) default NULL, +UNIQUE KEY knr (knr), +KEY em (em) +); +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; +# End of 5.0 tests === 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-09-16 18:16:26 +0000 @@ -281,4 +281,41 @@ unlock tables; connection default; drop table t1; -# End of 5.0 tests +--echo # +--echo # Bug #38691: segfault/abort in ``UPDATE ...JOIN'' while +--echo # ``FLUSH TABLES WITH READ LOCK'' +--echo # + +CREATE TABLE t1 ( + knr int(11) unsigned default NULL, + em varchar(255) default NULL, + UNIQUE KEY knr (knr), + KEY em (em) +); + +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(knr)) USING(knr) + SET knr = NULL WHERE t1.em <> t2.em; + + 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; + +--echo # End of 5.0 tests === modified file 'sql/item.cc' --- a/sql/item.cc 2008-08-20 09:49:28 +0000 +++ b/sql/item.cc 2008-09-16 18:16:26 +0000 @@ -1756,9 +1756,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_update.cc' --- a/sql/sql_update.cc 2008-07-15 14:13:21 +0000 +++ b/sql/sql_update.cc 2008-09-16 18:16:26 +0000 @@ -857,20 +857,33 @@ reopen_tables: if (!need_reopen) DBUG_RETURN(TRUE); + DBUG_PRINT("info", ("reopen tables for multiupdate")); + /* We have to reopen tables since some of them were altered or dropped during lock_tables() or something was done with their triggers. Let us do some cleanups to be able do setup_table() and setup_fields() once again. */ - List_iterator_fast it(*fields); - Item *item; - while ((item= it++)) - item->cleanup(); + cleanup_items(thd->free_list); + /* We have to cleanup NATURAL/USING JOIN column structures. */ + List_iterator_fast from_it(lex->select_lex.top_join_list); + TABLE_LIST *tbl; + while ((tbl= from_it++)) + { + tbl->cleanup_join_columns(); + if (tbl->nested_join) + { + List_iterator_fast join_it(tbl->nested_join->join_list); + TABLE_LIST *tbl1; + while ((tbl1= join_it++)) + tbl1->cleanup_join_columns(); + } + } /* We have to cleanup translation tables of views. */ - for (TABLE_LIST *tbl= table_list; tbl; tbl= tbl->next_global) - tbl->cleanup_items(); + for (tbl= table_list; tbl; tbl= tbl->next_global) + tbl->cleanup_join_columns(); close_tables_for_reopen(thd, &table_list); goto reopen_tables; === modified file 'sql/table.h' --- a/sql/table.h 2007-09-24 12:34:10 +0000 +++ b/sql/table.h 2008-09-16 18:16:26 +0000 @@ -768,6 +768,15 @@ struct TABLE_LIST procedure. */ void reinit_before_use(THD *thd); + /* + Cleanup for a reopen after lock_tables() failure in a multiupdate + query. + */ + void cleanup_join_columns() + { + join_columns= NULL; + is_join_columns_complete= FALSE; + } Item_subselect *containing_subselect(); private: @@ -775,10 +784,6 @@ private: bool prep_where(THD *thd, Item **conds, bool no_where_clause); void print_index_hint(THD *thd, String *str, const char *hint, uint32 hint_length, List); - /* - Cleanup for re-execution in a prepared statement or a stored - procedure. - */ }; class Item;