Backport of:

------------------------------------------------------------
revno: 2630.4.16
committer: Dmitry Lenev <dlenev@mysql.com>
branch nick: mysql-6.0-3726-w
timestamp: Thu 2008-05-29 09:45:02 +0400
message:
  WL#3726 "DDL locking for all metadata objects".

  After review changes in progress.

  Tweaked some comments and did some renames to
  avoid ambiguites.


sql/mysql_priv.h:
  Removed name_lock_locked_table() function.
sql/sql_base.cc:
  Got rid of name_lock_locked_table() function after replacing
  the only call to it with its body.
  Simplified open_table() code by making "action" argument
  mandatory (i.e. one now should always pass non-0 pointer
  in this argument).
  Renamed TABLE_LIST::open_table_type to open_type to
  avoid confusing it with type of table.
  Adjusted comments according to review.
sql/sql_handler.cc:
  Added comment clarifying in which cases we can have TABLE::mdl_lock
  set to 0.
sql/sql_insert.cc:
  Now the 4th argument of open_table() is mandatory (it makes
  no sense to complicate open_table() code when we can simply
  pass dummy variable).
sql/sql_parse.cc:
  Renamed TABLE_LIST::open_table_type to open_type to
  avoid confusing it with type of table.
sql/sql_prepare.cc:
  Renamed TABLE_LIST::open_table_type to open_type to
  avoid confusing it with type of table.
sql/sql_table.cc:
  Now the 4th argument of open_table() is mandatory (it makes
  no sense to complicate open_table() code when we can simply
  pass dummy variable).
sql/sql_trigger.cc:
  Replaced the only call to name_lock_locked_table() function
  with its body.
sql/sql_view.cc:
  Renamed TABLE_LIST::open_table_type to open_type to
  avoid confusing it with type of table.
sql/table.h:
  Renamed TABLE_LIST::open_table_type to open_type (to
  avoid confusing it with type of table) and improved
  comments describing this member.

10 files changed, 93 insertions, 94 deletions

diff --git a/sql/mysql_priv.h b/sql/mysql_priv.h
index 1028c7d0d29..3e3d3b6df24 100644
--- a/sql/mysql_priv.h
+++ b/sql/mysql_priv.h
@@ -1227,7 +1227,6 @@ TABLE *open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT* mem,
 bool tdc_open_view(THD *thd, TABLE_LIST *table_list, const char *alias,
                    char *cache_key, uint cache_key_length,
                    MEM_ROOT *mem_root, uint flags);
-bool name_lock_locked_table(THD *thd, TABLE_LIST *tables);
 bool reopen_name_locked_table(THD* thd, TABLE_LIST* table_list);
 TABLE *find_locked_table(TABLE *list, const char *db, const char *table_name);
 TABLE *find_write_locked_table(TABLE *list, const char *db,
diff --git a/sql/sql_base.cc b/sql/sql_base.cc
index b21a9f3b931..13218f3a193 100644
--- a/sql/sql_base.cc
+++ b/sql/sql_base.cc
@@ -2340,39 +2340,6 @@ void wait_for_condition(THD *thd, pthread_mutex_t *mutex, pthread_cond_t *cond)
 }
 
 
-/**
-  Exclusively name-lock a table that is already write-locked by the
-  current thread.
-
-  @param thd current thread context
-  @param tables table list containing one table to open.
-
-  @return FALSE on success, TRUE otherwise.
-*/
-
-bool name_lock_locked_table(THD *thd, TABLE_LIST *tables)
-{
-  bool result= TRUE;
-
-  DBUG_ENTER("name_lock_locked_table");
-
-  /* Under LOCK TABLES we must only accept write locked tables. */
-  tables->table= find_write_locked_table(thd->open_tables, tables->db,
-                                         tables->table_name);
-
-  if (tables->table)
-  {
-    /*
-      Ensures that table is opened only by this thread and that no
-      other statement will open this table.
-    */
-    result= wait_while_table_is_used(thd, tables->table, HA_EXTRA_FORCE_REOPEN);
-  }
-
-  DBUG_RETURN(result);
-}
-
-
 /*
   Open table for which this thread has exclusive meta-data lock.
 
@@ -2576,9 +2543,7 @@ TABLE *open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT *mem_root,
   /* Parsing of partitioning information from .frm needs thd->lex set up. */
   DBUG_ASSERT(thd->lex->is_lex_started);
 
-  /* find a unused table in the open table cache */
-  if (action)
-    *action= OT_NO_ACTION;
+  *action= OT_NO_ACTION;
 
   /* an open table operation needs a lot of the stack space */
   if (check_stack_overrun(thd, STACK_MIN_SIZE_FOR_OPEN, (uchar *)&alias))
