Simplified away statistics_for_tables_is_needed()

Removed redundant loops, integrated logics into the caller instead.
Unified condition in read_statistics_for_tables(), less
"table_share != NULL" checks, no more potential "table_share == NULL"
dereferencing.

Part of MDEV-19061 - table_share used for reading statistical tables is
                     not protected

1 files changed, 31 insertions, 81 deletions

diff --git a/sql/sql_statistics.cc b/sql/sql_statistics.cc
index 11e2db9ae44..0f8daa6e52f 100644
--- a/sql/sql_statistics.cc
+++ b/sql/sql_statistics.cc
@@ -3037,71 +3037,6 @@ void delete_stat_values_for_table_share(TABLE_SHARE *table_share)
 
 /**
   @brief
-  Check whether any statistics is to be read for tables from a table list
-
-  @param
-  thd         The thread handle
-  @param
-  tables      The tables list for whose tables the check is to be done
-
-  @details
-  The function checks whether for any of the tables opened and locked for
-  a statement statistics from statistical tables is needed to be read.
-
-  @retval
-  TRUE        statistics for any of the tables is needed to be read 
-  @retval
-  FALSE       Otherwise
-*/
-
-static
-bool statistics_for_tables_is_needed(THD *thd, TABLE_LIST *tables)
-{
-  if (!tables)
-    return FALSE;
-
-  /* 
-    Do not read statistics for any query that explicity involves
-    statistical tables, failure to to do so we may end up
-    in a deadlock.
-  */
-
-  for (TABLE_LIST *tl= tables; tl; tl= tl->next_global)
-  {
-    if (!tl->is_view_or_derived() && !is_temporary_table(tl) && tl->table)
-    {
-      TABLE_SHARE *table_share= tl->table->s;
-      if (table_share && 
-          table_share->table_category != TABLE_CATEGORY_USER
-          && is_stat_table(tl->db, tl->alias))
-        return FALSE;
-    }
-  }
-
-  for (TABLE_LIST *tl= tables; tl; tl= tl->next_global)
-  {
-    if (!tl->is_view_or_derived() && !is_temporary_table(tl) && tl->table)
-    {
-      TABLE_SHARE *table_share= tl->table->s;
-      if (table_share && 
-          table_share->stats_cb.stats_can_be_read &&
-          (!table_share->stats_cb.stats_is_read ||
-           (!table_share->stats_cb.histograms_are_read &&
-            thd->variables.optimizer_use_condition_selectivity > 3)))
-        return TRUE;
-      if (table_share->stats_cb.stats_is_read)
-        tl->table->stats_is_read= TRUE;
-      if (table_share->stats_cb.histograms_are_read)
-        tl->table->histograms_are_read= TRUE;
-    } 
-  }
-
-  return FALSE;
-}
-
-
-/**
-  @brief
   Read histogram for a table from the persistent statistical tables
 
   @param
@@ -3221,13 +3156,16 @@ int read_statistics_for_tables(THD *thd, TABLE_LIST *tables)
   if (thd->bootstrap || thd->variables.use_stat_tables == NEVER)
     DBUG_RETURN(0);
 
+  bool found_stat_table= false;
+  bool statistics_for_tables_is_needed= false;
+
   for (TABLE_LIST *tl= tables; tl; tl= tl->next_global)
   {
-    if (tl->table)
+    TABLE_SHARE *table_share;
+    if (!tl->is_view_or_derived() && tl->table && (table_share= tl->table->s) &&
+        table_share->tmp_table == NO_TMP_TABLE)
     {
-      TABLE_SHARE *table_share= tl->table->s;
-      if (table_share && table_share->table_category == TABLE_CATEGORY_USER &&
-          table_share->tmp_table == NO_TMP_TABLE)
+      if (table_share->table_category == TABLE_CATEGORY_USER)
       {
         if (table_share->stats_cb.stats_can_be_read ||
             !alloc_statistics_for_table_share(thd, table_share))
@@ -3244,15 +3182,29 @@ int read_statistics_for_tables(THD *thd, TABLE_LIST *tables)
             for ( ; *field_ptr; field_ptr++, table_field_ptr++)
               (*table_field_ptr)->read_stats= (*field_ptr)->read_stats;
             tl->table->stats_is_read= table_share->stats_cb.stats_is_read;
+            tl->table->histograms_are_read=
+              table_share->stats_cb.histograms_are_read;
+
+            if (!tl->table->stats_is_read ||
+                (!table_share->stats_cb.histograms_are_read &&
+                 thd->variables.optimizer_use_condition_selectivity > 3))
+              statistics_for_tables_is_needed= true;
           }
         }
       }
+      else if (is_stat_table(tl->db, tl->alias))
+        found_stat_table= true;
     }
   }
 
   DEBUG_SYNC(thd, "statistics_read_start");
 
-  if (!statistics_for_tables_is_needed(thd, tables))
+  /*
+    Do not read statistics for any query that explicity involves
+    statistical tables, failure to to do so we may end up
+    in a deadlock.
+  */
+  if (found_stat_table || !statistics_for_tables_is_needed)
     DBUG_RETURN(0);
 
   if (open_stat_tables(thd, stat_tables, &open_tables_backup, FALSE))
@@ -3260,14 +3212,12 @@ int read_statistics_for_tables(THD *thd, TABLE_LIST *tables)
 
   for (TABLE_LIST *tl= tables; tl; tl= tl->next_global)
   {
-    if (!tl->is_view_or_derived() && !is_temporary_table(tl) && tl->table)
-    { 
-      TABLE_SHARE *table_share= tl->table->s;
-      if (table_share && !(table_share->table_category == TABLE_CATEGORY_USER))
-        continue;
-
-      if (table_share && 
-          table_share->stats_cb.stats_can_be_read &&
+    TABLE_SHARE *table_share;
+    if (!tl->is_view_or_derived() && tl->table && (table_share= tl->table->s) &&
+        table_share->tmp_table == NO_TMP_TABLE &&
+        table_share->table_category == TABLE_CATEGORY_USER)
+    {
+      if (table_share->stats_cb.stats_can_be_read &&
 	  !table_share->stats_cb.stats_is_read)
       {
         (void) read_statistics_for_table(thd, tl->table, stat_tables);
@@ -3275,8 +3225,8 @@ int read_statistics_for_tables(THD *thd, TABLE_LIST *tables)
       }
       if (table_share->stats_cb.stats_is_read)
         tl->table->stats_is_read= TRUE;
-      if (thd->variables.optimizer_use_condition_selectivity > 3 && 
-          table_share && table_share->stats_cb.stats_can_be_read &&
+      if (thd->variables.optimizer_use_condition_selectivity > 3 &&
+          table_share->stats_cb.stats_can_be_read &&
           !table_share->stats_cb.histograms_are_read)
       {
         (void) read_histograms_for_table(thd, tl->table, stat_tables);
@@ -3285,7 +3235,7 @@ int read_statistics_for_tables(THD *thd, TABLE_LIST *tables)
       if (table_share->stats_cb.histograms_are_read)
         tl->table->histograms_are_read= TRUE;
     }
-  }  
+  }
 
   close_system_tables(thd, &open_tables_backup);