A follow up patch for the fix for Bug#51263 "Deadlock between

transactional SELECT and ALTER TABLE ...  REBUILD PARTITION".

Make open flags part of Open_table_context.
This allows to simplify some code and (in future)
enforce the invariant that we don't, say, request a back 
off on the table when there is MYSQL_OPEN_IGNORE_FLUSH 
flag.



sql/sql_base.cc:
  open_table() flags are part of Open_table_context.
  Remove dead code that would check for OPEN_VIEW_NO_PARSE,
  which is not an open table flag.
sql/sql_base.h:
  Move flags to Open_table_context. Reorder Open_table_context
  members to compact the structure footprint.
sql/sql_insert.cc:
  Update with a new calling signature of open_table().
sql/sql_table.cc:
  Update with a new calling signature of open_table().

sql/sql_base.cc

@@ -2491,12 +2491,13 @@ open_table_get_mdl_lock(THD *thd, Open_table_context *ot_ctx,
 
 
 bool open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT *mem_root,
-                Open_table_context *ot_ctx, uint flags)
+                Open_table_context *ot_ctx)
 {
   reg1	TABLE *table;
   char	key[MAX_DBKEY_LENGTH];
   uint	key_length;
   char	*alias= table_list->alias;
+  uint flags= ot_ctx->get_flags();
   MDL_ticket *mdl_ticket;
   int error;
   TABLE_SHARE *share;
@@ -2806,26 +2807,15 @@ bool open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT *mem_root,
       if (open_new_frm(thd, share, alias,
                        (uint) (HA_OPEN_KEYFILE | HA_OPEN_RNDFILE |
                                HA_GET_INDEX | HA_TRY_READ_ONLY),
-                       READ_KEYINFO | COMPUTE_TYPES | EXTRA_RECORD |
-                       (flags & OPEN_VIEW_NO_PARSE), thd->open_options,
+                       READ_KEYINFO | COMPUTE_TYPES | EXTRA_RECORD,
+                       thd->open_options,
                        0, table_list, mem_root))
         goto err_unlock;
 
       /* TODO: Don't free this */
       release_table_share(share);
 
-      if (flags & OPEN_VIEW_NO_PARSE)
-      {
-        /*
-          VIEW not really opened, only frm were read.
-          Set 1 as a flag here
-        */
-        table_list->view= (LEX*)1;
-      }
-      else
-      {
-        DBUG_ASSERT(table_list->view);
-      }
+      DBUG_ASSERT(table_list->view);
 
       mysql_mutex_unlock(&LOCK_open);
       DBUG_RETURN(FALSE);
@@ -3358,7 +3348,7 @@ unlink_all_closed_tables(THD *thd, MYSQL_LOCK *lock, size_t reopen_count)
 bool
 Locked_tables_list::reopen_tables(THD *thd)
 {
-  Open_table_context ot_ctx_unused(thd, LONG_TIMEOUT);
+  Open_table_context ot_ctx(thd, MYSQL_OPEN_REOPEN);
   size_t reopen_count= 0;
   MYSQL_LOCK *lock;
   MYSQL_LOCK *merged_lock;
@@ -3370,8 +3360,7 @@ Locked_tables_list::reopen_tables(THD *thd)
       continue;
 
     /* Links into thd->open_tables upon success */
-    if (open_table(thd, table_list, thd->mem_root, &ot_ctx_unused,
-                   MYSQL_OPEN_REOPEN))
+    if (open_table(thd, table_list, thd->mem_root, &ot_ctx))
     {
       unlink_all_closed_tables(thd, 0, reopen_count);
       return TRUE;
@@ -3793,16 +3782,18 @@ end_with_lock_open:
 
 /** Open_table_context */
 
-Open_table_context::Open_table_context(THD *thd, ulong timeout)
-  :m_action(OT_NO_ACTION),
-   m_failed_mdl_request(NULL),
+Open_table_context::Open_table_context(THD *thd, uint flags)
+  :m_failed_mdl_request(NULL),
    m_failed_table(NULL),
    m_start_of_statement_svp(thd->mdl_context.mdl_savepoint()),
+   m_global_mdl_request(NULL),
+   m_timeout(flags & MYSQL_LOCK_IGNORE_TIMEOUT ?
+             LONG_TIMEOUT : thd->variables.lock_wait_timeout),
+   m_flags(flags),
+   m_action(OT_NO_ACTION),
    m_has_locks((thd->in_multi_stmt_transaction_mode() &&
                 thd->mdl_context.has_locks()) ||
-                thd->mdl_context.trans_sentinel()),
-   m_global_mdl_request(NULL),
-   m_timeout(timeout)
+                thd->mdl_context.trans_sentinel())
 {}
 
 
@@ -4290,12 +4281,12 @@ open_and_process_table(THD *thd, LEX *lex, TABLE_LIST *tables,
     */
     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);
     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);
 
   free_root(new_frm_mem, MYF(MY_KEEP_PREALLOC));
 
@@ -4610,8 +4601,7 @@ bool open_tables(THD *thd, TABLE_LIST **start, uint *counter, uint flags,
   TABLE_LIST **table_to_open;
   Sroutine_hash_entry **sroutine_to_open;
   TABLE_LIST *tables;
-  Open_table_context ot_ctx(thd, (flags & MYSQL_LOCK_IGNORE_TIMEOUT) ?
-                            LONG_TIMEOUT : thd->variables.lock_wait_timeout);
+  Open_table_context ot_ctx(thd, flags);
   bool error= FALSE;
   MEM_ROOT new_frm_mem;
   bool has_prelocking_list;
@@ -5183,8 +5173,7 @@ TABLE *open_ltable(THD *thd, TABLE_LIST *table_list, thr_lock_type lock_type,
                    uint lock_flags)
 {
   TABLE *table;
-  Open_table_context ot_ctx(thd, (lock_flags & MYSQL_LOCK_IGNORE_TIMEOUT) ?
-                            LONG_TIMEOUT : thd->variables.lock_wait_timeout);
+  Open_table_context ot_ctx(thd, lock_flags);
   bool error;
   DBUG_ENTER("open_ltable");
 
@@ -5199,7 +5188,7 @@ TABLE *open_ltable(THD *thd, TABLE_LIST *table_list, thr_lock_type lock_type,
   /* This function can't properly handle requests for such metadata locks. */
   DBUG_ASSERT(table_list->mdl_request.type < MDL_SHARED_NO_WRITE);
 
-  while ((error= open_table(thd, table_list, thd->mem_root, &ot_ctx, lock_flags)) &&
+  while ((error= open_table(thd, table_list, thd->mem_root, &ot_ctx)) &&
          ot_ctx.can_recover_from_failed_open())
   {
     /*

sql/sql_base.h

@@ -89,7 +89,7 @@ TABLE_SHARE *get_cached_table_share(const char *db, const char *table_name);
 TABLE *open_ltable(THD *thd, TABLE_LIST *table_list, thr_lock_type update,
                    uint lock_flags);
 bool open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT *mem_root,
-                Open_table_context *ot_ctx, uint flags);
+                Open_table_context *ot_ctx);
 bool name_lock_locked_table(THD *thd, TABLE_LIST *tables);
 bool reopen_name_locked_table(THD* thd, TABLE_LIST* table_list, bool link_in);
 TABLE *table_cache_insert_placeholder(THD *thd, const char *key,
@@ -456,7 +456,7 @@ public:
     OT_DISCOVER,
     OT_REPAIR
   };
-  Open_table_context(THD *thd, ulong timeout);
+  Open_table_context(THD *thd, uint flags);
 
   bool recover_from_failed_open(THD *thd);
   bool request_backoff_action(enum_open_table_action action_arg,
@@ -486,11 +486,10 @@ public:
     return m_timeout;
   }
 
+  uint get_flags() const { return m_flags; }
 private:
   /** List of requests for all locks taken so far. Used for waiting on locks. */
   MDL_request_list m_mdl_requests;
-  /** Back off action. */
-  enum enum_open_table_action m_action;
   /** For OT_WAIT_MDL_LOCK action, the request for which we should wait. */
   MDL_request *m_failed_mdl_request;
   /**
@@ -500,12 +499,6 @@ private:
   */
   TABLE_LIST *m_failed_table;
   MDL_ticket *m_start_of_statement_svp;
-  /**
-    Whether we had any locks when this context was created.
-    If we did, they are from the previous statement of a transaction,
-    and we can't safely do back-off (and release them).
-  */
-  bool m_has_locks;
   /**
     Request object for global intention exclusive lock which is acquired during
     opening tables for statements which take upgradable shared metadata locks.
@@ -515,7 +508,17 @@ private:
     Lock timeout in seconds. Initialized to LONG_TIMEOUT when opening system
     tables or to the "lock_wait_timeout" system variable for regular tables.
   */
-  uint m_timeout;
+  ulong m_timeout;
+  /* open_table() flags. */
+  uint m_flags;
+  /** Back off action. */
+  enum enum_open_table_action m_action;
+  /**
+    Whether we had any locks when this context was created.
+    If we did, they are from the previous statement of a transaction,
+    and we can't safely do back-off (and release them).
+  */
+  bool m_has_locks;
 };
 
 

sql/sql_insert.cc

@@ -3608,13 +3608,12 @@ static TABLE *create_table_from_items(THD *thd, HA_CREATE_INFO *create_info,
 
       if (!(create_info->options & HA_LEX_CREATE_TMP_TABLE))
       {
-        Open_table_context ot_ctx_unused(thd, LONG_TIMEOUT);
+        Open_table_context ot_ctx(thd, MYSQL_OPEN_REOPEN);
         /*
           Here we open the destination table, on which we already have
           an exclusive metadata lock.
         */
-        if (open_table(thd, create_table, thd->mem_root,
-                       &ot_ctx_unused, MYSQL_OPEN_REOPEN))
+        if (open_table(thd, create_table, thd->mem_root, &ot_ctx))
         {
           mysql_mutex_lock(&LOCK_open);
           quick_rm_table(create_info->db_type, create_table->db,
@@ -3627,9 +3626,8 @@ static TABLE *create_table_from_items(THD *thd, HA_CREATE_INFO *create_info,
       }
       else
       {
-        Open_table_context ot_ctx_unused(thd, LONG_TIMEOUT);
-        if (open_table(thd, create_table, thd->mem_root, &ot_ctx_unused,
-                       MYSQL_OPEN_TEMPORARY_ONLY))
+        Open_table_context ot_ctx(thd, MYSQL_OPEN_TEMPORARY_ONLY);
+        if (open_table(thd, create_table, thd->mem_root, &ot_ctx))
         {
           /*
             This shouldn't happen as creation of temporary table should make

sql/sql_table.cc

@@ -4426,10 +4426,10 @@ static int prepare_for_repair(THD *thd, TABLE_LIST *table_list,
   char from[FN_REFLEN],tmp[FN_REFLEN+32];
   const char **ext;
   MY_STAT stat_info;
-  Open_table_context ot_ctx_unused(thd, LONG_TIMEOUT);
+  Open_table_context ot_ctx(thd, (MYSQL_OPEN_IGNORE_FLUSH |
+                                  MYSQL_OPEN_HAS_MDL_LOCK |
+                                  MYSQL_LOCK_IGNORE_TIMEOUT));
   DBUG_ENTER("prepare_for_repair");
-  uint reopen_for_repair_flags= (MYSQL_OPEN_IGNORE_FLUSH |
-                                 MYSQL_OPEN_HAS_MDL_LOCK);
 
   if (!(check_opt->sql_flags & TT_USEFRM))
     DBUG_RETURN(0);
@@ -4584,8 +4584,7 @@ static int prepare_for_repair(THD *thd, TABLE_LIST *table_list,
     Now we should be able to open the partially repaired table
     to finish the repair in the handler later on.
   */
-  if (open_table(thd, table_list, thd->mem_root,
-                 &ot_ctx_unused, reopen_for_repair_flags))
+  if (open_table(thd, table_list, thd->mem_root, &ot_ctx))
   {
     error= send_check_errmsg(thd, table_list, "repair",
                              "Failed to open partially repaired table");
@@ -5374,7 +5373,7 @@ bool mysql_create_like_table(THD* thd, TABLE_LIST* table, TABLE_LIST* src_table,
         char buf[2048];
         String query(buf, sizeof(buf), system_charset_info);
         query.length(0);  // Have to zero it since constructor doesn't
-        Open_table_context ot_ctx_unused(thd, LONG_TIMEOUT);
+        Open_table_context ot_ctx(thd, MYSQL_OPEN_REOPEN);
 
         /*
           The condition avoids a crash as described in BUG#48506. Other
@@ -5389,8 +5388,7 @@ bool mysql_create_like_table(THD* thd, TABLE_LIST* table, TABLE_LIST* src_table,
             to work. The table will be closed by close_thread_table() at
             the end of this branch.
           */
-          if (open_table(thd, table, thd->mem_root, &ot_ctx_unused,
-                         MYSQL_OPEN_REOPEN))
+          if (open_table(thd, table, thd->mem_root, &ot_ctx))
             goto err;
 
           int result __attribute__((unused))=
@@ -7139,14 +7137,14 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
   {
     if (table->s->tmp_table)
     {
-      Open_table_context ot_ctx_unused(thd, LONG_TIMEOUT);
+      Open_table_context ot_ctx(thd, (MYSQL_OPEN_IGNORE_FLUSH |
+                                      MYSQL_LOCK_IGNORE_TIMEOUT));
       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 */
-      (void) open_table(thd, &tbl, thd->mem_root, &ot_ctx_unused,
-                        MYSQL_OPEN_IGNORE_FLUSH);
+      (void) open_table(thd, &tbl, thd->mem_root, &ot_ctx);
       new_table= tbl.table;
     }
     else
@@ -7425,7 +7423,7 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
       To do this we need to obtain a handler object for it.
       NO need to tamper with MERGE tables. The real open is done later.
     */
-    Open_table_context ot_ctx_unused(thd, LONG_TIMEOUT);
+    Open_table_context ot_ctx(thd, MYSQL_OPEN_REOPEN);
     TABLE *t_table;
     if (new_name != table_name || new_db != db)
     {
@@ -7445,8 +7443,7 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
       */
       table_list->mdl_request.ticket= mdl_ticket;
     }
-    if (open_table(thd, table_list, thd->mem_root,
-                   &ot_ctx_unused, MYSQL_OPEN_REOPEN))
+    if (open_table(thd, table_list, thd->mem_root, &ot_ctx))
     {
       goto err_with_mdl;
     }