------------------------------------------------------------

revno: 2617.68.23
committer: Dmitry Lenev <dlenev@mysql.com>
branch nick: mysql-next-bg-pre1
timestamp: Wed 2009-09-16 09:34:42 +0400
message:
  Pre-requisite patch for fixing bug #30977 "Concurrent statement
  using stored function and DROP FUNCTION breaks SBR".

  CREATE TABLE SELECT statements take exclusive metadata lock on table
  being created. Invariant of metadata locking subsystem states that
  such lock should be taken before taking any kind of shared locks.
  Once metadata locks on stored routines are introduced statements like
  "CREATE TABLE ... SELECT f1()" will break this invariant by taking
  shared locks on routines before exclusive lock on target table.
  To avoid this, open_tables() is reworked to process tables which are
  directly used by the statement before stored routines are processed.

sql/sql_base.cc:
  Refactored open_tables() implementation to process stored routines 
  only after tables which are directly used by statement were processed.
            
  To achieve this moved handling of routines in open_tables() out of 
  loop which iterates over tables to a new separate loop. And in its 
  turn this allowed to split handling of particular table or view to 
  an auxiliary function, which made code in open_tables() simpler and 
  more easy to understand.

sql/sql_base.cc

@@ -3753,16 +3753,13 @@ thr_lock_type read_lock_type_for_table(THD *thd, TABLE *table)
 
 
 /*
-  Perform steps of prelocking algorithm for elements of the
-  prelocking set other than tables. E.g. cache routines and, if
-  prelocking strategy prescribes so, extend the prelocking set with
-  tables and routines used by them.
+  Handle element of prelocking set other than table. E.g. cache routine
+  and, if prelocking strategy prescribes so, extend the prelocking set
+  with tables and routines used by it.
 
   @param[in]  thd                  Thread context.
   @param[in]  prelocking_ctx       Prelocking context.
-  @param[in]  start                First element in the list representing
-                                   subset of the prelocking set to be
-                                   processed.
+  @param[in]  rt                   Element of prelocking set to be processed.
   @param[in]  prelocking_strategy  Strategy which specifies how the
                                    prelocking set should be extended when
                                    one of its elements is processed.
@@ -3775,17 +3772,15 @@ thr_lock_type read_lock_type_for_table(THD *thd, TABLE *table)
 */
 
 static bool
