List:Internals« Previous MessageNext Message »
From:dlenev Date:September 14 2005 12:27am
Subject:bk commit into 5.0 tree (dlenev:1.1951) BUG#12704
View as plain text  
Below is the list of changes that have just been committed into a local
5.0 repository of dlenev. When dlenev does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet
  1.1951 05/09/14 02:27:24 dlenev@stripped +16 -0
  Possible solution for bug #12704 "Server crashes during trigger execution".
  
  This bug occurs when some trigger for table used by DML statement is created
  or changed while statement was waiting in lock_tables(). In this situation
  prelocking set which we have calculated becames invalid which can easily lead
  to errors and even in some cases to crashes.
  
  With proposed patch we no longer silently reopen tables in lock_tables(),
  instead caller of lock_tables() becomes responsible for reopening tables and
  recalculation of prelocking set.
  
  Question: does not this mean that to avoid similar problems with SP and PS we
  have to recalculate prelocking set for each instruction in SP or each execution
  of PS? Or SergeyP's implementation of stored routines invalidation should handle
  this case? When may be it is possible to use this common method for solving this
  problem for ordinary statements?

  sql/sql_update.cc
    1.169 05/09/14 02:27:18 dlenev@stripped +29 -30
    Now instead of silently reopening altered or dropped tables in
    lock_tables() we notify caller and rely on that the caller will
    reopen tables.

  sql/sql_trigger.h
    1.14 05/09/14 02:27:18 dlenev@stripped +2 -0
    Added Table_triggers_list::set_table() method to adjust Table_triggers_list
    to new pointer to TABLE instance.

  sql/sql_trigger.cc
    1.28 05/09/14 02:27:18 dlenev@stripped +19 -0
    Added Table_triggers_list::set_table() method to adjust Table_triggers_list
    to new pointer to TABLE instance.

  sql/sql_table.cc
    1.270 05/09/14 02:27:18 dlenev@stripped +8 -1
    mysql_lock_tables() has new 'need_reopen' argument which allows to control
    whether mysql_lock_tables() should reopen deleted or altered tables by itself
    or notify caller about such situation and rely on it in this.

  sql/sql_prepare.cc
    1.152 05/09/14 02:27:18 dlenev@stripped +19 -13
    Now instead of silently reopening altered or dropped tables in
    lock_tables() we notify caller and rely on that the caller will
    reopen tables.

  sql/sql_lex.h
    1.196 05/09/14 02:27:18 dlenev@stripped +16 -0
    LEX:
      Added 'sroutines_list_own_last' and 'sroutines_list_own_elements' members
      which are used for keeping state in which 'sroutines_list' was right after
      statement parsing (and for restoring of this list to this state).
      Added chop_off_not_own_tables() method to simplify throwing away list
      of implicitly used (prelocked) tables.

  sql/sql_lex.cc
    1.168 05/09/14 02:27:18 dlenev@stripped +6 -0
    LEX:
      Added 'sroutines_list_own_last' and 'sroutines_list_own_elements' members
      which are used for keeping state in which 'sroutines_list' was right after
      statement parsing (and for restoring of this list to this state).

  sql/sql_insert.cc
    1.174 05/09/14 02:27:18 dlenev@stripped +2 -1
    mysql_lock_tables() has new 'need_reopen' argument which allows to control
    whether mysql_lock_tables() should reopen deleted or altered tables by itself
    or notify caller about such situation and rely on it in this.

  sql/sql_handler.cc
    1.74 05/09/14 02:27:18 dlenev@stripped +1 -1
    mysql_lock_tables() has new 'need_reopen' argument which allows to control
    whether mysql_lock_tables() should reopen deleted or altered tables by itself
    or notify caller about such situation and rely on it in this.

  sql/sql_base.cc
    1.297 05/09/14 02:27:18 dlenev@stripped +77 -31
    reopen_table():
      When we re-open table and do shallow copy of TABLE object we should adjust
      pointers to it in associated Table_triggers_list object.
    open_tables():
      Now this function is able to rebuild prelocking set for statement if it is
      needed. It also correctly handles FLUSH TABLES which may occur during its
      execution.
    lock_tables():
      Instead of allowing mysql_lock_tables() to silently reopen altered or dropped
      tables let us notify caller and rely on that it will do reopen itself.
      We also do proper preparations for recalculation of prelocking set.
      This solves problem when trigger suddenly appears or changed during
      mysq_lock_tables().

  sql/sp_head.cc
    1.180 05/09/14 02:27:18 dlenev@stripped +0 -4
    Removed assert which is no longer always true.

  sql/sp.h
    1.29 05/09/14 02:27:18 dlenev@stripped +1 -0
    sp_remove_not_own_routines():
      Added procedure for restoring LEX::sroutines/sroutines_list to their state
      right after parsing (by throwing out non-directly used routines).

  sql/sp.cc
    1.93 05/09/14 02:27:18 dlenev@stripped +52 -4
    sp_add_used_routine():
      To be able to restore LEX::sroutines_list to its state right after parsing
      we now adjust  LEX::sroutines_list_own_last/sroutines_list_own_elements when
      we add directly used routine.
    sp_remove_not_own_routines():
      Added procedure for restoring LEX::sroutines/sroutines_list to their state
      right after parsing (by throwing out non-directly used routines).
    sp_cache_routines_and_add_tables_for_view()/sp_update_stmt_used_routines():
      We should use LEX::sroutines_list instead of LEX::sroutines as source of
      routines used by view, since LEX::sroutines is not availiable for view
      on second attempt to open it (see comment in open_tables() about it).

  sql/mysql_priv.h
    1.347 05/09/14 02:27:18 dlenev@stripped +3 -2
    mysql_lock_tables() has new 'need_reopen' argument which allows to control
    whether mysql_lock_tables() should reopen deleted or altered tables by itself
    or notify caller about such situation and rely on it in this.
    Also lock_tables() has new 'need_reopen' out parameter through which it
    notifies caller that some tables were altered or dropped so he needs to reopen
    them (and rebuild prelocking set some triggers may change or simply appear).

  sql/lock.cc
    1.77 05/09/14 02:27:17 dlenev@stripped +15 -3
    mysql_lock_tables():
      Added need_reopen argument which allows to control whether mysql_lock_tables()
      should reopen deleted or altered tables by itself or notify caller about such
      situation and rely on it in this.

  mysql-test/t/trigger.test
    1.27 05/09/14 02:27:17 dlenev@stripped +113 -3
    Added tests for bug #12704 "Server crashes during trigger execution".
    Unfortunately these tests rely on the order in which tables are locked
    by statement so they are non-determenistic and therefore should be disabled.

