Fixed bug mdev-3891.

If a query referenced some system statistical tables, but not all of them,
then executing an ANALYZE command simultaneously with this query could
lead to a deadlock.
The fix prohibited reading statistics from system statistical tables
for such queries.

Removed the function unlock_tables_n_open_system_tables_for_write()
as not used anymore.
Performed some minor refactoring of the code in sql_statistics.cc. 

mysql-test/r/stat_tables_par.result

@@ -202,6 +202,21 @@ dbt3_s001	lineitem	i_l_shipdate	1	2.6500
 dbt3_s001	lineitem	i_l_suppkey	1	600.5000
 dbt3_s001	lineitem	i_l_suppkey_partkey	1	30.0250
 dbt3_s001	lineitem	i_l_suppkey_partkey	2	8.5786
+set @save_global_use_stat_tables=@@global.use_stat_tables;
+set global use_stat_tables='preferably';
+set debug_sync='RESET';
+set debug_sync='statistics_update_start SIGNAL parker WAIT_FOR go1 EXECUTE 1';
+set debug_sync='thr_multi_lock_after_thr_lock SIGNAL go2 EXECUTE 2';
+use dbt3_s001;
+analyze table lineitem persistent for all;
+set debug_sync='open_and_process_table WAIT_FOR parker';
+set debug_sync='statistics_read_start SIGNAL go1 WAIT_FOR go2';
+use dbt3_s001;
+select * from mysql.index_stats, lineitem where index_name= 'i_l_shipdate' and l_orderkey=1 and l_partkey=68;
+db_name	table_name	index_name	prefix_arity	avg_frequency	l_orderkey	l_partkey	l_suppkey	l_linenumber	l_quantity	l_extendedprice	l_discount	l_tax	l_returnflag	l_linestatus	l_shipDATE	l_commitDATE	l_receiptDATE	l_shipinstruct	l_shipmode	l_comment
+dbt3_s001	lineitem	i_l_shipdate	1	2.6500	1	68	9	2	36	34850.16	0.09	0.06	N	O	1996-04-12	1996-02-28	1996-04-20	TAKE BACK RETURN	MAIL	slyly bold pinto beans detect s
+set debug_sync='RESET';
+set global use_stat_tables=@save_global_use_stat_tables;
 DROP DATABASE dbt3_s001;
 use test;
 set use_stat_tables=@save_use_stat_tables;

mysql-test/r/stat_tables_par_innodb.result

@@ -211,6 +211,21 @@ dbt3_s001	lineitem	i_l_shipdate	1	2.6496
 dbt3_s001	lineitem	i_l_suppkey	1	600.4000
 dbt3_s001	lineitem	i_l_suppkey_partkey	1	30.0200
 dbt3_s001	lineitem	i_l_suppkey_partkey	2	8.5771
+set @save_global_use_stat_tables=@@global.use_stat_tables;
+set global use_stat_tables='preferably';
+set debug_sync='RESET';
+set debug_sync='statistics_update_start SIGNAL parker WAIT_FOR go1 EXECUTE 1';
+set debug_sync='thr_multi_lock_after_thr_lock SIGNAL go2 EXECUTE 2';
+use dbt3_s001;
+analyze table lineitem persistent for all;
+set debug_sync='open_and_process_table WAIT_FOR parker';
+set debug_sync='statistics_read_start SIGNAL go1 WAIT_FOR go2';
+use dbt3_s001;
+select * from mysql.index_stats, lineitem where index_name= 'i_l_shipdate' and l_orderkey=1 and l_partkey=68;
+db_name	table_name	index_name	prefix_arity	avg_frequency	l_orderkey	l_partkey	l_suppkey	l_linenumber	l_quantity	l_extendedprice	l_discount	l_tax	l_returnflag	l_linestatus	l_shipDATE	l_commitDATE	l_receiptDATE	l_shipinstruct	l_shipmode	l_comment
+dbt3_s001	lineitem	i_l_shipdate	1	2.6496	1	68	9	2	36	34850.16	0.09	0.06	N	O	1996-04-12	1996-02-28	1996-04-20	TAKE BACK RETURN	MAIL	slyly bold pinto beans detect s
+set debug_sync='RESET';
+set global use_stat_tables=@save_global_use_stat_tables;
 DROP DATABASE dbt3_s001;
 use test;
 set use_stat_tables=@save_use_stat_tables;