@@ -2716,6 +2681,13 @@ TABLE *open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT *mem_root,
       enum legacy_db_type not_used;
       build_table_filename(path, sizeof(path) - 1,
                            table_list->db, table_list->table_name, reg_ext, 0);
+      /*
+        Note that we can't be 100% sure that it is a view since it's
+        possible that we either simply have not found unused TABLE
+        instance in THD::open_tables list or were unable to open table
+        during prelocking process (in this case in theory we still
+        should hold shared metadata lock on it).
+      */
       if (mysql_frm_type(thd, path, &not_used) == FRMTYPE_VIEW)
       {
         if (!tdc_open_view(thd, table_list, alias, key, key_length,
@@ -2741,28 +2713,25 @@ TABLE *open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT *mem_root,
   }
 
   /*
-    Non pre-locked/LOCK TABLES mode, and the table is not temporary:
-    this is the normal use case.
-    Now we should:
-    - try to find the table in the table cache.
-    - if one of the discovered TABLE instances is name-locked
-      (table->s->version == 0) or some thread has started FLUSH TABLES
-      (refresh_version > table->s->version), back off -- we have to wait
-      until no one holds a name lock on the table.
-    - if there is no such TABLE in the name cache, read the table definition
-    and insert it into the cache.
-    We perform all of the above under LOCK_open which currently protects
-    the open cache (also known as table cache) and table definitions stored
-    on disk.
+    Non pre-locked/LOCK TABLES mode, and the table is not temporary.
+    This is the normal use case.
   */
 
   mdl_lock= table_list->mdl_lock;
   mdl_add_lock(&thd->mdl_context, mdl_lock);
 
-  if (table_list->open_table_type)
+  if (table_list->open_type)
   {
+    /*
+      In case of CREATE TABLE .. If NOT EXISTS .. SELECT, the table
+      may not yet exist. Let's acquire an exclusive lock for that
+      case. If later it turns out the table existsed, we will
+      downgrade the lock to shared. Note that, according to the
+      locking protocol, all exclusive locks must be acquired before
+      shared locks. This invariant is preserved here and is also
+      enforced by asserts in metadata locking subsystem.
+    */
     mdl_set_lock_type(mdl_lock, MDL_EXCLUSIVE);
-    /* TODO: This case can be significantly optimized. */
     if (mdl_acquire_exclusive_locks(&thd->mdl_context))
       DBUG_RETURN(0);
   }
@@ -2776,7 +2745,7 @@ TABLE *open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT *mem_root,
                                     MDL_HIGH_PRIO : MDL_NORMAL_PRIO);
     if (mdl_acquire_shared_lock(mdl_lock, &retry))
     {
-      if (action && retry)
+      if (retry)
         *action= OT_BACK_OFF_AND_RETRY;
       DBUG_RETURN(0);
     }
@@ -2798,13 +2767,12 @@ TABLE *open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT *mem_root,
            ! (flags & MYSQL_LOCK_IGNORE_FLUSH))
   {
     /* Someone did a refresh while thread was opening tables */
-    if (action)
-      *action= OT_BACK_OFF_AND_RETRY;
+    *action= OT_BACK_OFF_AND_RETRY;
     pthread_mutex_unlock(&LOCK_open);
     DBUG_RETURN(0);
   }
 
-  if (table_list->open_table_type == TABLE_LIST::OPEN_OR_CREATE)
+  if (table_list->open_type == TABLE_LIST::OPEN_OR_CREATE)
   {
     bool exists;
 
@@ -2818,7 +2786,7 @@ TABLE *open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT *mem_root,
     }
     /* Table exists. Let us try to open it. */
   }