# This is a BitKeeper patch.  What follows are the unified diffs for the
# set of deltas contained in the patch.  The rest of the patch, the part
# that BitKeeper cares about, is below these diffs.
# User:	dlenev
# Host:	brandersnatch.localdomain
# Root:	/home/dlenev/src/mysql-5.0-bg12704-2

--- 1.76/sql/lock.cc	2005-08-11 16:58:11 +04:00
+++ 1.77/sql/lock.cc	2005-09-14 02:27:17 +04:00
@@ -93,21 +93,28 @@
     flags                       Options:
       MYSQL_LOCK_IGNORE_GLOBAL_READ_LOCK      Ignore a global read lock
       MYSQL_LOCK_IGNORE_FLUSH                 Ignore a flush tables.
+    need_reopen                 If 0 then lock_tables() should reopen tables
+                                which were altered or dropped by itself.
+                                If non-0 then instead of reopening tables by
+                                itself it should notify upper level by setting
+                                *need_reopen to TRUE and returning NULL.
 
   RETURN
     A lock structure pointer on success.
-    NULL on error.
+    NULL on error or if some tables were altered or deleted and
+    therefore should be reopen.
 */
 
+/* Map the return value of thr_lock to an error from errmsg.txt */
 static int thr_lock_errno_to_mysql[]=
 { 0, 1, ER_LOCK_WAIT_TIMEOUT, ER_LOCK_DEADLOCK };
 
-MYSQL_LOCK *mysql_lock_tables(THD *thd, TABLE **tables, uint count, uint flags)
+MYSQL_LOCK *mysql_lock_tables(THD *thd, TABLE **tables, uint count,
+                              uint flags, bool *need_reopen)
 {
   MYSQL_LOCK *sql_lock;
   TABLE *write_lock_used;
   int rc;
-  /* Map the return value of thr_lock to an error from errmsg.txt */
   DBUG_ENTER("mysql_lock_tables");
 
   for (;;)
@@ -178,6 +185,11 @@
     thd->locked=0;
 retry:
     sql_lock=0;
+    if (need_reopen)
+    {
+      *need_reopen= TRUE;
+      break;
+    }
     if (wait_for_tables(thd))
       break;					// Couldn't open tables
   }

--- 1.346/sql/mysql_priv.h	2005-09-03 03:15:02 +04:00
+++ 1.347/sql/mysql_priv.h	2005-09-14 02:27:18 +04:00
@@ -942,7 +942,7 @@
 int simple_open_n_lock_tables(THD *thd,TABLE_LIST *tables);
 bool open_and_lock_tables(THD *thd,TABLE_LIST *tables);
 bool open_normal_and_derived_tables(THD *thd, TABLE_LIST *tables, uint flags);
-int lock_tables(THD *thd, TABLE_LIST *tables, uint counter);
+int lock_tables(THD *thd, TABLE_LIST *tables, uint counter, bool *need_reopen);
 TABLE *open_temporary_table(THD *thd, const char *path, const char *db,
 			    const char *table_name, bool link_in_list);
 bool rm_temporary_table(enum db_type base, char *path);
@@ -1227,7 +1227,8 @@
 extern struct st_VioSSLAcceptorFd * ssl_acceptor_fd;
 #endif /* HAVE_OPENSSL */
 
-MYSQL_LOCK *mysql_lock_tables(THD *thd, TABLE **table, uint count, uint flags);
+MYSQL_LOCK *mysql_lock_tables(THD *thd, TABLE **table, uint count,
+                              uint flags, bool *need_reopen);
 /* mysql_lock_tables() flags bits */
 #define MYSQL_LOCK_IGNORE_GLOBAL_READ_LOCK      0x0001
 #define MYSQL_LOCK_IGNORE_FLUSH                 0x0002

