MDEV-19536 - Server crash or ASAN heap-use-after-free in is_temporary_table /

read_statistics_for_tables_if_needed

Regression after 279a907, read_statistics_for_tables_if_needed() was
called after open_normal_and_derived_tables() failure.

Fixed by moving read_statistics_for_tables() call to a branch of
get_schema_stat_record() where result of open_normal_and_derived_tables()
is checked.

Removed THD::force_read_stats, added read_statistics_for_tables() instead.
Simplified away statistics_for_command_is_needed().

sql/sql_class.cc

@@ -986,7 +986,6 @@ THD::THD(bool is_wsrep_applier)
   memset(&invoker_host, 0, sizeof(invoker_host));
   prepare_derived_at_open= FALSE;
   create_tmp_table_for_derived= FALSE;
-  force_read_stats= FALSE;
   save_prep_leaf_list= FALSE;
   /* Restore THR_THD */
   set_current_thd(old_THR_THD);

sql/sql_class.h

@@ -2200,9 +2200,6 @@ public:
   */ 
   bool create_tmp_table_for_derived;
 
-  /* The flag to force reading statistics from EITS tables */
-  bool force_read_stats;
-
   bool save_prep_leaf_list;
 
   /* container for handler's private per-connection data */

sql/sql_show.cc

@@ -4272,7 +4272,6 @@ fill_schema_table_by_open(THD *thd, bool is_show_fields_or_keys,
     SQLCOM_SHOW_FIELDS is used because it satisfies
     'only_view_structure()'.
   */
-  thd->force_read_stats= get_schema_table_idx(schema_table) == SCH_STATISTICS;
   lex->sql_command= SQLCOM_SHOW_FIELDS;
   result= (open_temporary_tables(thd, table_list) ||
            open_normal_and_derived_tables(thd, table_list,
@@ -4287,9 +4286,6 @@ fill_schema_table_by_open(THD *thd, bool is_show_fields_or_keys,
   */
   lex->sql_command= old_lex->sql_command;
 
-  (void) read_statistics_for_tables_if_needed(thd, table_list);
-  thd->force_read_stats= false;
-
   DEBUG_SYNC(thd, "after_open_table_ignore_flush");
 
   /*
@@ -6165,6 +6161,7 @@ static int get_schema_stat_record(THD *thd, TABLE_LIST *tables,
     KEY *key_info=show_table->s->key_info;
     if (show_table->file)
     {
+      (void) read_statistics_for_tables(thd, tables);
       show_table->file->info(HA_STATUS_VARIABLE |
                              HA_STATUS_NO_LOCK |
                              HA_STATUS_TIME);

sql/sql_statistics.cc

@@ -2160,54 +2160,6 @@ int alloc_statistics_for_table(THD* thd, TABLE *table)
 }
 
 
-/**
-  @brief
-  Check whether any persistent statistics for the processed command is needed
-
-  @param
-  thd         The thread handle
-
-  @details
-  The function checks whether any persitent statistics for the processed
-  command is needed to be read.
-
-  @retval
-  TRUE        statistics is needed to be read 
-  @retval
-  FALSE       Otherwise
-*/
-
-static
-inline bool statistics_for_command_is_needed(THD *thd)
-{
-  if (thd->bootstrap || thd->variables.use_stat_tables == NEVER)
-    return FALSE;
-
-  if (thd->force_read_stats)
-    return TRUE;
-
-  switch(thd->lex->sql_command) {
-  case SQLCOM_SELECT:
-  case SQLCOM_INSERT:
-  case SQLCOM_INSERT_SELECT:
-  case SQLCOM_UPDATE:
-  case SQLCOM_UPDATE_MULTI:
-  case SQLCOM_DELETE:
-  case SQLCOM_DELETE_MULTI:
-  case SQLCOM_REPLACE:
-  case SQLCOM_REPLACE_SELECT:
-  case SQLCOM_CREATE_TABLE:
-  case SQLCOM_SET_OPTION:
-  case SQLCOM_DO:
-    break;
-  default: 
-    return FALSE;
-  }
-
-  return TRUE;
-} 
-
-
 /**
   @brief 
   Allocate memory for the statistical data used by a table share
@@ -2255,9 +2207,6 @@ static int alloc_statistics_for_table_share(THD* thd, TABLE_SHARE *table_share)
   DEBUG_SYNC(thd, "statistics_mem_alloc_start1");
   DEBUG_SYNC(thd, "statistics_mem_alloc_start2");
 
-  if (!statistics_for_command_is_needed(thd))
-    DBUG_RETURN(1);
-
   mysql_mutex_lock(&table_share->LOCK_share);
 
   if (stats_cb->stats_can_be_read)
@@ -3111,9 +3060,6 @@ bool statistics_for_tables_is_needed(THD *thd, TABLE_LIST *tables)
   if (!tables)
     return FALSE;
 
-  if (!statistics_for_command_is_needed(thd))
-    return FALSE;
-
   /* 
     Do not read statistics for any query that explicity involves
     statistical tables, failure to to do so we may end up
@@ -3244,15 +3190,40 @@ int read_histograms_for_table(THD *thd, TABLE *table, TABLE_LIST *stat_tables)
 */
 
 int read_statistics_for_tables_if_needed(THD *thd, TABLE_LIST *tables)
+{
+  switch (thd->lex->sql_command) {
+  case SQLCOM_SELECT:
+  case SQLCOM_INSERT:
+  case SQLCOM_INSERT_SELECT:
+  case SQLCOM_UPDATE:
+  case SQLCOM_UPDATE_MULTI:
+  case SQLCOM_DELETE:
+  case SQLCOM_DELETE_MULTI:
+  case SQLCOM_REPLACE:
+  case SQLCOM_REPLACE_SELECT:
+  case SQLCOM_CREATE_TABLE:
+  case SQLCOM_SET_OPTION:
+  case SQLCOM_DO:
+    return read_statistics_for_tables(thd, tables);
+  default:
+    return 0;
+  }
+}
+
+
+int read_statistics_for_tables(THD *thd, TABLE_LIST *tables)
 {
   TABLE_LIST stat_tables[STATISTICS_TABLES];
   Open_tables_backup open_tables_backup;
 
-  DBUG_ENTER("read_statistics_for_tables_if_needed");
+  DBUG_ENTER("read_statistics_for_tables");
+
+  if (thd->bootstrap || thd->variables.use_stat_tables == NEVER)
+    DBUG_RETURN(0);
 
   for (TABLE_LIST *tl= tables; tl; tl= tl->next_global)
   {
-    if (get_use_stat_tables_mode(thd) > NEVER && tl->table)
+    if (tl->table)
     {
       TABLE_SHARE *table_share= tl->table->s;
       if (table_share && table_share->table_category == TABLE_CATEGORY_USER &&

sql/sql_statistics.h

@@ -89,6 +89,7 @@ Use_stat_tables_mode get_use_stat_tables_mode(THD *thd)
 }
 
 int read_statistics_for_tables_if_needed(THD *thd, TABLE_LIST *tables);
+int read_statistics_for_tables(THD *thd, TABLE_LIST *tables);
 int collect_statistics_for_table(THD *thd, TABLE *table);
 void delete_stat_values_for_table_share(TABLE_SHARE *table_share);
 int alloc_statistics_for_table(THD *thd, TABLE *table);