-  else if (table_list->open_table_type == TABLE_LIST::TAKE_EXCLUSIVE_MDL)
+  else if (table_list->open_type == TABLE_LIST::TAKE_EXCLUSIVE_MDL)
   {
     pthread_mutex_unlock(&LOCK_open);
     DBUG_RETURN(0);
@@ -2926,8 +2894,17 @@ TABLE *open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT *mem_root,
   {
     if (!(flags & MYSQL_LOCK_IGNORE_FLUSH))
     {
-      if (action)
-        *action= OT_BACK_OFF_AND_RETRY;
+       /*
+         We already have an MDL lock. But we have encountered an old
+         version of table in the table definition cache which is possible
+         when someone changes the table version directly in the cache
+         without acquiring a metadata lock (e.g. this can happen during
+         "rolling" FLUSH TABLE(S)).
+         Note, that to avoid a "busywait" in this case, we have to wait
+         separately in the caller for old table versions to go away
+         (see tdc_wait_for_old_versions()).
+       */
+      *action= OT_BACK_OFF_AND_RETRY;
       release_table_share(share);
       pthread_mutex_unlock(&LOCK_open);
       DBUG_RETURN(0);
@@ -2966,18 +2943,15 @@ TABLE *open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT *mem_root,
     {
       my_free(table, MYF(0));
 
-      if (action)
+      if (error == 7)
       {
-        if (error == 7)
-        {
-          share->version= 0;
-          *action= OT_DISCOVER;
-        }
-        else if (share->crashed)
-        {
-          share->version= 0;
-          *action= OT_REPAIR;
-        }
+        share->version= 0;
+        *action= OT_DISCOVER;
+      }
+      else if (share->crashed)
+      {
+        share->version= 0;
+        *action= OT_REPAIR;
       }
 
       goto err_unlock;
@@ -2996,16 +2970,19 @@ TABLE *open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT *mem_root,
 
   pthread_mutex_unlock(&LOCK_open);
 
-  // Table existed
-  if (table_list->open_table_type == TABLE_LIST::OPEN_OR_CREATE)
+  /*
+    In CREATE TABLE .. If NOT EXISTS .. SELECT we have found that
+    table exists now we should downgrade our exclusive metadata
+    lock on this table to shared metadata lock.
+  */
+  if (table_list->open_type == TABLE_LIST::OPEN_OR_CREATE)
     mdl_downgrade_exclusive_locks(&thd->mdl_context);
 
   table->mdl_lock= mdl_lock;
-  if (action)
-  {
-    table->next=thd->open_tables;		/* Link into simple list */
-    thd->open_tables=table;
-  }
+
+  table->next=thd->open_tables;		/* Link into simple list */
+  thd->open_tables=table;
+
   table->reginfo.lock_type=TL_READ;		/* Assume read */
 
  reset:
@@ -3856,8 +3833,8 @@ err:
 
 
 /**
-   Auxiliary routine which finalizes process of TABLE object creation
-   by loading triggers and handling implicitly emptied tables.
+   Finalize the process of TABLE creation by loading table triggers
+   and taking action if a HEAP table content was emptied implicitly.
 */
 
 static bool open_table_entry_fini(THD *thd, TABLE_SHARE *share, TABLE *entry)
@@ -4636,7 +4613,7 @@ int open_tables(THD *thd, TABLE_LIST **start, uint *counter, uint flags)
         table and successful table creation.
         ...
       */
-      if (tables->open_table_type)
+      if (tables->open_type)
         continue;
 
       if (action)
diff --git a/sql/sql_handler.cc b/sql/sql_handler.cc
index 6087b0b7700..87e9538b48f 100644
--- a/sql/sql_handler.cc
+++ b/sql/sql_handler.cc
@@ -798,6 +798,7 @@ void mysql_ha_flush(THD *thd)
   for (uint i= 0; i < thd->handler_tables_hash.records; i++)
   {
     hash_tables= (TABLE_LIST*) my_hash_element(&thd->handler_tables_hash, i);
+    /* TABLE::mdl_lock is 0 for temporary tables so we need extra check. */
     if (hash_tables->table &&
         (hash_tables->table->mdl_lock &&
          mdl_has_pending_conflicting_lock(hash_tables->table->mdl_lock) ||
diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc
index 5da9f2e6bd4..a0c198f3196 100644
--- a/sql/sql_insert.cc
+++ b/sql/sql_insert.cc
@@ -3453,6 +3453,7 @@ static TABLE *create_table_from_items(THD *thd, HA_CREATE_INFO *create_info,
   Item *item;
   Field *tmp_field;
   bool not_used;
+  enum_open_table_action not_used2;
   DBUG_ENTER("create_table_from_items");
 
   tmp_table.alias= 0;
@@ -3544,8 +3545,7 @@ static TABLE *create_table_from_items(THD *thd, HA_CREATE_INFO *create_info,
       }
       else
       {
-        if (!(table= open_table(thd, create_table, thd->mem_root,
-                                (enum_open_table_action*) 0,
+        if (!(table= open_table(thd, create_table, thd->mem_root, &not_used2,
                                 MYSQL_OPEN_TEMPORARY_ONLY)) &&
             !create_info->table_existed)
         {
diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc
index 2b2c736fd9e..2c043922cc8 100644
--- a/sql/sql_parse.cc
+++ b/sql/sql_parse.cc
@@ -2631,7 +2631,7 @@ case SQLCOM_PREPARE:
       if (!(create_info.options & HA_LEX_CREATE_TMP_TABLE))
       {
         lex->link_first_table_back(create_table, link_to_local);
-        create_table->open_table_type= TABLE_LIST::OPEN_OR_CREATE;
+        create_table->open_type= TABLE_LIST::OPEN_OR_CREATE;
       }
 
       if (!(res= open_and_lock_tables(thd, lex->query_tables)))
diff --git a/sql/sql_prepare.cc b/sql/sql_prepare.cc
index 582e18a3abf..5efa0cea7a9 100644
--- a/sql/sql_prepare.cc
+++ b/sql/sql_prepare.cc
@@ -1673,7 +1673,7 @@ static bool mysql_test_create_table(Prepared_statement *stmt)
     if (!(lex->create_info.options & HA_LEX_CREATE_TMP_TABLE))
     {
       lex->link_first_table_back(create_table, link_to_local);
-      create_table->open_table_type= TABLE_LIST::OPEN_OR_CREATE;
+      create_table->open_type= TABLE_LIST::OPEN_OR_CREATE;
     }
 
     if (open_normal_and_derived_tables(stmt->thd, lex->query_tables, 0))
diff --git a/sql/sql_table.cc b/sql/sql_table.cc
index 649ba28bcac..9888dceef54 100644
--- a/sql/sql_table.cc
+++ b/sql/sql_table.cc
@@ -7208,12 +7208,13 @@ view_err:
   {
     if (table->s->tmp_table)
     {
+      enum_open_table_action not_used;
       TABLE_LIST tbl;
       bzero((void*) &tbl, sizeof(tbl));
       tbl.db= new_db;
       tbl.table_name= tbl.alias= tmp_name;
       /* Table is in thd->temporary_tables */
-      new_table= open_table(thd, &tbl, thd->mem_root, (enum_open_table_action*) 0,
+      new_table= open_table(thd, &tbl, thd->mem_root, &not_used,
                             MYSQL_LOCK_IGNORE_FLUSH);
     }
     else
diff --git a/sql/sql_trigger.cc b/sql/sql_trigger.cc
index a7a4d48b593..4e2b77292d8 100644
--- a/sql/sql_trigger.cc
+++ b/sql/sql_trigger.cc
@@ -446,8 +446,17 @@ bool mysql_create_or_drop_trigger(THD *thd, TABLE_LIST *tables, bool create)
 
   if (thd->locked_tables)
   {
-    if (name_lock_locked_table(thd, tables))
+    /* Under LOCK TABLES we must only accept write locked tables. */
+    if (!(tables->table= find_write_locked_table(thd->open_tables, tables->db,
+                                                 tables->table_name)))
       goto end;
+    /*
+      Ensure that table is opened only by this thread and that no other
+      statement will open this table.
+    */
+    if (wait_while_table_is_used(thd, tables->table, HA_EXTRA_FORCE_REOPEN))
+      goto end;
+
     pthread_mutex_lock(&LOCK_open);
   }
   else
diff --git a/sql/sql_view.cc b/sql/sql_view.cc
index 55181a58e53..c40f6643042 100644
--- a/sql/sql_view.cc
+++ b/sql/sql_view.cc
@@ -395,7 +395,7 @@ bool mysql_create_view(THD *thd, TABLE_LIST *views,
     goto err;
 
   lex->link_first_table_back(view, link_to_local);
-  view->open_table_type= TABLE_LIST::TAKE_EXCLUSIVE_MDL;
+  view->open_type= TABLE_LIST::TAKE_EXCLUSIVE_MDL;
 
   if (open_and_lock_tables(thd, lex->query_tables))
   {
diff --git a/sql/table.h b/sql/table.h
index a31b96e0828..e64111ef988 100644
--- a/sql/table.h
+++ b/sql/table.h
@@ -1349,14 +1349,26 @@ struct TABLE_LIST
     used for implicit LOCK TABLES only and won't be used in real statement.
   */
   bool          prelocking_placeholder;
-  /*
-    This TABLE_LIST object corresponds to the table/view which requires
-    special handling/meta-data locking. For example this is a target
-    table in CREATE TABLE ... SELECT so it is possible that it does not
-    exist and we should take exclusive meta-data lock on it in this
-    case.
+  /**
+     Indicates that if TABLE_LIST object corresponds to the table/view
+     which requires special handling/meta-data locking.
   */
-  enum {NORMAL_OPEN= 0, OPEN_OR_CREATE, TAKE_EXCLUSIVE_MDL} open_table_type;
+  enum
+  {
+    /* Normal open, shared metadata lock should be taken. */
+    NORMAL_OPEN= 0,
+    /*
+      It's target table of CREATE TABLE ... SELECT so we should
+      either open table if it exists (and take shared metadata lock)
+      or take exclusive metadata lock if it doesn't exist.
+    */
+    OPEN_OR_CREATE,
+    /*
+      It's target view of CREATE/ALTER VIEW. We should take exclusive
+      metadata lock for this table list element.
+    */
+    TAKE_EXCLUSIVE_MDL
+  } open_type;
   /**
      Indicates that for this table/view we need to take shared metadata
      lock which should be upgradable to exclusive metadata lock.