MDEV-4856 SQL_ERROR_LOG shows 1146 errors which didnt appear in mysql client.

    The fill_schema_table() function used to call get_table_share() for a table name in WHERE
    then clear the error list. That way plugins receive the superfluous error notification if it
    happens in it. Also the problem was that error handler didn't prevent the suppressed
    error message from logging anyway as the logging happens in THD::raise_condition
    before the handler call.
    Trigger_error_handler is remade into Warnings_only_error_handler, so it stores the error
    message in all cases in the thd->stmt_da.
    Then later the stored error is raised.

5 files changed, 49 insertions, 131 deletions

diff --git a/mysql-test/suite/plugins/r/sql_error_log.result b/mysql-test/suite/plugins/r/sql_error_log.result
index f3e6716302b..3086b1f2d1f 100644
--- a/mysql-test/suite/plugins/r/sql_error_log.result
+++ b/mysql-test/suite/plugins/r/sql_error_log.result
@@ -33,6 +33,8 @@ insert into t1 values ('aa');
 ERROR 22007: Incorrect integer value: 'aa' for column 'id' at row 1
 SET SQL_MODE = '';
 drop table t1;
+SELECT TABLE_NAME FROM information_schema.TABLES WHERE TABLE_SCHEMA = 'not_exists' AND TABLE_NAME = 'not_exists';
+TABLE_NAME
 uninstall plugin SQL_ERROR_LOG;
 Warnings:
 Warning	1620	Plugin is busy and will be uninstalled on shutdown
diff --git a/mysql-test/suite/plugins/t/sql_error_log.test b/mysql-test/suite/plugins/t/sql_error_log.test
index 8d04720aac3..a28e2d2e259 100644
--- a/mysql-test/suite/plugins/t/sql_error_log.test
+++ b/mysql-test/suite/plugins/t/sql_error_log.test
@@ -46,6 +46,8 @@ insert into t1 values ('aa');
 SET SQL_MODE = '';
 drop table t1;
 
+SELECT TABLE_NAME FROM information_schema.TABLES WHERE TABLE_SCHEMA = 'not_exists' AND TABLE_NAME = 'not_exists';
+
 uninstall plugin SQL_ERROR_LOG;
 
 let $MYSQLD_DATADIR= `SELECT @@datadir`;
