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*.

mysql-test/r/lock_multi.result

@@ -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;

mysql-test/t/lock_multi.test

@@ -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

sql/item.cc

@@ -1758,14 +1758,16 @@ Item_field::Item_field(THD *thd, Name_resolution_context *context_arg,
     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

sql/sql_base.cc

@@ -3617,8 +3617,21 @@ find_field_in_natural_join(THD *thd, TABLE_LIST *table_ref, const char *name,
   {
     /* 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 *table_ref_1, TABLE_LIST *table_ref_2,
     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 *table_ref_1, TABLE_LIST *table_ref_2,
     {
       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", 
@@ -4957,15 +4970,24 @@ static bool setup_natural_join_row_types(THD *thd,
   TABLE_LIST *left_neighbor;
   /* Table reference to the right of the current. */
   TABLE_LIST *right_neighbor= NULL;
+  bool save_first_natural_join_processing=
+    context->select_lex->first_natural_join_processing;
+
+  context->select_lex->first_natural_join_processing= FALSE;
 
   /* Note that tables in the list are in reversed order */
   for (left_neighbor= table_ref_it++; left_neighbor ; )
   {
     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 (save_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;

sql/sql_lex.cc

@@ -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;

sql/sql_lex.h

@@ -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;

sql/sql_update.cc

@@ -852,11 +852,14 @@ reopen_tables:
   }
 
   /* now lock and fill tables */
-  if (lock_tables(thd, table_list, table_count, &need_reopen))
+  if (!thd->stmt_arena->is_stmt_prepare() &&
+      lock_tables(thd, table_list, table_count, &need_reopen))
   {
     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 +875,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;
   }

sql/table.cc

@@ -2191,7 +2191,7 @@ TABLE_LIST *TABLE_LIST::find_underlying_table(TABLE *table_to_find)
 }
 
 /*
-  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(Field_translator *field_param,
 }
 
 
-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(THD *thd)
     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::grant()
 */
 
 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_column_ref(TABLE_LIST *parent_table_ref)
   {
     /* 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_column_ref(TABLE_LIST *parent_table_ref)
     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_column_ref()
   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;
 }
 

sql/table.h

@@ -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_alloc
 {
 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();
 };