bzr commit into mysql-5.0 branch (gshchepa:2674) Bug#38691

[Gleb Shchepa]

#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<Item> 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<TABLE_LIST> 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<TABLE_LIST> 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<String>);
-  /*
-    Cleanup for re-execution in a prepared statement or a stored
-    procedure.
-  */
 };
 
 class Item;