mysql-test/t/stat_tables_par.test

@@ -196,6 +196,48 @@ set debug_sync='RESET';
 select * from mysql.index_stats where table_name='lineitem'
   order by index_name, prefix_arity;
 
+#
+# Bug mdev-3891: deadlock for ANALYZE and SELECT over mysql.index_stats
+# 
+
+set @save_global_use_stat_tables=@@global.use_stat_tables;
+set global use_stat_tables='preferably';
+set debug_sync='RESET';
+
+connect (con1, localhost, root,,);
+connect (con2, localhost, root,,);
+
+connection con1;
+set debug_sync='statistics_update_start SIGNAL parker WAIT_FOR go1 EXECUTE 1';
+set debug_sync='thr_multi_lock_after_thr_lock SIGNAL go2 EXECUTE 2';
+use dbt3_s001;
+--send analyze table lineitem persistent for all
+
+connection con2;
+set debug_sync='open_and_process_table WAIT_FOR parker';
+set debug_sync='statistics_read_start SIGNAL go1 WAIT_FOR go2';
+use dbt3_s001;
+--send select * from mysql.index_stats, lineitem where index_name= 'i_l_shipdate' and l_orderkey=1 and l_partkey=68
+
+connection con1;
+--disable_result_log
+--disable_warnings
+--reap
+--enable_warnings
+--enable_result_log
+
+connection con2;
+--disable_warnings
+--reap
+--enable_warnings
+
+connection default;
+disconnect con1;
+disconnect con2;
+set debug_sync='RESET';
+
+set global use_stat_tables=@save_global_use_stat_tables;
+
 DROP DATABASE dbt3_s001;
 
 use test;

sql/sql_base.cc

@@ -9604,6 +9604,12 @@ has_write_table_auto_increment_not_first_in_pk(TABLE_LIST *tables)
     must call close_system_tables() to close systems tables opened
     with this call.
 
+  NOTES
+   In some situations we  use this function to open system tables for
+   writing. It happens, for examples, with statistical tables when
+   they are updated by an ANALYZE command. In these cases we should
+   guarantee that system tables will not be deadlocked.
+
   RETURN
     FALSE   Success
     TRUE    Error
@@ -9648,70 +9654,6 @@ open_system_tables_for_read(THD *thd, TABLE_LIST *table_list,
 }
 
 
-/*
-  Unlock opened tables and open and lock system tables for write.
-
-  SYNOPSIS
-    unlock_tables_n_open_system_tables_for_write()
-      thd         Thread context.
-      table_list  List of tables to open.
-      backup      Pointer to Open_tables_state instance where
-                  information about currently open tables will be
-                  saved, and from which will be restored when we will
-                  end work with system tables.
-
-  DESCRIPTION
-    The function first unlocks the opened tables, but do not close them.
-    Then it opens and locks for write the specified system tables.
-  
-  NOTE
-    The system tables cannot be locked for write without unlocking
-    the current opened tables. Yet in some cases we still need valid TABLE
-    structures for these tables to be able to extract data that is to be
-    written into the system tables.
-    This function is used when updating the statistical tables. 
-
-  RETURN
-    FALSE   Success
-    TRUE    Error
-*/
-
-bool
-unlock_tables_n_open_system_tables_for_write(THD *thd,
-                                             TABLE_LIST *table_list,
-                                             Open_tables_backup *backup)
-{
-  Query_tables_list query_tables_list_backup;
-  LEX *lex= thd->lex;
-
-  DBUG_ENTER("unlock_tables_n_open_system_tables_for_write");
-
-  lex->reset_n_backup_query_tables_list(&query_tables_list_backup);
-  thd->reset_n_backup_open_tables_state(backup);
-
- if (open_and_lock_tables(thd, table_list, FALSE,
-                          MYSQL_OPEN_IGNORE_FLUSH | MYSQL_LOCK_IGNORE_TIMEOUT))
- {
-    lex->restore_backup_query_tables_list(&query_tables_list_backup);
-    goto error;
-  }
-
-  for (TABLE_LIST *tables= table_list; tables; tables= tables->next_global)
-  {
-    DBUG_ASSERT(tables->table->s->table_category == TABLE_CATEGORY_SYSTEM);
-    tables->table->use_all_columns();
-  }
-  lex->restore_backup_query_tables_list(&query_tables_list_backup);
-
-  DBUG_RETURN(FALSE);
-
-error:
-  close_system_tables(thd, backup);
-
-  DBUG_RETURN(TRUE);
-}
-
-
 /*
   Close system tables, opened with open_system_tables_for_read().
 

sql/sql_base.h

@@ -275,9 +275,6 @@ bool is_equal(const LEX_STRING *a, const LEX_STRING *b);
 /* Functions to work with system tables. */
 bool open_system_tables_for_read(THD *thd, TABLE_LIST *table_list,
                                  Open_tables_backup *backup);