-open_routines(THD *thd, Query_tables_list *prelocking_ctx,
-              Sroutine_hash_entry *start,
+open_and_process_routine(THD *thd, Query_tables_list *prelocking_ctx,
+                         Sroutine_hash_entry *rt,
                          Prelocking_strategy *prelocking_strategy,
                          bool *need_prelocking)
 {
-  DBUG_ENTER("open_routines");
-
-  for (Sroutine_hash_entry *rt= start; rt; rt= rt->next)
-  {
   int type= rt->key.str[0];
 
+  DBUG_ENTER("open_and_process_routine");
+
   switch (type)
   {
   case TYPE_ENUM_FUNCTION:
@@ -3810,116 +3805,47 @@ open_routines(THD *thd, Query_tables_list *prelocking_ctx,
     /* Impossible type value. */
     DBUG_ASSERT(0);
   }
-  }
   DBUG_RETURN(FALSE);
 }
 
 
 /**
-  Open all tables in list
+  Handle table list element by obtaining metadata lock, opening table or view
+  and, if prelocking strategy prescribes so, extending the prelocking set with
+  tables and routines used by it.
 
   @param[in]     thd                  Thread context.
-  @param[in,out] start    List of tables to be open (it can be adjusted for
-                          statement that uses tables only implicitly, e.g.
-                          for "SELECT f1()").
-  @param[out]    counter  Number of tables which were open.
-  @param[in]     flags    Bitmap of flags to modify how the tables will be
-                          open, see open_table() description for details.
-  @param[in]     prelocking_strategy  Strategy which specifies how prelocking
-                                      algorithm should work for this statement.
-
-  @note
-    Unless we are already in prelocked mode and prelocking strategy prescribes
-    so this function will also precache all SP/SFs explicitly or implicitly
-    (via views and triggers) used by the query and add tables needed for their
-    execution to table list. Statement that uses SFs, invokes triggers or
-    requires foreign key checks will be marked as requiring prelocking.
-    Prelocked mode will be enabled for such query during lock_tables() call.
-
-    If query for which we are opening tables is already marked as requiring
-    prelocking it won't do such precaching and will simply reuse table list
-    which is already built.
+  @param[in]     lex                  LEX structure for statement.
+  @param[in]     tables               Table list element to be processed.
+  @param[in,out] counter              Number of tables which are open.
+  @param[in]     flags                Bitmap of flags to modify how the tables
+                                      will be open, see open_table() description
+                                      for details.
+  @param[in]     prelocking_strategy  Strategy which specifies how the
+                                      prelocking set should be extended
+                                      when table or view is processed.
+  @param[in]     has_prelocking_list  Indicates that prelocking set/list for
+                                      this statement has already been built.
+  @param[in]     ot_ctx               Context used to recover from a failed
+                                      open_table() attempt.
+  @param[in]     new_frm_mem          Temporary MEM_ROOT to be used for
+                                      parsing .FRMs for views.
 
   @retval  FALSE  Success.
-  @retval  TRUE   Error, reported.
+  @retval  TRUE   Error, reported unless there is a chance to recover from it.
 */
 
-bool open_tables(THD *thd, TABLE_LIST **start, uint *counter, uint flags,
-                Prelocking_strategy *prelocking_strategy)
+static bool
+open_and_process_table(THD *thd, LEX *lex, TABLE_LIST *tables,
+                       uint *counter, uint flags,
+                       Prelocking_strategy *prelocking_strategy,
+                       bool has_prelocking_list,
+                       Open_table_context *ot_ctx,
+                       MEM_ROOT *new_frm_mem)
 {
-  TABLE_LIST *tables= NULL;
-  Open_table_context ot_ctx(thd);
   bool error= FALSE;
-  MEM_ROOT new_frm_mem;
-  /* Also used for indicating that prelocking is need */
-  TABLE_LIST **query_tables_last_own;
-  bool safe_to_ignore_table;
-  DBUG_ENTER("open_tables");
-  /*
-    temporary mem_root for new .frm parsing.
-    TODO: variables for size
-  */
-  init_sql_alloc(&new_frm_mem, 8024, 8024);
-
-  thd->current_tablenr= 0;
- restart:
-  *counter= 0;
-  query_tables_last_own= 0;
-  thd_proc_info(thd, "Opening tables");
-
-  /*
-    Close HANDLER tables which are marked for flush or against which there
-    are pending exclusive metadata locks. Note that we do this not to avoid
-    deadlocks (calls to mysql_ha_flush() in mdl_wait_for_locks() and
-    tdc_wait_for_old_version() are enough for this) but in order to have
-    a point during statement execution at which such HANDLERs are closed
-    even if they don't create problems for current thread (i.e. to avoid
-    having DDL blocked by HANDLERs opened for long time).
-  */
-  if (thd->handler_tables)
-    mysql_ha_flush(thd);
-
-  /*
-    If we are not already executing prelocked statement and don't have
-    statement for which table list for prelocking is already built, let
-    us cache routines and try to build such table list.
-  */
-
-  if (thd->locked_tables_mode <= LTM_LOCK_TABLES &&
-      !thd->lex->requires_prelocking() &&
-      thd->lex->uses_stored_routines())
-  {
-    bool need_prelocking= FALSE;
-    TABLE_LIST **save_query_tables_last= thd->lex->query_tables_last;
-
-    DBUG_ASSERT(thd->lex->query_tables == *start);
-
-    error= open_routines(thd, thd->lex,
-                         (Sroutine_hash_entry *)thd->lex->sroutines_list.first,
-                         prelocking_strategy, &need_prelocking);
-    if (error)
-    {
-      /*
-        Serious error during reading stored routines from mysql.proc table.
-        Something's wrong with the table or its contents, and an error has
-        been emitted; we must abort.
-      */
-      goto err;
-    }
-    else if (need_prelocking)
-    {
-      query_tables_last_own= save_query_tables_last;
-      *start= thd->lex->query_tables;
-    }
-  }
-
-  /*
-    For every table in the list of tables to open, try to find or open
-    a table.
-  */
-  for (tables= *start; tables ;tables= tables->next_global)
-  {
-    safe_to_ignore_table= FALSE;
+  bool safe_to_ignore_table= FALSE;
+  DBUG_ENTER("open_and_process_table");
 
   /*
     Ignore placeholders for derived tables. After derived tables
@@ -3930,7 +3856,7 @@ bool open_tables(THD *thd, TABLE_LIST **start, uint *counter, uint flags,
   if (tables->derived)
   {
     if (!tables->view)
-        continue;
+      goto end;
     /*
       We restore view's name and database wiped out by derived tables
       processing and fall back to standard open process in order to
@@ -3957,15 +3883,16 @@ bool open_tables(THD *thd, TABLE_LIST **start, uint *counter, uint flags,
     */
     if (tables->view)
       goto process_view_routines;
-      if (!mysql_schema_table(thd, thd->lex, tables) &&
+    if (!mysql_schema_table(thd, lex, tables) &&
         !check_and_update_table_version(thd, tables, tables->table->s))
     {
-        continue;
+      goto end;
     }
-      DBUG_RETURN(TRUE);
+    error= TRUE;
+    goto end;
   }
-    DBUG_PRINT("tcache", ("opening table: '%s'.'%s'  item: 0x%lx",
-                          tables->db, tables->table_name, (long) tables));
+  DBUG_PRINT("tcache", ("opening table: '%s'.'%s'  item: %p",
+                        tables->db, tables->table_name, tables)); //psergey: invalid read of size 1 here
   (*counter)++;
 
   /* Not a placeholder: must be a base table or a view. Let us open it. */
@@ -3981,56 +3908,24 @@ bool open_tables(THD *thd, TABLE_LIST **start, uint *counter, uint flags,
     */
     Prelock_error_handler prelock_handler;
     thd->push_internal_handler(& prelock_handler);
-      error= open_table(thd, tables, &new_frm_mem, &ot_ctx, flags);
+    error= open_table(thd, tables, new_frm_mem, ot_ctx, flags);
     thd->pop_internal_handler();
     safe_to_ignore_table= prelock_handler.safely_trapped_errors();
   }
   else
-      error= open_table(thd, tables, &new_frm_mem, &ot_ctx, flags);
+    error= open_table(thd, tables, new_frm_mem, ot_ctx, flags);
 
-    free_root(&new_frm_mem, MYF(MY_KEEP_PREALLOC));
+  free_root(new_frm_mem, MYF(MY_KEEP_PREALLOC));
 
   if (error)
   {
-      if (ot_ctx.can_recover_from_failed_open_table())
-      {
-        /*
-          We have met exclusive metadata lock or old version of table. Now we
-          have to close all tables which are not up to date/release metadata
-          locks. We also have to throw away set of prelocked tables (and thus
-          close tables from this set that were open by now) since it possible
-          that one of tables which determined its content was changed.
-
-          Instead of implementing complex/non-robust logic mentioned
-          above we simply close and then reopen all tables.
-
-          In order to prepare for recalculation of set of prelocked tables
-          we pretend that we have finished calculation which we were doing
-          currently.
-        */
-        if (query_tables_last_own)
-          thd->lex->mark_as_requiring_prelocking(query_tables_last_own);
-        close_tables_for_reopen(thd, start);
-        /*
-          Here we rely on the fact that 'tables' still points to the valid
-          TABLE_LIST element. Altough currently this assumption is valid
-          it may change in future.
-        */
-        if (ot_ctx.recover_from_failed_open_table_attempt(thd, tables))
-          goto err;
-
-        error= FALSE;
-	goto restart;
-      }
-
-      if (safe_to_ignore_table)
+    if (! ot_ctx->can_recover_from_failed_open_table() && safe_to_ignore_table)
     {
       DBUG_PRINT("info", ("open_table: ignoring table '%s'.'%s'",
                           tables->db, tables->alias));
       error= FALSE;
-        continue;
     }
-      goto err;
+    goto end;
   }
 
   /*
@@ -4055,9 +3950,9 @@ bool open_tables(THD *thd, TABLE_LIST **start, uint *counter, uint flags,
       has added its base tables after itself, adjust the boundary pointer
       accordingly.
     */
-      if (query_tables_last_own == &(tables->next_global) &&
+    if (lex->query_tables_own_last == &(tables->next_global) &&
         tables->view->query_tables)
-        query_tables_last_own= tables->view->query_tables_last;
+      lex->query_tables_own_last= tables->view->query_tables_last;
     /*
       Let us free memory used by 'sroutines' hash here since we never
       call destructor for this LEX.
@@ -4071,7 +3966,7 @@ bool open_tables(THD *thd, TABLE_LIST **start, uint *counter, uint flags,
     TABLE_LIST::table to anything.
   */
   if (tables->open_strategy && !tables->table)
-      continue;
+    goto end;
 
   /*
     If we are not already in prelocked mode and extended table list is not
@@ -4082,15 +3977,11 @@ bool open_tables(THD *thd, TABLE_LIST **start, uint *counter, uint flags,
     to be changed.
   */
   if (thd->locked_tables_mode <= LTM_LOCK_TABLES &&
-        !thd->lex->requires_prelocking() &&
+      ! has_prelocking_list &&
       tables->lock_type >= TL_WRITE_ALLOW_WRITE)
   {
     bool need_prelocking= FALSE;
-      bool not_used;
-      TABLE_LIST **save_query_tables_last= thd->lex->query_tables_last;
-      Sroutine_hash_entry **sroutines_next= 
-        (Sroutine_hash_entry **)thd->lex->sroutines_list.next;
-
+    TABLE_LIST **save_query_tables_last= lex->query_tables_last;
     /*
       Extend statement's table list and the prelocking set with
       tables and routines according to the current prelocking
@@ -4100,27 +3991,14 @@ bool open_tables(THD *thd, TABLE_LIST **start, uint *counter, uint flags,
       used by triggers which are going to be invoked for this element of
       table list and also add tables required for handling of foreign keys.
     */
-      error= prelocking_strategy->handle_table(thd, thd->lex, tables,
+    error= prelocking_strategy->handle_table(thd, lex, tables,
                                              &need_prelocking);
 
-      if (need_prelocking && ! query_tables_last_own)
-        query_tables_last_own= save_query_tables_last;
-
-      if (error)
-        goto err;
-
-      /*
-        Process elements of the prelocking set which were added
-        by the above invocation of Prelocking_strategy method.
+    if (need_prelocking && ! lex->requires_prelocking())
+      lex->mark_as_requiring_prelocking(save_query_tables_last);
 
-        For example, if new element is a routine, cache it and then, if
-        prelocking strategy prescribes so, add tables it uses to the table
-        list and routines it might invoke to the prelocking set.
-      */
-      error= open_routines(thd, thd->lex, *sroutines_next, prelocking_strategy,
-                           &not_used);
     if (error)
-        goto err;
+      goto end;
   }
 
   if (tables->lock_type != TL_UNLOCK && ! thd->locked_tables_mode)
@@ -4139,7 +4017,7 @@ bool open_tables(THD *thd, TABLE_LIST **start, uint *counter, uint flags,
   error= check_and_update_table_version(thd, tables, tables->table->s);
 
   if (error)
-      goto err;
+    goto end;
   /*
     After opening a MERGE table add the children to the query list of
     tables, so that they are opened too.
@@ -4151,7 +4029,7 @@ bool open_tables(THD *thd, TABLE_LIST **start, uint *counter, uint flags,
   if (tables->table->file->extra(HA_EXTRA_ADD_CHILDREN_LIST))
   {
     error= TRUE;
-      goto err;
+    goto end;
   }
 
 process_view_routines:
@@ -4161,25 +4039,179 @@ bool open_tables(THD *thd, TABLE_LIST **start, uint *counter, uint flags,
   */
   if (tables->view &&
       thd->locked_tables_mode <= LTM_LOCK_TABLES &&
-        !thd->lex->requires_prelocking())
+      ! has_prelocking_list)
   {
     bool need_prelocking= FALSE;
-      bool not_used;
-      TABLE_LIST **save_query_tables_last= thd->lex->query_tables_last;
-      Sroutine_hash_entry **sroutines_next= 
-        (Sroutine_hash_entry **)thd->lex->sroutines_list.next;
+    TABLE_LIST **save_query_tables_last= lex->query_tables_last;
 
-      error= prelocking_strategy->handle_view(thd, thd->lex, tables,
+    error= prelocking_strategy->handle_view(thd, lex, tables,
                                             &need_prelocking);
 
-      if (need_prelocking && ! query_tables_last_own)
-        query_tables_last_own= save_query_tables_last;
+    if (need_prelocking && ! lex->requires_prelocking())
+      lex->mark_as_requiring_prelocking(save_query_tables_last);
+
+    if (error)
+      goto end;
+  }
+
+end:
+  DBUG_RETURN(error);
+}
+
+
+/**
+  Open all tables in list
+
+  @param[in]     thd      Thread context.
+  @param[in,out] start    List of tables to be open (it can be adjusted for
+                          statement that uses tables only implicitly, e.g.
+                          for "SELECT f1()").
+  @param[out]    counter  Number of tables which were open.
+  @param[in]     flags    Bitmap of flags to modify how the tables will be
+                          open, see open_table() description for details.
+  @param[in]     prelocking_strategy  Strategy which specifies how prelocking
+                                      algorithm should work for this statement.
+
+  @note
+    Unless we are already in prelocked mode and prelocking strategy prescribes
+    so this function will also precache all SP/SFs explicitly or implicitly
+    (via views and triggers) used by the query and add tables needed for their
+    execution to table list. Statement that uses SFs, invokes triggers or
+    requires foreign key checks will be marked as requiring prelocking.
+    Prelocked mode will be enabled for such query during lock_tables() call.
+
+    If query for which we are opening tables is already marked as requiring
+    prelocking it won't do such precaching and will simply reuse table list
+    which is already built.
+
+  @retval  FALSE  Success.
+  @retval  TRUE   Error, reported.
+*/
+
+bool open_tables(THD *thd, TABLE_LIST **start, uint *counter, uint flags,
+                Prelocking_strategy *prelocking_strategy)
+{
+  /*
+    We use pointers to "next_global" member in the last processed TABLE_LIST
+    element and to the "next" member in the last processed Sroutine_hash_entry
+    element as iterators over, correspondingly, the table list and stored routines
+    list which stay valid and allow to continue iteration when new elements are
+    added to the tail of the lists.
+  */
+  TABLE_LIST **table_to_open;
+  Sroutine_hash_entry **sroutine_to_open;
+  TABLE_LIST *tables;
+  Open_table_context ot_ctx(thd);
+  bool error= FALSE;
+  MEM_ROOT new_frm_mem;
+  bool has_prelocking_list= thd->lex->requires_prelocking();
+  DBUG_ENTER("open_tables");
+  /*
+    Close HANDLER tables which are marked for flush or against which there
+    are pending exclusive metadata locks. Note that we do this not to avoid
+    deadlocks (calls to mysql_ha_flush() in mdl_wait_for_locks() and
+    tdc_wait_for_old_version() are enough for this) but in order to have
+    a point during statement execution at which such HANDLERs are closed
+    even if they don't create problems for current thread (i.e. to avoid
+    having DDL blocked by HANDLERs opened for long time).
+  */
+  if (thd->handler_tables)
+    mysql_ha_flush(thd);
+
+  /*
+    temporary mem_root for new .frm parsing.
+    TODO: variables for size
+  */
+  init_sql_alloc(&new_frm_mem, 8024, 8024);
+
+  thd->current_tablenr= 0;
+ restart:
+  table_to_open= start;
+  sroutine_to_open= (Sroutine_hash_entry**) &thd->lex->sroutines_list.first;
+  *counter= 0;
+  thd_proc_info(thd, "Opening tables");
+
+  /*
+    Perform steps of prelocking algorithm until there are unprocessed
+    elements in prelocking list/set.
+  */
+  while (*table_to_open  ||
+         (thd->locked_tables_mode <= LTM_LOCK_TABLES &&
+          ! has_prelocking_list &&
+          *sroutine_to_open))
+  {
+    /*
+      For every table in the list of tables to open, try to find or open
+      a table.
+    */
+    for (tables= *table_to_open; tables;
+         table_to_open= &tables->next_global, tables= tables->next_global)
+    {
+      error= open_and_process_table(thd, thd->lex, tables, counter,
+                                    flags, prelocking_strategy,
+                                    has_prelocking_list, &ot_ctx,
+                                    &new_frm_mem);
 
       if (error)
+      {
+        if (ot_ctx.can_recover_from_failed_open_table())
+        {
+          /*
+            We have met exclusive metadata lock or old version of table.
+            Now we have to close all tables and release metadata locks.
+            We also have to throw away set of prelocked tables (and thus
+            close tables from this set that were open by now) since it
+            is possible that one of tables which determined its content
+            was changed.
+
+            Instead of implementing complex/non-robust logic mentioned
+            above we simply close and then reopen all tables.
+
+            We have to save pointer to table list element for table which we
+            have failed to open since closing tables can trigger removal of
+            elements from the table list (if MERGE tables are involved),
+          */
+          TABLE_LIST *failed_table= *table_to_open;
+          close_tables_for_reopen(thd, start);
+          /*
+            Here we rely on the fact that 'tables' still points to the valid
+            TABLE_LIST element. Altough currently this assumption is valid
+            it may change in future.
+          */
+          if (ot_ctx.recover_from_failed_open_table_attempt(thd, failed_table))
             goto err;
 
-      error= open_routines(thd, thd->lex, *sroutines_next, prelocking_strategy,
-                           &not_used);
+          error= FALSE;
+          goto restart;
+        }
+        goto err;
+      }
+    }
+
+    /*
+      If we are not already in prelocked mode and extended table list is
+      not yet built for our statement we need to cache routines it uses
+      and build the prelocking list for it.
+    */
+    if (thd->locked_tables_mode <= LTM_LOCK_TABLES && ! has_prelocking_list)
+    {
+      bool need_prelocking= FALSE;
+      TABLE_LIST **save_query_tables_last= thd->lex->query_tables_last;
+      /*
+        Process elements of the prelocking set which are present there
+        since parsing stage or were added to it by invocations of
+        Prelocking_strategy methods in the above loop over tables.
+
+        For example, if element is a routine, cache it and then,
+        if prelocking strategy prescribes so, add tables it uses to the
+        table list and routines it might invoke to the prelocking set.
+      */
+      for (Sroutine_hash_entry *rt= *sroutine_to_open; rt;
+           sroutine_to_open= &rt->next, rt= rt->next)
+      {
+        error= open_and_process_routine(thd, thd->lex, rt,
+                                        prelocking_strategy,
+                                        &need_prelocking);
 
         if (error)
         {
@@ -4191,6 +4223,13 @@ bool open_tables(THD *thd, TABLE_LIST **start, uint *counter, uint flags,
           goto err;
         }
       }
+
+      if (need_prelocking && ! thd->lex->requires_prelocking())
+        thd->lex->mark_as_requiring_prelocking(save_query_tables_last);
+
+      if (need_prelocking && ! *start)
+        *start= thd->lex->query_tables;
+    }
   }
 
   /*
@@ -4220,12 +4259,9 @@ bool open_tables(THD *thd, TABLE_LIST **start, uint *counter, uint flags,
   thd_proc_info(thd, 0);
   free_root(&new_frm_mem, MYF(0));              // Free pre-alloced block
 
-  if (query_tables_last_own)
-    thd->lex->mark_as_requiring_prelocking(query_tables_last_own);
-
-  if (error && tables)
+  if (error && *table_to_open)
   {
-    tables->table= NULL;
+    (*table_to_open)->table= NULL;
   }
   DBUG_PRINT("open_tables", ("returning: %d", (int) error));
   DBUG_RETURN(error);