--- 1.296/sql/sql_base.cc	2005-09-02 18:06:09 +04:00
+++ 1.297/sql/sql_base.cc	2005-09-14 02:27:18 +04:00
@@ -1397,6 +1397,7 @@
   tmp.status=		table->status;
   tmp.keys_in_use_for_query= tmp.s->keys_in_use;
   tmp.used_keys= 	tmp.s->keys_for_keyread;
+  /* FIXME: seems to be error ? */
   tmp.force_index=	tmp.force_index;
 
   /* Get state */
@@ -1428,6 +1429,9 @@
   for (key=0 ; key < table->s->keys ; key++)
     for (part=0 ; part < table->key_info[key].usable_key_parts ; part++)
       table->key_info[key].key_part[part].field->table= table;
+  if (table->triggers)
+    table->triggers->set_table(table);
+
   VOID(pthread_cond_broadcast(&COND_refresh));
   error=0;
 
@@ -1517,7 +1521,8 @@
     MYSQL_LOCK *lock;
     /* We should always get these locks */
     thd->some_tables_deleted=0;
-    if ((lock= mysql_lock_tables(thd, tables, (uint) (tables_ptr - tables), 0)))
+    if ((lock= mysql_lock_tables(thd, tables, (uint) (tables_ptr - tables),
+                                 0, 0)))
     {
       thd->locked_tables=mysql_lock_merge(thd->locked_tables,lock);
     }
@@ -1967,9 +1972,15 @@
     /*
       Ignore placeholders for derived tables. After derived tables
       processing, link to created temporary table will be put here.
+      If this is derived table for view then we still want to process
+      routines used by this view.
      */
     if (tables->derived)
+    {
+      if (tables->view)
+        goto process_view_routines;
       continue;
+    }
     if (tables->schema_table)
     {
       if (!mysql_schema_table(thd, thd->lex, tables))
@@ -2001,23 +2012,12 @@
         if (query_tables_last_own == &(tables->next_global) &&
             tables->view->query_tables)
           query_tables_last_own= tables->view->query_tables_last;
-        
         /*
-          Again if needed we have to get cache all routines used by this view
-          and add tables used by them to table list.
+          Let us free memory used by 'sroutines' hash here since we never
+          call destructor for this LEX.
         */
-        if (!thd->prelocked_mode && !thd->lex->requires_prelocking()
&&
-            tables->view->sroutines.records)
-        {
-          /* We have at least one table in TL here */
-          if (!query_tables_last_own)
-            query_tables_last_own= thd->lex->query_tables_last;
-          sp_cache_routines_and_add_tables_for_view(thd, thd->lex,
-                                                    tables->view);
-        }
-        /* Cleanup hashes because destructo for this LEX is never called */
         hash_free(&tables->view->sroutines);
-	continue;
+	goto process_view_routines;
       }
 
       if (refresh)				// Refresh in progress
@@ -2029,11 +2029,6 @@
 	thd->version=refresh_version;
 	TABLE **prev_table= &thd->open_tables;
 	bool found=0;
-        /*
-          QQ: What we should do if we have started building of table list
-          for prelocking ??? Probably throw it away ? But before we should
-          mark all temporary tables as free? How about locked ?
-        */
 	for (TABLE_LIST *tmp= *start; tmp; tmp= tmp->next_global)
 	{
 	  /* Close normal (not temporary) changed tables */
@@ -2057,6 +2052,10 @@
 	pthread_mutex_unlock(&LOCK_open);
 	if (found)
 	  VOID(pthread_cond_broadcast(&COND_refresh)); // Signal to refresh
+        /* We have to prepare statement for recalution of prelocked set. */
+        thd->lex->mark_as_requiring_prelocking(query_tables_last_own);
+        thd->lex->chop_off_not_own_tables();
+        sp_remove_not_own_routines(thd->lex);
 	goto restart;
       }
       result= -1;				// Fatal error
@@ -2087,6 +2086,21 @@
     if (tables->lock_type != TL_UNLOCK && ! thd->locked_tables)
       tables->table->reginfo.lock_type=tables->lock_type;
     tables->table->grant= tables->grant;
+
+process_view_routines:
+    /*
+      Again we may need cache all routines used by this view and add
+      tables used by them to table list.
+    */
+    if (tables->view && !thd->prelocked_mode &&
+        !thd->lex->requires_prelocking() &&
+        tables->view->sroutines_list.elements)
+    {
+      /* We have at least one table in TL here. */
+      if (!query_tables_last_own)
+        query_tables_last_own= thd->lex->query_tables_last;
+      sp_cache_routines_and_add_tables_for_view(thd, thd->lex, tables->view);
+    }
   }
   thd->proc_info=0;
   free_root(&new_frm_mem, MYF(0));              // Free pre-alloced block
@@ -2191,7 +2205,7 @@
     {
       DBUG_ASSERT(thd->lock == 0);	// You must lock everything at once
       if ((table->reginfo.lock_type= lock_type) != TL_UNLOCK)
-	if (! (thd->lock= mysql_lock_tables(thd, &table_list->table, 1, 0)))
+	if (! (thd->lock= mysql_lock_tables(thd, &table_list->table, 1, 0, 0)))
 	  table= 0;
     }
   }
