MDEV-10435 crash with bad stat tables.

        Functions from sql/statistics.cc don't seem to expect
        stat tables to fail or to have inadequate structure.
        Table open errors suppressed and some validity
        checks added. Invalid tables reported to the server
        log.

8 files changed, 216 insertions, 41 deletions

diff --git a/mysql-test/r/statistics.result b/mysql-test/r/statistics.result
index be2e39665b3..2d0f18f1ed7 100644
--- a/mysql-test/r/statistics.result
+++ b/mysql-test/r/statistics.result
@@ -1676,4 +1676,24 @@ analyze table t1;
 Table	Op	Msg_type	Msg_text
 test.t1	analyze	status	Table is already up to date
 drop table t1;
+#
+# MDEV-10435 crash with bad stat tables
+#
+set use_stat_tables='preferably';
+call mtr.add_suppression("Column count of mysql.table_stats is wrong. Expected 3, found 1. The table is probably corrupted");
+rename table mysql.table_stats to test.table_stats;
+flush tables;
+create table t1 (a int);
+rename table t1 to t2, t3 to t4;
+ERROR 42S02: Table 'test.t3' doesn't exist
+drop table t1;
+rename table test.table_stats to mysql.table_stats;
+rename table mysql.table_stats to test.table_stats;
+create table mysql.table_stats (a int);
+flush tables;
+create table t1 (a int);
+rename table t1 to t2, t3 to t4;
+ERROR 42S02: Table 'test.t3' doesn't exist
+drop table t1, mysql.table_stats;
+rename table test.table_stats to mysql.table_stats;
 set use_stat_tables=@save_use_stat_tables;
diff --git a/mysql-test/t/statistics.test b/mysql-test/t/statistics.test
index 805c169b2a4..3f08e2e133c 100644
--- a/mysql-test/t/statistics.test
+++ b/mysql-test/t/statistics.test
@@ -740,4 +740,28 @@ show variables like 'use_stat_tables';
 analyze table t1;
 drop table t1;
 
+--echo #
+--echo # MDEV-10435 crash with bad stat tables
+--echo #
+
+set use_stat_tables='preferably';
+call mtr.add_suppression("Column count of mysql.table_stats is wrong. Expected 3, found 1. The table is probably corrupted");
+
+rename table mysql.table_stats to test.table_stats;
+flush tables;
+create table t1 (a int);
+--error ER_NO_SUCH_TABLE
+rename table t1 to t2, t3 to t4;
+drop table t1;
+rename table test.table_stats to mysql.table_stats;
+
+rename table mysql.table_stats to test.table_stats;
+create table mysql.table_stats (a int);
+flush tables;
+create table t1 (a int);
+--error ER_NO_SUCH_TABLE
+rename table t1 to t2, t3 to t4;
+drop table t1, mysql.table_stats;
+rename table test.table_stats to mysql.table_stats;
+
 set use_stat_tables=@save_use_stat_tables;
diff --git a/sql/event_db_repository.cc b/sql/event_db_repository.cc
index 666f6e3e24c..ba782a19f8c 100644
--- a/sql/event_db_repository.cc
+++ b/sql/event_db_repository.cc
@@ -166,20 +166,8 @@ const TABLE_FIELD_TYPE event_table_fields[ET_FIELD_COUNT] =
 static const TABLE_FIELD_DEF
 event_table_def= {ET_FIELD_COUNT, event_table_fields, 0, (uint*) 0};
 
-class Event_db_intact : public Table_check_intact
-{
-protected:
-  void report_error(uint, const char *fmt, ...)
-  {
-    va_list args;
-    va_start(args, fmt);
-    error_log_print(ERROR_LEVEL, fmt, args);
-    va_end(args);
-  }
-};
-
 /** In case of an error, a message is printed to the error log. */