-bool unlock_tables_n_open_system_tables_for_write(THD *thd,
-                                                  TABLE_LIST *table_list,
-                                                  Open_tables_backup *backup);
 void close_system_tables(THD *thd, Open_tables_backup *backup);
 void close_mysql_tables(THD *thd);
 TABLE *open_system_table_for_update(THD *thd, TABLE_LIST *one_table);

sql/sql_statistics.cc

@@ -126,6 +126,38 @@ inline void init_table_list_for_single_stat_table(TABLE_LIST *tbl,
 }
 
 
+/**
+  @brief
+  Open all statistical tables and lock them
+*/
+
+static
+inline int open_stat_tables(THD *thd, TABLE_LIST *tables,
+                            Open_tables_backup *backup,
+                            bool for_write)
+{
+  init_table_list_for_stat_tables(tables, for_write);
+  init_mdl_requests(tables);
+  return open_system_tables_for_read(thd, tables, backup);
+}
+
+
+/**
+  @brief
+  Open a statistical table and lock it
+*/
+static
+inline int open_single_stat_table(THD *thd, TABLE_LIST *table,
+                                  const LEX_STRING *stat_tab_name,
+                                  Open_tables_backup *backup,
+                                  bool for_write)
+{
+  init_table_list_for_single_stat_table(table, stat_tab_name, for_write);
+  init_mdl_requests(table);
+  return open_system_tables_for_read(thd, table, backup);
+}
+
+
 /**
   @details
   If the value of the parameter is_safe is TRUE then the function
@@ -1199,7 +1231,7 @@ public:
   the number of distinct values for a column. The class employs the
   Unique class for this purpose.
   The class Count_distinct_field is used only by the function
-  collect_statistics_from_table to calculate the values for 
+  collect_statistics_for_table to calculate the values for 
   column avg_frequency of the statistical table column_stats.
 */
     
@@ -2179,12 +2211,9 @@ int update_statistics_for_table(THD *thd, TABLE *table)
 
   DBUG_ENTER("update_statistics_for_table");
 
-  init_table_list_for_stat_tables(tables, TRUE);
-  init_mdl_requests(tables);
+  DEBUG_SYNC(thd, "statistics_update_start");
 
-  if (unlock_tables_n_open_system_tables_for_write(thd,
-                                                   tables,
-                                                   &open_tables_backup))
+  if (open_stat_tables(thd, tables, &open_tables_backup, TRUE))
   {
     thd->clear_error();
     DBUG_RETURN(rc);
@@ -2266,7 +2295,7 @@ int update_statistics_for_table(THD *thd, TABLE *table)
   The parameter stat_tables should point to an array of TABLE_LIST
   objects for all statistical tables linked into a list. All statistical
   tables are supposed to be opened.  
-  The function is called by read_statistics_for_table_if_needed().
+  The function is called by read_statistics_for_tables_if_needed().
 
   @retval
   0         If data has been successfully read for the table  
@@ -2415,6 +2444,21 @@ bool statistics_for_tables_is_needed(THD *thd, TABLE_LIST *tables)
     return FALSE;
   }
 
