Honor lock_wait_timeout when updating stats tables

- Call delete_statistics_tables() after lock_table_names in drop tables.
  This avoids a deadlock issue with FTWRL and future backup locks.
- Added some missing clear_error()
- Ensure we don't clear error caused by the caller
- Updated function comments

sql/sql_base.cc

@@ -8885,7 +8885,6 @@ open_system_tables_for_read(THD *thd, TABLE_LIST *table_list,
 {
   Query_tables_list query_tables_list_backup;
   LEX *lex= thd->lex;
-
   DBUG_ENTER("open_system_tables_for_read");
 
   /*
@@ -8899,9 +8898,15 @@ open_system_tables_for_read(THD *thd, TABLE_LIST *table_list,
   thd->reset_n_backup_open_tables_state(backup);
   thd->lex->sql_command= SQLCOM_SELECT;
 
+  /*
+    Only use MYSQL_LOCK_IGNORE_TIMEOUT for tables opened for read.
+    This is to ensure that lock_wait_timeout is honored when trying
+    to update stats tables.
+  */
   if (open_and_lock_tables(thd, table_list, FALSE,
-                           MYSQL_OPEN_IGNORE_FLUSH |
+                           (MYSQL_OPEN_IGNORE_FLUSH |
-                           MYSQL_LOCK_IGNORE_TIMEOUT))
+                            (table_list->lock_type < TL_WRITE_ALLOW_WRITE ?
+                             MYSQL_LOCK_IGNORE_TIMEOUT : 0))))
   {
     lex->restore_backup_query_tables_list(&query_tables_list_backup);
     thd->restore_backup_open_tables_state(backup);

sql/sql_statistics.cc

@@ -3253,7 +3253,7 @@ int read_statistics_for_tables_if_needed(THD *thd, TABLE_LIST *tables)
 {
   TABLE_LIST stat_tables[STATISTICS_TABLES];
   Open_tables_backup open_tables_backup;
-
+  bool has_error_active= thd->is_error();
   DBUG_ENTER("read_statistics_for_tables_if_needed");
 
   DEBUG_SYNC(thd, "statistics_read_start");
@@ -3263,7 +3263,8 @@ int read_statistics_for_tables_if_needed(THD *thd, TABLE_LIST *tables)
 
   if (open_stat_tables(thd, stat_tables, &open_tables_backup, FALSE))
   {
-    thd->clear_error();
+    if (!has_error_active)
+      thd->clear_error();
     DBUG_RETURN(1);
   }
 
@@ -3317,7 +3318,7 @@ int read_statistics_for_tables_if_needed(THD *thd, TABLE_LIST *tables)
   'db' from all statistical tables: table_stats, column_stats, index_stats.
 
   @retval
-  0         If all deletions are successful  
+  0         If all deletions are successful or we couldn't open statistics table
   @retval
   1         Otherwise
 
@@ -3325,7 +3326,8 @@ int read_statistics_for_tables_if_needed(THD *thd, TABLE_LIST *tables)
   The function is called when executing the statement DROP TABLE 'tab'.
 */
 
-int delete_statistics_for_table(THD *thd, const LEX_CSTRING *db, const LEX_CSTRING *tab)
+int delete_statistics_for_table(THD *thd, const LEX_CSTRING *db,
+                                const LEX_CSTRING *tab)
 {
   int err;
   enum_binlog_format save_binlog_format;
@@ -3333,11 +3335,15 @@ int delete_statistics_for_table(THD *thd, const LEX_CSTRING *db, const LEX_CSTRI
   TABLE_LIST tables[STATISTICS_TABLES];
   Open_tables_backup open_tables_backup;
   int rc= 0;
-
+  bool has_error_active= thd->is_error();
   DBUG_ENTER("delete_statistics_for_table");
    
   if (open_stat_tables(thd, tables, &open_tables_backup, TRUE))
-    DBUG_RETURN(rc);
+  {
+    if (!has_error_active)
+      thd->clear_error();
+    DBUG_RETURN(0);
+  }
 
   save_binlog_format= thd->set_current_stmt_binlog_format_stmt();
 
@@ -3402,7 +3408,7 @@ int delete_statistics_for_table(THD *thd, const LEX_CSTRING *db, const LEX_CSTRI
   'tab' from the statistical table column_stats. 
 
   @retval
-  0         If the deletion is successful  
+  0         If all deletions are successful or we couldn't open statistics table
   @retval
   1         Otherwise
 
@@ -3419,14 +3425,15 @@ int delete_statistics_for_column(THD *thd, TABLE *tab, Field *col)
   TABLE_LIST tables;
   Open_tables_backup open_tables_backup;
   int rc= 0;
-
+  bool has_error_active= thd->is_error();
   DBUG_ENTER("delete_statistics_for_column");
    
   if (open_single_stat_table(thd, &tables, &stat_table_name[1],
                              &open_tables_backup, TRUE))
   {
-    thd->clear_error();
+    if (!has_error_active)
-    DBUG_RETURN(rc);
+      thd->clear_error();
+    DBUG_RETURN(0);
   }
 
   save_binlog_format= thd->set_current_stmt_binlog_format_stmt();
@@ -3468,7 +3475,7 @@ int delete_statistics_for_column(THD *thd, TABLE *tab, Field *col)
   defined on the table 'tab' from the statistical table index_stats.
 
   @retval
-  0         If the deletion is successful  
+  0         If all deletions are successful or we couldn't open statistics table
   @retval
   1         Otherwise
 
@@ -3486,14 +3493,15 @@ int delete_statistics_for_index(THD *thd, TABLE *tab, KEY *key_info,
   TABLE_LIST tables;
   Open_tables_backup open_tables_backup;
   int rc= 0;
-
+  bool has_error_active= thd->is_error();
   DBUG_ENTER("delete_statistics_for_index");
    
   if (open_single_stat_table(thd, &tables, &stat_table_name[2],
 			     &open_tables_backup, TRUE))
   {
-    thd->clear_error();
+    if (!has_error_active)
-    DBUG_RETURN(rc);
+      thd->clear_error();
+    DBUG_RETURN(0);
   }
 
   save_binlog_format= thd->set_current_stmt_binlog_format_stmt();
@@ -3563,8 +3571,10 @@ int delete_statistics_for_index(THD *thd, TABLE *tab, KEY *key_info,
   The function is called when executing any statement that renames a table
 */
 
-int rename_table_in_stat_tables(THD *thd, const LEX_CSTRING *db, const LEX_CSTRING *tab,
+int rename_table_in_stat_tables(THD *thd, const LEX_CSTRING *db,
-                                const LEX_CSTRING *new_db, const LEX_CSTRING *new_tab)
+                                const LEX_CSTRING *tab,
+                                const LEX_CSTRING *new_db,
+                                const LEX_CSTRING *new_tab)
 {
   int err;
   enum_binlog_format save_binlog_format;
@@ -3575,7 +3585,9 @@ int rename_table_in_stat_tables(THD *thd, const LEX_CSTRING *db, const LEX_CSTRI
   DBUG_ENTER("rename_table_in_stat_tables");
    
   if (open_stat_tables(thd, tables, &open_tables_backup, TRUE))
+  {
     DBUG_RETURN(0); // not an error
+  }
 
   save_binlog_format= thd->set_current_stmt_binlog_format_stmt();
 
@@ -3667,7 +3679,7 @@ int rename_column_in_stat_tables(THD *thd, TABLE *tab, Field *col,
   TABLE_LIST tables;
   Open_tables_backup open_tables_backup;
   int rc= 0;
-
+  bool has_error_active= thd->is_error();
   DBUG_ENTER("rename_column_in_stat_tables");
   
   if (tab->s->tmp_table != NO_TMP_TABLE)
@@ -3676,7 +3688,8 @@ int rename_column_in_stat_tables(THD *thd, TABLE *tab, Field *col,
   if (open_single_stat_table(thd, &tables, &stat_table_name[1],
                              &open_tables_backup, TRUE))
   {
-    thd->clear_error();
+    if (!has_error_active)
+      thd->clear_error();
     DBUG_RETURN(rc);
   }
 

sql/sql_table.cc

@@ -2040,18 +2040,6 @@ bool mysql_rm_table(THD *thd,TABLE_LIST *tables, bool if_exists,
 
   if (!drop_temporary)
   {
-    if (!in_bootstrap)
-    {
-      for (table= tables; table; table= table->next_local)
-      {
-        LEX_CSTRING db_name= table->db;
-        LEX_CSTRING table_name= table->table_name;
-        if (table->open_type == OT_BASE_ONLY ||
-            !thd->find_temporary_table(table))
-          (void) delete_statistics_for_table(thd, &db_name, &table_name);
-      }
-    }
-
     if (!thd->locked_tables_mode)
     {
       if (drop_sequence)
@@ -2115,6 +2103,18 @@ bool mysql_rm_table(THD *thd,TABLE_LIST *tables, bool if_exists,
         }
       }
     }
+    /* We remove statistics for table last, after we have the DDL lock */
+    if (!in_bootstrap)
+    {
+      for (table= tables; table; table= table->next_local)
+      {
+        LEX_CSTRING db_name= table->db;
+        LEX_CSTRING table_name= table->table_name;
+        if (table->open_type == OT_BASE_ONLY ||
+            !thd->find_temporary_table(table))
+          (void) delete_statistics_for_table(thd, &db_name, &table_name);
+      }
+    }
   }
 
   /* mark for close and remove all cached entries */
@@ -2127,7 +2127,6 @@ bool mysql_rm_table(THD *thd,TABLE_LIST *tables, bool if_exists,
     DBUG_RETURN(TRUE);
   my_ok(thd);
   DBUG_RETURN(FALSE);
-
 }