diff --git a/sql/sql_class.cc b/sql/sql_class.cc
index 91705de7c9b..16c484977d7 100644
--- a/sql/sql_class.cc
+++ b/sql/sql_class.cc
@@ -1145,7 +1145,6 @@ MYSQL_ERROR* THD::raise_condition(uint sql_errno,
     got_warning= 1;
     break;
   case MYSQL_ERROR::WARN_LEVEL_ERROR:
-    mysql_audit_general(this, MYSQL_AUDIT_GENERAL_ERROR, sql_errno, msg);
     break;
   default:
     DBUG_ASSERT(FALSE);
@@ -1156,6 +1155,8 @@ MYSQL_ERROR* THD::raise_condition(uint sql_errno,
 
   if (level == MYSQL_ERROR::WARN_LEVEL_ERROR)
   {
+    mysql_audit_general(this, MYSQL_AUDIT_GENERAL_ERROR, sql_errno, msg);
+
     is_slave_error=  1; // needed to catch query errors during replication
 
     if (! stmt_da->is_error())
diff --git a/sql/sql_class.h b/sql/sql_class.h
index dff9b7354cd..9365727393e 100644
--- a/sql/sql_class.h
+++ b/sql/sql_class.h
@@ -2608,8 +2608,11 @@ public:
     Clear the current error, if any.
     We do not clear is_fatal_error or is_fatal_sub_stmt_error since we
     assume this is never called if the fatal error is set.
+
     @todo: To silence an error, one should use Internal_error_handler
-    mechanism. In future this function will be removed.
+    mechanism. Issuing an error that can be possibly later "cleared" is not
+    compatible with other installed error handlers and audit plugins.
+    In future this function will be removed.
   */
   inline void clear_error()
   {
diff --git a/sql/sql_show.cc b/sql/sql_show.cc
index feeb96f280d..e07a904d416 100644
--- a/sql/sql_show.cc
+++ b/sql/sql_show.cc
@@ -4281,25 +4281,7 @@ end:
 }
 
 
-/**
-  Trigger_error_handler is intended to intercept and silence SQL conditions
-  that might happen during trigger loading for SHOW statements.
-  The potential SQL conditions are:
-
-    - ER_PARSE_ERROR -- this error is thrown if a trigger definition file
-      is damaged or contains invalid CREATE TRIGGER statement. That should
-      not happen in normal life.
-
-    - ER_TRG_NO_DEFINER -- this warning is thrown when we're loading a
-      trigger created/imported in/from the version of MySQL, which does not
-      support trigger definers.
-
-    - ER_TRG_NO_CREATION_CTX -- this warning is thrown when we're loading a
-      trigger created/imported in/from the version of MySQL, which does not
-      support trigger creation contexts.
-*/
-
-class Trigger_error_handler : public Internal_error_handler
+class Warnings_only_error_handler : public Internal_error_handler
 {
 public:
   bool handle_condition(THD *thd,
@@ -4314,12 +4296,16 @@ public:
         sql_errno == ER_TRG_NO_CREATION_CTX)
       return true;
 
-    return false;
+    if (level != MYSQL_ERROR::WARN_LEVEL_ERROR)
+      return false;
+
+    if (!thd->stmt_da->is_error())
+      thd->stmt_da->set_error_status(thd, sql_errno, msg, sqlstate);
+    return true; // handled!
   }
 };
 
 
-
 /**
   @brief          Fill I_S tables whose data are retrieved
                   from frm files and storage engine
@@ -4517,25 +4503,11 @@ int get_all_tables(THD *thd, TABLE_LIST *tables, COND *cond)
             if (!(table_open_method & ~OPEN_FRM_ONLY) &&
                 !with_i_schema)
             {
-              /*
-                Here we need to filter out warnings, which can happen
-                during loading of triggers in fill_schema_table_from_frm(),
-                because we don't need those warnings to pollute output of
-                SELECT from I_S / SHOW-statements.
-              */
-
-              Trigger_error_handler err_handler;
-              thd->push_internal_handler(&err_handler);
-
-              int res= fill_schema_table_from_frm(thd, tables, schema_table,
-                                                  db_name, table_name,
-                                                  schema_table_idx,
-                                                  &open_tables_state_backup,
-                                                  can_deadlock);
-
-              thd->pop_internal_handler();
-
-              if (!res)
+              if (!fill_schema_table_from_frm(thd, tables, schema_table,
+                                              db_name, table_name,
+                                              schema_table_idx,
+                                              &open_tables_state_backup,
+                                              can_deadlock))
                 continue;
             }
 
@@ -7660,92 +7632,6 @@ int make_schema_select(THD *thd, SELECT_LEX *sel,
 }
 
 
-/**
-  Fill INFORMATION_SCHEMA-table, leave correct Diagnostics_area /
-  Warning_info state after itself.
-
-  This function is a wrapper around ST_SCHEMA_TABLE::fill_table(), which
-  may "partially silence" some errors. The thing is that during
-  fill_table() many errors might be emitted. These errors stem from the
-  nature of fill_table().
-
-  For example, SELECT ... FROM INFORMATION_SCHEMA.xxx WHERE TABLE_NAME = 'xxx'
-  results in a number of 'Table <db name>.xxx does not exist' errors,
-  because fill_table() tries to open the 'xxx' table in every possible
-  database.
-
-  Those errors are cleared (the error status is cleared from
-  Diagnostics_area) inside fill_table(), but they remain in Warning_info
-  (Warning_info is not cleared because it may contain useful warnings).
-
-  This function is responsible for making sure that Warning_info does not
-  contain warnings corresponding to the cleared errors.
-
-  @note: THD::no_warnings_for_error used to be set before calling
-  fill_table(), thus those errors didn't go to Warning_info. This is not
-  the case now (THD::no_warnings_for_error was eliminated as a hack), so we
-  need to take care of those warnings here.
-
-  @param thd            Thread context.
-  @param table_list     I_S table.
-  @param join_table     JOIN/SELECT table.
-
-  @return Error status.
-  @retval TRUE Error.
-  @retval FALSE Success.
-*/
-static bool do_fill_table(THD *thd,
-                          TABLE_LIST *table_list,
-                          JOIN_TAB *join_table)
-{
-  // NOTE: fill_table() may generate many "useless" warnings, which will be
-  // ignored afterwards. On the other hand, there might be "useful"
-  // warnings, which should be presented to the user. Warning_info usually
-  // stores no more than THD::variables.max_error_count warnings.
-  // The problem is that "useless warnings" may occupy all the slots in the
-  // Warning_info, so "useful warnings" get rejected. In order to avoid
-  // that problem we create a Warning_info instance, which is capable of
-  // storing "unlimited" number of warnings.
-  Warning_info wi(thd->query_id, true);
-  Warning_info *wi_saved= thd->warning_info;
-
-  thd->warning_info= &wi;
-
-  bool res= table_list->schema_table->fill_table(
-    thd, table_list, join_table->select_cond);
-
-  thd->warning_info= wi_saved;
-
-  // Pass an error if any.
-
-  if (thd->stmt_da->is_error())
-  {
-    thd->warning_info->push_warning(thd,
-                                    thd->stmt_da->sql_errno(),
-                                    thd->stmt_da->get_sqlstate(),
-                                    MYSQL_ERROR::WARN_LEVEL_ERROR,
-                                    thd->stmt_da->message());
-  }
-
-  // Pass warnings (if any).
-  //
-  // Filter out warnings with WARN_LEVEL_ERROR level, because they
-  // correspond to the errors which were filtered out in fill_table().
-
-
-  List_iterator_fast<MYSQL_ERROR> it(wi.warn_list());
-  MYSQL_ERROR *err;
-
-  while ((err= it++))
-  {
-    if (err->get_level() != MYSQL_ERROR::WARN_LEVEL_ERROR)
-      thd->warning_info->push_warning(thd, err);
-  }
-
-  return res;
-}
-
-
 /*
   Fill temporary schema tables before SELECT
 
@@ -7767,6 +7653,8 @@ bool get_schema_tables_result(JOIN *join,
   bool result= 0;
   DBUG_ENTER("get_schema_tables_result");
 
+  Warnings_only_error_handler err_handler;
+  thd->push_internal_handler(&err_handler);
   for (JOIN_TAB *tab= first_linear_tab(join, WITH_CONST_TABLES); 
        tab; 
        tab= next_linear_tab(join, tab, WITHOUT_BUSH_ROOTS))
@@ -7819,20 +7707,42 @@ bool get_schema_tables_result(JOIN *join,
       else
         table_list->table->file->stats.records= 0;
 
-      if (do_fill_table(thd, table_list, tab))
+
+      if (table_list->schema_table->fill_table(thd, table_list,
+                                               tab->select_cond))
       {
         result= 1;
         join->error= 1;
         tab->read_record.table->file= table_list->table->file;
         table_list->schema_table_state= executed_place;
-        if (!thd->is_error())
-          my_error(ER_UNKNOWN_ERROR, MYF(0));
         break;
       }
       tab->read_record.table->file= table_list->table->file;
       table_list->schema_table_state= executed_place;
     }
   }
+  thd->pop_internal_handler();
+  if (thd->is_error())
+  {
+    /*
+      This hack is here, because I_S code uses thd->clear_error() a lot.
+      Which means, a Warnings_only_error_handler cannot handle the error
+      corectly as it does not know whether an error is real (e.g. caused
+      by tab->select_cond->val_int()) or will be cleared later.
+      Thus it ignores all errors, and the real one (that is, the error
+      that was not cleared) is pushed now.
+
+      It also means that an audit plugin cannot process the error correctly
+      either. See also thd->clear_error()
+    */
+    thd->warning_info->push_warning(thd,
+                                    thd->stmt_da->sql_errno(),
+                                    thd->stmt_da->get_sqlstate(),
+                                    MYSQL_ERROR::WARN_LEVEL_ERROR,
+                                    thd->stmt_da->message());
+  }
+  else if (result)
+    my_error(ER_UNKNOWN_ERROR, MYF(0));
   DBUG_RETURN(result);
 }