+  /* 
+    Do not read statistics for any query over non-user tables.
+    If the query references some statistical tables, but not all 
+    of them, reading the statistics may lead to a deadlock
+  */ 
+  for (TABLE_LIST *tl= tables; tl; tl= tl->next_global)
+  {
+    if (!tl->is_view_or_derived() && tl->table)
+    {
+      TABLE_SHARE *table_share= tl->table->s;
+      if (table_share && table_share->table_category != TABLE_CATEGORY_USER)
+        return FALSE;
+    }
+  }
+
   for (TABLE_LIST *tl= tables; tl; tl= tl->next_global)
   {
     if (!tl->is_view_or_derived() && tl->table)
@@ -2460,12 +2504,12 @@ int read_statistics_for_tables_if_needed(THD *thd, TABLE_LIST *tables)
 
   DBUG_ENTER("read_statistics_for_table_if_needed");
 
+  DEBUG_SYNC(thd, "statistics_read_start");
+
   if (!statistics_for_tables_is_needed(thd, tables))
     DBUG_RETURN(0);
 
-  init_table_list_for_stat_tables(stat_tables, FALSE);  
-  init_mdl_requests(stat_tables);
-  if (open_system_tables_for_read(thd, stat_tables, &open_tables_backup))
+  if (open_stat_tables(thd, stat_tables, &open_tables_backup, FALSE))
   {
     thd->clear_error();
     DBUG_RETURN(1);
@@ -2527,12 +2571,7 @@ int delete_statistics_for_table(THD *thd, LEX_STRING *db, LEX_STRING *tab)
 
   DBUG_ENTER("delete_statistics_for_table");
    
-  init_table_list_for_stat_tables(tables, TRUE);
-  init_mdl_requests(tables);
-
-  if (open_system_tables_for_read(thd,
-                                  tables,
-                                  &open_tables_backup))
+  if (open_stat_tables(thd, tables, &open_tables_backup, TRUE))
   {
     thd->clear_error();
     DBUG_RETURN(rc);
@@ -2619,12 +2658,8 @@ int delete_statistics_for_column(THD *thd, TABLE *tab, Field *col)
 
   DBUG_ENTER("delete_statistics_for_column");
    
-  init_table_list_for_single_stat_table(&tables, &stat_table_name[1], TRUE);
-  init_mdl_requests(&tables);
-
-  if (open_system_tables_for_read(thd,
-                                  &tables,
-                                  &open_tables_backup))
+  if (open_single_stat_table(thd, &tables, &stat_table_name[1],
+                             &open_tables_backup, TRUE))
   {
     thd->clear_error();
     DBUG_RETURN(rc);
@@ -2692,12 +2727,8 @@ int delete_statistics_for_index(THD *thd, TABLE *tab, KEY *key_info,
 
   DBUG_ENTER("delete_statistics_for_index");
    
-  init_table_list_for_single_stat_table(&tables, &stat_table_name[2], TRUE);
-  init_mdl_requests(&tables);
-
-  if (open_system_tables_for_read(thd,
-                                  &tables,
-                                  &open_tables_backup))
+  if (open_single_stat_table(thd, &tables, &stat_table_name[2],
+			     &open_tables_backup, TRUE))
   {
     thd->clear_error();
     DBUG_RETURN(rc);
@@ -2779,12 +2810,7 @@ int rename_table_in_stat_tables(THD *thd, LEX_STRING *db, LEX_STRING *tab,
 
   DBUG_ENTER("rename_table_in_stat_tables");
    
-  init_table_list_for_stat_tables(tables, TRUE);
-  init_mdl_requests(tables);
-
-  if (open_system_tables_for_read(thd,
-                                  tables,
-                                  &open_tables_backup))
+  if (open_stat_tables(thd, tables, &open_tables_backup, TRUE))
   {
     thd->clear_error();
     DBUG_RETURN(rc);
@@ -2876,12 +2902,8 @@ int rename_column_in_stat_tables(THD *thd, TABLE *tab, Field *col,
 
   DBUG_ENTER("rename_column_in_stat_tables");
    
-  init_table_list_for_single_stat_table(&tables, &stat_table_name[1], TRUE);
-  init_mdl_requests(&tables);
-
-  if (open_system_tables_for_read(thd,
-                                  &tables,
-                                  &open_tables_backup))
+  if (open_single_stat_table(thd, &tables, &stat_table_name[1],
+                             &open_tables_backup, TRUE))
   {
     thd->clear_error();
     DBUG_RETURN(rc);