@@ -2219,11 +2233,16 @@
 
 int simple_open_n_lock_tables(THD *thd, TABLE_LIST *tables)
 {
-  DBUG_ENTER("simple_open_n_lock_tables");
   uint counter;
-  if (open_tables(thd, &tables, &counter, 0) ||
-      lock_tables(thd, tables, counter))
-    DBUG_RETURN(-1);				/* purecov: inspected */
+  bool need_reopen;
+  DBUG_ENTER("simple_open_n_lock_tables");
+
+  do
+  {
+    if (open_tables(thd, &tables, &counter, 0) ||
+        lock_tables(thd, tables, counter, &need_reopen))
+    DBUG_RETURN(-1);
+  } while (need_reopen);
   DBUG_RETURN(0);
 }
 
@@ -2248,10 +2267,16 @@
 bool open_and_lock_tables(THD *thd, TABLE_LIST *tables)
 {
   uint counter;
+  bool need_reopen;
   DBUG_ENTER("open_and_lock_tables");
-  if (open_tables(thd, &tables, &counter, 0) ||
-      lock_tables(thd, tables, counter) ||
-      mysql_handle_derived(thd->lex, &mysql_derived_prepare) ||
+
+  do
+  {
+    if (open_tables(thd, &tables, &counter, 0) ||
+        lock_tables(thd, tables, counter, &need_reopen))
+    DBUG_RETURN(-1);
+  } while (need_reopen);
+  if (mysql_handle_derived(thd->lex, &mysql_derived_prepare) ||
       (thd->fill_derived_tables() &&
        mysql_handle_derived(thd->lex, &mysql_derived_filling)))
     DBUG_RETURN(TRUE); /* purecov: inspected */
@@ -2319,7 +2344,11 @@
     lock_tables()
     thd			Thread handler
     tables		Tables to lock
-    count		umber of opened tables
+    count		Number of opened tables
+    need_reopen         Out parameter which indicates that some tables were
+                        dropped or altered during this call and therefore
+                        invoker should reopen tables and try to lock them
+                        once again.
 
   NOTES
     You can't call lock_tables twice, as this would break the dead-lock-free
@@ -2335,7 +2364,7 @@
    -1	Error
 */
 
-int lock_tables(THD *thd, TABLE_LIST *tables, uint count)
+int lock_tables(THD *thd, TABLE_LIST *tables, uint count, bool *need_reopen)
 {
   TABLE_LIST *table;
 
@@ -2351,6 +2380,8 @@
   */
   DBUG_ASSERT(!thd->lex->requires_prelocking() || tables);
 
+  *need_reopen= FALSE;
+
   if (!tables)
     DBUG_RETURN(0);
 
@@ -2383,12 +2414,27 @@
       thd->options|= OPTION_TABLE_LOCK;
     }
 
-    if (! (thd->lock= mysql_lock_tables(thd, start, (uint) (ptr - start), 0)))
+    if (! (thd->lock= mysql_lock_tables(thd, start, (uint) (ptr - start),
+                                        0, need_reopen)))
     {
       if (thd->lex->requires_prelocking())
       {
         thd->options&= ~(ulong) (OPTION_TABLE_LOCK);
         thd->in_lock_tables=0;
+      }
+      if (*need_reopen)
+      {
+        /*
+          Prepare statement for reopening of tables. Particularly
+          prepare to recalculate set of prelocked tables.
+        */
+        thd->lex->chop_off_not_own_tables();
+        sp_remove_not_own_routines(thd->lex);
+        for (TABLE_LIST *tmp= tables; tmp; tmp= tmp->next_global)
+          if (tmp->table && !tmp->table->s->tmp_table)
+            tmp->table= 0;
+        close_thread_tables(thd);
+        DBUG_RETURN(0);
       }
       DBUG_RETURN(-1);
     }

--- 1.173/sql/sql_insert.cc	2005-09-02 10:50:10 +04:00
+++ 1.174/sql/sql_insert.cc	2005-09-14 02:27:18 +04:00
@@ -1837,7 +1837,8 @@
         inserts are done.
       */
       if (! (thd->lock= mysql_lock_tables(thd, &di->table, 1,
-                                          MYSQL_LOCK_IGNORE_GLOBAL_READ_LOCK)))
+                                          MYSQL_LOCK_IGNORE_GLOBAL_READ_LOCK,
+                                          0)))
       {
 	/* Fatal error */
 	di->dead= 1;

--- 1.167/sql/sql_lex.cc	2005-09-03 03:13:10 +04:00
+++ 1.168/sql/sql_lex.cc	2005-09-14 02:27:18 +04:00
@@ -179,6 +179,8 @@
   if (lex->sroutines.records)
     my_hash_reset(&lex->sroutines);
   lex->sroutines_list.empty();
+  lex->sroutines_list_own_last= lex->sroutines_list.next;
+  lex->sroutines_list_own_elements= 0;
   DBUG_VOID_RETURN;
 }
 