-static Event_db_intact table_intact;
+static Table_check_intact_log_error table_intact;
 
 
 /**
diff --git a/sql/rpl_gtid.cc b/sql/rpl_gtid.cc
index f54ef2b0081..dfec97bf021 100644
--- a/sql/rpl_gtid.cc
+++ b/sql/rpl_gtid.cc
@@ -448,19 +448,7 @@ static const TABLE_FIELD_DEF mysql_gtid_slave_pos_tabledef= {
   mysql_rpl_slave_state_pk_parts
 };
 
-class Gtid_db_intact : public Table_check_intact
-{
-protected:
-  void report_error(uint, const char *fmt, ...)
-  {
-    va_list args;
-    va_start(args, fmt);
-    error_log_print(ERROR_LEVEL, fmt, args);
-    va_end(args);
-  }
-};
-
-static Gtid_db_intact gtid_table_intact;
+static Table_check_intact_log_error gtid_table_intact;
 
 /*
   Check that the mysql.gtid_slave_pos table has the correct definition.
diff --git a/sql/sql_statistics.cc b/sql/sql_statistics.cc
index 27bc0fb4cd3..62d56a700b8 100644
--- a/sql/sql_statistics.cc
+++ b/sql/sql_statistics.cc
@@ -129,6 +129,128 @@ inline void init_table_list_for_single_stat_table(TABLE_LIST *tbl,
 }
 
 
+static Table_check_intact_log_error stat_table_intact;
+
+static const
+TABLE_FIELD_TYPE table_stat_fields[TABLE_STAT_N_FIELDS] =
+{
+  {
+    { C_STRING_WITH_LEN("db_name") },
+    { C_STRING_WITH_LEN("varchar(64)") },
+    { C_STRING_WITH_LEN("utf8") }
+  },
+  {
+    { C_STRING_WITH_LEN("table_name") },
+    { C_STRING_WITH_LEN("varchar(64)") },
+    { C_STRING_WITH_LEN("utf8") }
+  },
+  {
+    { C_STRING_WITH_LEN("cardinality") },
+    { C_STRING_WITH_LEN("bigint(21)") },
+    { NULL, 0 }
+  },
+};
+static const uint table_stat_pk_col[]= {0,1};
+static const TABLE_FIELD_DEF
+table_stat_def= {TABLE_STAT_N_FIELDS, table_stat_fields, 2, table_stat_pk_col };
+
+static const
+TABLE_FIELD_TYPE column_stat_fields[COLUMN_STAT_N_FIELDS] =
+{
+  {
+    { C_STRING_WITH_LEN("db_name") },
+    { C_STRING_WITH_LEN("varchar(64)") },
+    { C_STRING_WITH_LEN("utf8") }
+  },
+  {
+    { C_STRING_WITH_LEN("table_name") },
+    { C_STRING_WITH_LEN("varchar(64)") },
+    { C_STRING_WITH_LEN("utf8") }
+  },
+  {
+    { C_STRING_WITH_LEN("column_name") },
+    { C_STRING_WITH_LEN("varchar(64)") },
+    { C_STRING_WITH_LEN("utf8") }
+  },
+  {
+    { C_STRING_WITH_LEN("min_value") },
+    { C_STRING_WITH_LEN("varbinary(255)") },
+    { NULL, 0 }
+  },
+  {
+    { C_STRING_WITH_LEN("max_value") },
+    { C_STRING_WITH_LEN("varbinary(255)") },
+    { NULL, 0 }
+  },
+  {
+    { C_STRING_WITH_LEN("nulls_ratio") },
+    { C_STRING_WITH_LEN("decimal(12,4)") },
+    { NULL, 0 }
+  },
+  {
+    { C_STRING_WITH_LEN("avg_length") },
+    { C_STRING_WITH_LEN("decimal(12,4)") },
+    { NULL, 0 }
+  },
+  {
+    { C_STRING_WITH_LEN("avg_frequency") },
+    { C_STRING_WITH_LEN("decimal(12,4)") },
+    { NULL, 0 }
+  },
+  {
+    { C_STRING_WITH_LEN("hist_size") },
+    { C_STRING_WITH_LEN("tinyint(3)") },
+    { NULL, 0 }
+  },
+  {
+    { C_STRING_WITH_LEN("hist_type") },
+    { C_STRING_WITH_LEN("enum('SINGLE_PREC_HB','DOUBLE_PREC_HB')") },
+    { C_STRING_WITH_LEN("utf8") }
+  },
+  {
+    { C_STRING_WITH_LEN("histogram") },
+    { C_STRING_WITH_LEN("varbinary(255)") },
+    { NULL, 0 }
+  }
+};
+static const uint column_stat_pk_col[]= {0,1,2};
+static const TABLE_FIELD_DEF
+column_stat_def= {COLUMN_STAT_N_FIELDS, column_stat_fields, 3, column_stat_pk_col};
+
+static const
+TABLE_FIELD_TYPE index_stat_fields[INDEX_STAT_N_FIELDS] =
+{
+  {
+    { C_STRING_WITH_LEN("db_name") },
+    { C_STRING_WITH_LEN("varchar(64)") },
+    { C_STRING_WITH_LEN("utf8") }
+  },
+  {
+    { C_STRING_WITH_LEN("table_name") },
+    { C_STRING_WITH_LEN("varchar(64)") },
+    { C_STRING_WITH_LEN("utf8") }
+  },
+  {
+    { C_STRING_WITH_LEN("index") },
+    { C_STRING_WITH_LEN("varchar(64)") },
+    { C_STRING_WITH_LEN("utf8") }
+  },
+  {
+    { C_STRING_WITH_LEN("prefix_arity") },
+    { C_STRING_WITH_LEN("int(11)") },
+    { NULL, 0 }
+  },
+  {
+    { C_STRING_WITH_LEN("avg_frequency") },
+    { C_STRING_WITH_LEN("decimal(12,4)") },
+    { NULL, 0 }
+  }
+};
+static const uint index_stat_pk_col[]= {0,1,2,3};
+static const TABLE_FIELD_DEF
+index_stat_def= {INDEX_STAT_N_FIELDS, index_stat_fields, 4, index_stat_pk_col};
+
+
 /**
   @brief
   Open all statistical tables and lock them
@@ -139,10 +261,30 @@ inline int open_stat_tables(THD *thd, TABLE_LIST *tables,
                             Open_tables_backup *backup,
                             bool for_write)
 {
+  int rc;
+
+  Dummy_error_handler deh; // suppress errors
+  thd->push_internal_handler(&deh);
   init_table_list_for_stat_tables(tables, for_write);
   init_mdl_requests(tables);
-  return open_system_tables_for_read(thd, tables, backup);
-}
+  rc= open_system_tables_for_read(thd, tables, backup);
+  thd->pop_internal_handler();
+
+
+  /* If the number of tables changes, we should revise the check below. */
+  DBUG_ASSERT(STATISTICS_TABLES == 3);
+
+  if (!rc &&
+      (stat_table_intact.check(tables[TABLE_STAT].table, &table_stat_def) ||
+       stat_table_intact.check(tables[COLUMN_STAT].table, &column_stat_def) ||
+       stat_table_intact.check(tables[INDEX_STAT].table, &index_stat_def)))
+  {
+    close_system_tables(thd, backup);
+    rc= 1;
+  }
+
+  return rc;
+} 
 
 
 /**
@@ -2725,10 +2867,7 @@ int update_statistics_for_table(THD *thd, TABLE *table)
   DEBUG_SYNC(thd, "statistics_update_start");
 
   if (open_stat_tables(thd, tables, &open_tables_backup, TRUE))
-  {
-    thd->clear_error();
     DBUG_RETURN(rc);
-  }
    
   save_binlog_format= thd->set_current_stmt_binlog_format_stmt();
 
@@ -3156,10 +3295,7 @@ int delete_statistics_for_table(THD *thd, LEX_STRING *db, LEX_STRING *tab)
   DBUG_ENTER("delete_statistics_for_table");
    
   if (open_stat_tables(thd, tables, &open_tables_backup, TRUE))
-  {
-    thd->clear_error();
     DBUG_RETURN(rc);
-  }
 
   save_binlog_format= thd->set_current_stmt_binlog_format_stmt();
 
@@ -3398,10 +3534,7 @@ int rename_table_in_stat_tables(THD *thd, LEX_STRING *db, LEX_STRING *tab,
   DBUG_ENTER("rename_table_in_stat_tables");
    
   if (open_stat_tables(thd, tables, &open_tables_backup, TRUE))
-  {
-    thd->clear_error();
-    DBUG_RETURN(rc);
-  }
+    DBUG_RETURN(0); // not an error
 
   save_binlog_format= thd->set_current_stmt_binlog_format_stmt();
 
diff --git a/sql/sql_statistics.h b/sql/sql_statistics.h
index 20b2eb66449..f46583839d1 100644
--- a/sql/sql_statistics.h
+++ b/sql/sql_statistics.h
@@ -52,7 +52,8 @@ enum enum_table_stat_col
 {
   TABLE_STAT_DB_NAME,
   TABLE_STAT_TABLE_NAME,
-  TABLE_STAT_CARDINALITY
+  TABLE_STAT_CARDINALITY,
+  TABLE_STAT_N_FIELDS
 };
 
 enum enum_column_stat_col
@@ -67,7 +68,8 @@ enum enum_column_stat_col
   COLUMN_STAT_AVG_FREQUENCY,
   COLUMN_STAT_HIST_SIZE,
   COLUMN_STAT_HIST_TYPE,
-  COLUMN_STAT_HISTOGRAM
+  COLUMN_STAT_HISTOGRAM,
+  COLUMN_STAT_N_FIELDS
 };
 
 enum enum_index_stat_col
@@ -76,7 +78,8 @@ enum enum_index_stat_col
   INDEX_STAT_TABLE_NAME,
   INDEX_STAT_INDEX_NAME,
   INDEX_STAT_PREFIX_ARITY,
-  INDEX_STAT_AVG_FREQUENCY
+  INDEX_STAT_AVG_FREQUENCY,
+  INDEX_STAT_N_FIELDS
 };
 
 inline
diff --git a/sql/table.cc b/sql/table.cc
index 85ffd560992..686b3569a97 100644
--- a/sql/table.cc
+++ b/sql/table.cc
@@ -3861,6 +3861,15 @@ Table_check_intact::check(TABLE *table, const TABLE_FIELD_DEF *table_def)
 }
 
 
+void Table_check_intact_log_error::report_error(uint, const char *fmt, ...)
+{
+  va_list args;
+  va_start(args, fmt);
+  error_log_print(ERROR_LEVEL, fmt, args);
+  va_end(args);
+}
+
+
 /**
   Traverse portion of wait-for graph which is reachable through edge
   represented by this flush ticket in search for deadlocks.
diff --git a/sql/table.h b/sql/table.h
index eb4076e02e2..093c357e575 100644
--- a/sql/table.h
+++ b/sql/table.h
@@ -524,6 +524,16 @@ public:
 };
 
 
+/*
+  If the table isn't valid, report the error to the server log only.
+*/
+class Table_check_intact_log_error : public Table_check_intact
+{
+protected:
+  void report_error(uint, const char *fmt, ...);
+};
+
+
 /**
   Class representing the fact that some thread waits for table
   share to be flushed. Is used to represent information about