@@ -1613,6 +1615,8 @@
 {
   hash_init(&sroutines, system_charset_info, 0, 0, 0, sp_sroutine_key, 0, 0);
   sroutines_list.empty();
+  sroutines_list_own_last= sroutines_list.next;
+  sroutines_list_own_elements= 0;
 }
 
 
@@ -2025,6 +2029,8 @@
   if (sroutines.records)
     my_hash_reset(&sroutines);
   sroutines_list.empty();
+  sroutines_list_own_last= sroutines_list.next;
+  sroutines_list_own_elements= 0;
 }
 
 

--- 1.195/sql/sql_lex.h	2005-09-03 03:13:10 +04:00
+++ 1.196/sql/sql_lex.h	2005-09-14 02:27:18 +04:00
@@ -843,8 +843,15 @@
   /*
     List linking elements of 'sroutines' set. Allows you to add new elements
     to this set as you iterate through the list of existing elements.
+    'sroutines_list_own_last' is pointer to ::next memebr of last element of
+    this list which represents routine which is explicitly used by query.
+    'sroutines_list_own_elements' number of explicitly used routines.
+    We use these two members for restoring of 'sroutines_list' to the state
+    in which it was right after query parsing.
   */
   SQL_LIST sroutines_list;
+  byte     **sroutines_list_own_last;
+  uint     sroutines_list_own_elements;
 
   st_sp_chistics sp_chistics;
   bool only_view;       /* used for SHOW CREATE TABLE/VIEW */
@@ -955,6 +962,15 @@
   TABLE_LIST* first_not_own_table()
   {
     return ( query_tables_own_last ? *query_tables_own_last : 0);
+  }
+  void chop_off_not_own_tables()
+  {
+    if (query_tables_own_last)
+    {
+      *query_tables_own_last= 0;
+      query_tables_last= query_tables_own_last;
+      query_tables_own_last= 0;
+    }
   }
   void cleanup_after_one_table_open();
 

--- 1.269/sql/sql_table.cc	2005-09-01 11:05:39 +04:00
+++ 1.270/sql/sql_table.cc	2005-09-14 02:27:18 +04:00
@@ -1762,8 +1762,15 @@
       DBUG_RETURN(0);
   }
 
+  /*
+    FIXME: What happens if trigger manages to be created while we are
+           obtaining this lock ? May be it is sensible just to disable
+           trigger execution in this case ? Or will MYSQL_LOCK_IGNORE_FLUSH
+           save us from that ?
+  */
   table->reginfo.lock_type=TL_WRITE;
-  if (! ((*lock)= mysql_lock_tables(thd, &table, 1, MYSQL_LOCK_IGNORE_FLUSH)))
+  if (! ((*lock)= mysql_lock_tables(thd, &table, 1,
+                                    MYSQL_LOCK_IGNORE_FLUSH, 0)))
   {
     VOID(pthread_mutex_lock(&LOCK_open));
     hash_delete(&open_cache,(byte*) table);

--- 1.168/sql/sql_update.cc	2005-09-01 23:42:22 +04:00
+++ 1.169/sql/sql_update.cc	2005-09-14 02:27:18 +04:00
@@ -134,25 +134,31 @@
   SQL_SELECT	*select;
   READ_RECORD	info;
   SELECT_LEX    *select_lex= &thd->lex->select_lex;
+  bool need_reopen;
   DBUG_ENTER("mysql_update");
 
   LINT_INIT(timestamp_query_id);
 
-  if (open_tables(thd, &table_list, &table_count, 0))
-    DBUG_RETURN(1);
-
-  if (table_list->multitable_view)
+  do
   {
-    DBUG_ASSERT(table_list->view != 0);
-    DBUG_PRINT("info", ("Switch to multi-update"));
-    /* pass counter value */
-    thd->lex->table_count= table_count;
-    /* convert to multiupdate */
-    return 2;
-  }
+    if (open_tables(thd, &table_list, &table_count, 0))
+      DBUG_RETURN(1);
+
+    if (table_list->multitable_view)
+    {
+      DBUG_ASSERT(table_list->view != 0);
+      DBUG_PRINT("info", ("Switch to multi-update"));
+      /* pass counter value */
+      thd->lex->table_count= table_count;
+      /* convert to multiupdate */
+      DBUG_RETURN(2);
+    }
+    if (lock_tables(thd, table_list, table_count, &need_reopen))
+        DBUG_RETURN(1);
 
-  if (lock_tables(thd, table_list, table_count) ||
-      mysql_handle_derived(thd->lex, &mysql_derived_prepare) ||
+  } while(need_reopen);
+
+  if (mysql_handle_derived(thd->lex, &mysql_derived_prepare) ||
       (thd->fill_derived_tables() &&
        mysql_handle_derived(thd->lex, &mysql_derived_filling)))
     DBUG_RETURN(1);
@@ -616,7 +622,6 @@
 bool mysql_multi_update_prepare(THD *thd)
 {
   LEX *lex= thd->lex;
-  ulong opened_tables;
   TABLE_LIST *table_list= lex->query_tables;
   TABLE_LIST *tl, *leaves;
   List<Item> *fields= &lex->select_lex.item_list;
@@ -630,13 +635,16 @@
   uint  table_count= lex->table_count;
   const bool using_lock_tables= thd->locked_tables != 0;
   bool original_multiupdate= (thd->lex->sql_command == SQLCOM_UPDATE_MULTI);
+  bool need_reopen= FALSE;
   DBUG_ENTER("mysql_multi_update_prepare");
 
   /* following need for prepared statements, to run next time multi-update */
   thd->lex->sql_command= SQLCOM_UPDATE_MULTI;
 
+reopen_tables:
+
   /* open tables and create derived ones, but do not lock and fill them */
-  if ((original_multiupdate &&
+  if (((original_multiupdate || need_reopen) &&
        open_tables(thd, &table_list, &table_count, 0)) ||
       mysql_handle_derived(lex, &mysql_derived_prepare))
     DBUG_RETURN(TRUE);
@@ -741,20 +749,16 @@
     }
   }
 
-  opened_tables= thd->status_var.opened_tables;
   /* now lock and fill tables */
-  if (lock_tables(thd, table_list, table_count))
+  if (lock_tables(thd, table_list, table_count, &need_reopen))
     DBUG_RETURN(TRUE);
 
-  /*
-    we have to re-call fixfields for fixed items, because lock maybe
-    reopened tables
-  */
-  if (opened_tables != thd->status_var.opened_tables)
+  if (need_reopen)
   {
     /*
-      Fields items cleanup(). There are only Item_fields in the list, so we
-      do not do Item tree walking
+      Some tables 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()/setup_fields() once again.
     */
     List_iterator_fast<Item> it(*fields);
     Item *item;
@@ -765,12 +769,7 @@
     for (TABLE_LIST *tbl= table_list; tbl; tbl= tbl->next_global)
       tbl->cleanup_items();
 
-    if (setup_tables(thd, &lex->select_lex.context,
-                     &lex->select_lex.top_join_list,
-                     table_list, &lex->select_lex.where,
-                     &lex->select_lex.leaf_tables, FALSE) ||
-        setup_fields_with_no_wrap(thd, 0, *fields, 1, 0, 0))
-      DBUG_RETURN(TRUE);
+    goto reopen_tables;
   }
 
   /*

--- 1.26/mysql-test/t/trigger.test	2005-09-03 03:13:09 +04:00
+++ 1.27/mysql-test/t/trigger.test	2005-09-14 02:27:17 +04:00
@@ -10,6 +10,11 @@
 drop procedure if exists p1;
 --enable_warnings
 
+# Create additional connections used through test
+connect (addconroot1, localhost, root,,);
+connect (addconroot2, localhost, root,,);
+connection default;
+
 create table t1 (i int);
 
 # let us test some very simple trigger
@@ -680,12 +685,10 @@
 delimiter ;|
 update t1 set data = 1;
 
-connect (addconroot, localhost, root,,);
-connection addconroot;
+connection addconroot1;
 update t1 set data = 2;
 
 connection default;
-disconnect addconroot;
 drop table t1;
 
 #
@@ -765,3 +768,110 @@
 select * from t1;
 drop trigger t1_bi;
 drop tables t1, t2;
+
+# Tests for bug #12704 "Server crashes during trigger execution".
+# If we run DML statements and CREATE TRIGGER statements concurrently
+# it may happen that trigger will be created while DML statement is
+# waiting for table lock. In this case we have to reopen tables and
+# recalculate prelocking set.
+# Unfortunately these tests rely on the order in which tables are locked
+# by statement so they are non determenistic and are disabled.
+--disable_parsing
+create table t1 (id int);
+create table t2 (id int);
+create table t3 (id int);
+create function f1() returns int return (select max(id)+2 from t2);
+create view v1 as select f1() as f;
+
+# Let us check that we notice trigger at all
+connection addconroot1;
+lock tables t2 write;
+connection default;
+send insert into t1 values ((select max(id) from t2)), (2);
+--sleep 1
+connection addconroot2;
+create trigger t1_trg before insert on t1 for each row set NEW.id:= 1;
+connection addconroot1;
+unlock tables;
+connection default;
+reap;
+select * from t1;
+
+# Check that we properly calculate new prelocking set
+insert into t2 values (3);
+connection addconroot1;
+lock tables t2 write;
+connection default;
+send insert into t1 values ((select max(id) from t2)), (4);
+--sleep 1
+connection addconroot2;
+drop trigger t1_trg;
+create trigger t1_trg before insert on t1 for each row
+  insert into t3 values (new.id);
+connection addconroot1;
+unlock tables;
+connection default;
+reap;
+select * from t1;
+select * from t3;
+
+# We should be able to do this even if fancy views are involved
+connection addconroot1;
+lock tables t2 write;
+connection default;
+send insert into t1 values ((select max(f) from v1)), (6);
+--sleep 1
+connection addconroot2;
+drop trigger t1_trg;
+create trigger t1_trg before insert on t1 for each row
+  insert into t3 values (new.id + 100);
+connection addconroot1;
+unlock tables;
+connection default;
+reap;
+select * from t1;
+select * from t3;
+
+# This also should work for multi-update
+# Let us drop trigger to demonstrate that prelocking set is really
+# rebuilt
+drop trigger t1_trg;
+connection addconroot1;
+lock tables t2 write;
+connection default;
+send update t1, t2 set t1.id=10 where t1.id=t2.id;
+--sleep 1
+connection addconroot2;
+create trigger t1_trg before update on t1 for each row
+  insert into t3 values (new.id);
+connection addconroot1;
+unlock tables;
+connection default;
+reap;
+select * from t1;
+select * from t3;
+
+# And even for multi-update converted from ordinary update thanks to view
+drop view v1;
+drop trigger t1_trg;
+create view v1 as select t1.id as id1 from t1, t2 where t1.id= t2.id;
+insert into t2 values (10);
+connection addconroot1;
+lock tables t2 write;
+connection default;
+send update v1 set id1= 11;
+--sleep 1
+connection addconroot2;
+create trigger t1_trg before update on t1 for each row
+  insert into t3 values (new.id + 100);
+connection addconroot1;
+unlock tables;
+connection default;
+reap;
+select * from t1;
+select * from t3;
+
+drop function f1;
+drop view v1;
+drop table t1, t2, t3;
+--enable_parsing

--- 1.27/sql/sql_trigger.cc	2005-08-15 19:15:04 +04:00
+++ 1.28/sql/sql_trigger.cc	2005-09-14 02:27:18 +04:00
@@ -511,6 +511,25 @@
 
 
 /*
+  Adjust Table_triggers_list with new TABLE pointer.
+
+  SYNOPSIS
+    set_table()
+      new_table - new pointer to TABLE instance
+*/
+
+void Table_triggers_list::set_table(TABLE *new_table)
+{
+  table= new_table;
+  for (Field **field= table->triggers->record1_field ; *field ; field++)
+  {
+    (*field)->table= (*field)->orig_table= new_table;
+    (*field)->table_name= &new_table->alias;
+  }
+}
+
+
+/*
   Check whenever .TRG file for table exist and load all triggers it contains.
 
   SYNOPSIS

--- 1.13/sql/sql_trigger.h	2005-08-15 19:15:05 +04:00
+++ 1.14/sql/sql_trigger.h	2005-09-14 02:27:18 +04:00
@@ -98,6 +98,8 @@
     return test(bodies[TRG_EVENT_UPDATE][TRG_ACTION_BEFORE]);
   }
 
+  void set_table(TABLE *new_table);
+
   friend class Item_trigger_field;
   friend void sp_cache_routines_and_add_tables_for_triggers(THD *thd, LEX *lex,
                 Table_triggers_list *triggers);

--- 1.92/sql/sp.cc	2005-09-02 17:21:07 +04:00
+++ 1.93/sql/sp.cc	2005-09-14 02:27:18 +04:00
@@ -138,7 +138,7 @@
     could lead to a deadlock if we have other tables opened.
   */
   if (!(thd->lock= mysql_lock_tables(thd, &table, 1,
-                                     MYSQL_LOCK_IGNORE_FLUSH)))
+                                     MYSQL_LOCK_IGNORE_FLUSH, 0)))
   {
     close_proc_table(thd, backup);
     DBUG_RETURN(0);
@@ -1265,7 +1265,8 @@
 
 
 /*
-  Add routine to the set of stored routines used by statement.
+  Add routine which is explicitly used by statement to the set of stored
+  routines used by this statement.
 
   SYNOPSIS
     sp_add_used_routine()
@@ -1276,7 +1277,8 @@
       rt_type - routine type (one of TYPE_ENUM_PROCEDURE/...)
 
   NOTES
-    Will also add element to end of 'LEX::sroutines_list' list.
+    Will also add element to end of 'LEX::sroutines_list' list (and will
+    take into account that this is explicitly used routine).
 
     To be friendly towards prepared statements one should pass
     persistent arena as second argument.
@@ -1287,6 +1289,30 @@
 {
   rt->set_routine_type(rt_type);
   (void)add_used_routine(lex, arena, &rt->m_sroutines_key);
+  lex->sroutines_list_own_last= lex->sroutines_list.next;
+  lex->sroutines_list_own_elements= lex->sroutines_list.elements;
+}
+
+
+/*
+  Remove routines which are only indirectly used by statement from
+  the set of routines used by this statement.
+
+  SYNOPSIS
+    sp_remove_not_own_routines()
+      lex  LEX representing statement
+*/
+
+void sp_remove_not_own_routines(LEX *lex)
+{
+  for (Sroutine_hash_entry *not_own_rt=
+         *(Sroutine_hash_entry **)lex->sroutines_list_own_last;
+       not_own_rt; not_own_rt= not_own_rt->next)
+    hash_delete(&lex->sroutines, (byte *)not_own_rt);
+
+  *(Sroutine_hash_entry **)lex->sroutines_list_own_last= NULL;
+  lex->sroutines_list.next= lex->sroutines_list_own_last;
+  lex->sroutines_list.elements= lex->sroutines_list_own_elements;
 }
 
 
@@ -1344,6 +1370,28 @@
 
 
 /*
+  Add contents of list representing set of routines to the set of
+  routines used by statement.
+
+  SYNOPSIS
+    sp_update_stmt_used_routines()
+      thd  Thread context
+      lex  LEX representing statement
+      src  List representing set from which routines will be added
+
+  NOTE
+    It will also add elements to end of 'LEX::sroutines_list' list.
+*/
+
+static void sp_update_stmt_used_routines(THD *thd, LEX *lex, SQL_LIST *src)
+{
+  for (Sroutine_hash_entry *rt= (Sroutine_hash_entry *)src->first;
+       rt; rt= rt->next)
+    (void)add_used_routine(lex, thd->stmt_arena, &rt->key);
+}
+
+
+/*
   Cache sub-set of routines used by statement, add tables used by these
   routines to statement table list. Do the same for all routines used
   by these routines.
@@ -1463,7 +1511,7 @@
 {
   Sroutine_hash_entry **last_cached_routine_ptr=
                           (Sroutine_hash_entry **)lex->sroutines_list.next;
-  sp_update_stmt_used_routines(thd, lex, &aux_lex->sroutines);
+  sp_update_stmt_used_routines(thd, lex, &aux_lex->sroutines_list);
   (void)sp_cache_routines_and_add_tables_aux(thd, lex, 
                                              *last_cached_routine_ptr, FALSE);
 }

--- 1.28/sql/sp.h	2005-08-12 04:04:11 +04:00
+++ 1.29/sql/sp.h	2005-09-14 02:27:18 +04:00
@@ -84,6 +84,7 @@
                             bool *first_no_prelocking);
 void sp_add_used_routine(LEX *lex, Query_arena *arena,
                          sp_name *rt, char rt_type);
+void sp_remove_not_own_routines(LEX *lex);
 void sp_update_sp_used_routines(HASH *dst, HASH *src);
 bool sp_cache_routines_and_add_tables(THD *thd, LEX *lex, 
                                       bool first_no_prelock);

--- 1.179/sql/sp_head.cc	2005-09-03 03:13:09 +04:00
+++ 1.180/sql/sp_head.cc	2005-09-14 02:27:18 +04:00
@@ -1887,10 +1887,6 @@
       attached it above in this function).
       Now we'll save the 'tail', and detach it.
     */
-    DBUG_ASSERT(!lex_query_tables_own_last ||
-                lex_query_tables_own_last == m_lex->query_tables_own_last &&
-                prelocking_tables == *(m_lex->query_tables_own_last));
-
     lex_query_tables_own_last= m_lex->query_tables_own_last;
     prelocking_tables= *lex_query_tables_own_last;
     *lex_query_tables_own_last= NULL;

--- 1.73/sql/sql_handler.cc	2005-08-08 17:46:01 +04:00
+++ 1.74/sql/sql_handler.cc	2005-09-14 02:27:18 +04:00
@@ -431,7 +431,7 @@
   protocol->send_fields(&list, Protocol::SEND_NUM_ROWS | Protocol::SEND_EOF);
 
   HANDLER_TABLES_HACK(thd);
-  lock= mysql_lock_tables(thd, &tables->table, 1, 0);
+  lock= mysql_lock_tables(thd, &tables->table, 1, 0, 0);
   HANDLER_TABLES_HACK(thd);
 
   if (!lock)

--- 1.151/sql/sql_prepare.cc	2005-09-03 03:13:10 +04:00
+++ 1.152/sql/sql_prepare.cc	2005-09-14 02:27:18 +04:00
@@ -1094,30 +1094,36 @@
 #ifndef NO_EMBEDDED_ACCESS_CHECKS
   uint          want_privilege;
 #endif
+  bool need_reopen;
   DBUG_ENTER("mysql_test_update");
 
   if (update_precheck(thd, table_list))
     goto error;
 
-  if (open_tables(thd, &table_list, &table_count, 0))
-    goto error;
-
-  if (table_list->multitable_view)
+  do
   {
-    DBUG_ASSERT(table_list->view != 0);
-    DBUG_PRINT("info", ("Switch to multi-update"));
-    /* pass counter value */
-    thd->lex->table_count= table_count;
-    /* convert to multiupdate */
-    DBUG_RETURN(2);
-  }
+    if (open_tables(thd, &table_list, &table_count, 0))
+      goto error;
+
+    if (table_list->multitable_view)
+    {
+      DBUG_ASSERT(table_list->view != 0);
+      DBUG_PRINT("info", ("Switch to multi-update"));
+      /* pass counter value */
+      thd->lex->table_count= table_count;
+      /* convert to multiupdate */
+      DBUG_RETURN(2);
+    }
+
+    if (lock_tables(thd, table_list, table_count, &need_reopen))
+      goto error;
+  } while (need_reopen);
 
   /*
     thd->fill_derived_tables() is false here for sure (because it is
     preparation of PS, so we even do not check it).
   */
-  if (lock_tables(thd, table_list, table_count) ||
-      mysql_handle_derived(thd->lex, &mysql_derived_prepare))
+  if (mysql_handle_derived(thd->lex, &mysql_derived_prepare))
     goto error;
 
 #ifndef NO_EMBEDDED_ACCESS_CHECKS
Thread
bk commit into 5.0 tree (dlenev:1.1951) BUG#12704dlenev14 Sep