List:Commits« Previous MessageNext Message »
From:Alexander Nozdrin Date:March 30 2011 4:04pm
Subject:bzr commit into mysql-trunk branch (alexander.nozdrin:3350) Bug#11763166
View as plain text  
#At file:///home/alik/MySQL/bzr/00/bug55847/mysql-trunk-bug55847/ based on revid:magne.mahre@stripped

 3350 Alexander Nozdrin	2011-03-30
      A patch for Bug#11763166 (55847: SHOW WARNINGS returns empty
      result set when SQLEXCEPTION is active.
      
      The problem was in a hackish THD::no_warnings_for_error attribute.
      When it was set, an error was not written to Warning_info -- only
      Diagnostics_area state was changed. That means, Diagnostics_area
      might contain error state, which is not present in Warning_info.
      
      The user-visible problem was that in some cases SHOW WARNINGS
      returned empty result set (i.e. there were no warnings) while
      the previous SQL statement failed. According to the MySQL
      protocol errors must be presented in warning list.
      
      The main idea of this patch is to remove THD::no_warnings_for_error.
      There were few places where it was used:
        - sql_admin.cc, handling of REPAIR TABLE USE_FRM.
        - sql_show.cc, when calling fill_schema_table_from_frm().
        - sql_show.cc, when calling fill_table().
      The fix is to either use internal-error-handlers, or to use
      temporary Warning_info storing warnings, which might be ignored.
      
      This patch is needed to fix Bug 11763162 (55843).

    modified:
      mysql-test/r/information_schema.result
      mysql-test/r/warnings.result
      mysql-test/t/warnings.test
      sql/sql_admin.cc
      sql/sql_class.cc
      sql/sql_class.h
      sql/sql_error.cc
      sql/sql_error.h
      sql/sql_parse.cc
      sql/sql_show.cc
      sql/sql_trigger.cc
=== modified file 'mysql-test/r/information_schema.result'
--- a/mysql-test/r/information_schema.result	2011-03-11 18:53:12 +0000
+++ b/mysql-test/r/information_schema.result	2011-03-30 16:04:37 +0000
@@ -1883,10 +1883,6 @@ DEFINER	root@localhost
 SECURITY_TYPE	DEFINER
 CHARACTER_SET_CLIENT	latin1
 COLLATION_CONNECTION	latin1_swedish_ci
-Warnings:
-Level	Warning
-Code	1356
-Message	View 'test.v1' references invalid table(s) or column(s) or function(s) or definer/invoker of view lack rights to use them
 unlock tables;
 #
 # Cleanup.

=== modified file 'mysql-test/r/warnings.result'
--- a/mysql-test/r/warnings.result	2010-08-30 06:38:09 +0000
+++ b/mysql-test/r/warnings.result	2011-03-30 16:04:37 +0000
@@ -316,3 +316,25 @@ SHOW ERRORS;
 Level	Code	Message
 Error	1051	Unknown table 'test.t1'
 End of 5.0 tests
+
+-- Bug#55847
+
+DROP TABLE IF EXISTS t1;
+DROP FUNCTION IF EXISTS f1;
+CREATE TABLE t1(a INT UNIQUE);
+CREATE FUNCTION f1(x INT) RETURNS INT
+BEGIN
+INSERT INTO t1 VALUES(x);
+INSERT INTO t1 VALUES(x);
+RETURN x;
+END|
+
+SHOW TABLES WHERE f1(11) = 11;
+ERROR 23000: Duplicate entry '11' for key 'a'
+
+SHOW WARNINGS;
+Level	Code	Message
+Error	1062	Duplicate entry '11' for key 'a'
+
+DROP TABLE t1;
+DROP FUNCTION f1;

=== modified file 'mysql-test/t/warnings.test'
--- a/mysql-test/t/warnings.test	2009-11-13 10:17:53 +0000
+++ b/mysql-test/t/warnings.test	2011-03-30 16:04:37 +0000
@@ -228,3 +228,43 @@ DROP TABLE t1;
 SHOW ERRORS;
 
 --echo End of 5.0 tests
+
+#
+# Bug#55847: SHOW WARNINGS returns empty result set when SQLEXCEPTION is active
+#
+
+--echo
+--echo -- Bug#55847
+--echo
+
+--disable_warnings
+DROP TABLE IF EXISTS t1;
+DROP FUNCTION IF EXISTS f1;
+--enable_warnings
+
+CREATE TABLE t1(a INT UNIQUE);
+
+delimiter |;
+
+CREATE FUNCTION f1(x INT) RETURNS INT
+BEGIN
+  INSERT INTO t1 VALUES(x);
+  INSERT INTO t1 VALUES(x);
+  RETURN x;
+END|
+
+delimiter ;|
+
+--echo
+
+--error ER_DUP_ENTRY
+SHOW TABLES WHERE f1(11) = 11;
+
+--echo
+
+SHOW WARNINGS;
+
+--echo
+
+DROP TABLE t1;
+DROP FUNCTION f1;

=== modified file 'sql/sql_admin.cc'
--- a/sql/sql_admin.cc	2011-03-08 09:21:39 +0000
+++ b/sql/sql_admin.cc	2011-03-30 16:04:37 +0000
@@ -263,7 +263,7 @@ static bool mysql_admin_table(THD* thd,
                               const char *operator_name,
                               thr_lock_type lock_type,
                               bool open_for_modify,
-                              bool no_warnings_for_error,
+                              bool repair_table_use_frm,
                               uint extra_open_options,
                               int (*prepare_func)(THD *, TABLE_LIST *,
                                                   HA_CHECK_OPT *),
@@ -331,18 +331,43 @@ static bool mysql_admin_table(THD* thd,
       lex->query_tables= table;
       lex->query_tables_last= &table->next_global;
       lex->query_tables_own_last= 0;
-      /*
-        Under locked tables, we know that the table can be opened,
-        so any errors opening the table are logical errors.
-        In these cases it makes sense to report them.
-      */
-      if (!thd->locked_tables_mode)
-        thd->no_warnings_for_error= no_warnings_for_error;
+
       if (view_operator_func == NULL)
         table->required_type=FRMTYPE_TABLE;
 
-      open_error= open_and_lock_tables(thd, table, TRUE, 0);
-      thd->no_warnings_for_error= 0;
+      if (!thd->locked_tables_mode && repair_table_use_frm)
+      {
+        /*
+          If we're not under LOCK TABLES and we're executing REPAIR TABLE
+          USE_FRM, we need to ignore errors from open_and_lock_tables().
+          REPAIR TABLE USE_FRM is a heavy weapon used when a table is
+          critically damaged, so open_and_lock_tables() will most likely
+          report errors. Those errors are not interesting for the user
+          because it's already known that the table is badly damaged.
+        */
+
+        Warning_info wi(thd->query_id);
+        Warning_info *wi_saved= thd->warning_info;
+
+        thd->warning_info= &wi;
+
+        open_error= open_and_lock_tables(thd, table, TRUE, 0);
+
+        thd->warning_info= wi_saved;
+      }
+      else
+      {
+        /*
+          It's assumed that even if it is REPAIR TABLE USE_FRM, the table
+          can be opened if we're under LOCK TABLES (otherwise LOCK TABLES
+          would fail). Thus, the only errors we could have from
+          open_and_lock_tables() are logical ones, like incorrect locking
+          mode. It does make sense for the user to see such errors.
+        */
+
+        open_error= open_and_lock_tables(thd, table, TRUE, 0);
+      }
+
       table->next_global= save_next_global;
       table->next_local= save_next_local;
       thd->open_options&= ~extra_open_options;

=== modified file 'sql/sql_class.cc'
--- a/sql/sql_class.cc	2011-03-24 08:00:03 +0000
+++ b/sql/sql_class.cc	2011-03-30 16:04:37 +0000
@@ -586,7 +586,7 @@ THD::THD(bool enable_plugins)
   client_capabilities= 0;                       // minimalistic client
   ull=0;
   system_thread= NON_SYSTEM_THREAD;
-  cleanup_done= abort_on_warning= no_warnings_for_error= 0;
+  cleanup_done= abort_on_warning= 0;
   peer_port= 0;					// For SHOW PROCESSLIST
   transaction.m_pending_rows_event= 0;
   transaction.on= 1;
@@ -859,10 +859,6 @@ MYSQL_ERROR* THD::raise_condition(uint s
 
   query_cache_abort(&query_cache_tls);
 
-  /* FIXME: broken special case */
-  if (no_warnings_for_error && (level == MYSQL_ERROR::WARN_LEVEL_ERROR))
-    DBUG_RETURN(NULL);
-
   /* When simulating OOM, skip writing to error log to avoid mtr errors */
   DBUG_EXECUTE_IF("simulate_out_of_memory", DBUG_RETURN(NULL););
 

=== modified file 'sql/sql_class.h'
--- a/sql/sql_class.h	2011-03-24 08:00:03 +0000
+++ b/sql/sql_class.h	2011-03-30 16:04:37 +0000
@@ -2178,7 +2178,6 @@ public:
   bool       enable_slow_log;   /* enable slow log for current statement */
   bool	     abort_on_warning;
   bool 	     got_warning;       /* Set on call to push_warning() */
-  bool	     no_warnings_for_error; /* no warnings on call to my_error() */
   /* set during loop of derived table processing */
   bool       derived_tables_processing;
   my_bool    tablespace_op;	/* This is TRUE in DISCARD/IMPORT TABLESPACE */
@@ -2972,6 +2971,7 @@ private:
 
   /** The current internal error handler for this thread, or NULL. */
   Internal_error_handler *m_internal_handler;
+
   /**
     The lex to hold the parsed tree of conventional (non-prepared) queries.
     Whereas for prepared and stored procedure statements we use an own lex

=== modified file 'sql/sql_error.cc'
--- a/sql/sql_error.cc	2011-03-09 20:54:55 +0000
+++ b/sql/sql_error.cc	2011-03-30 16:04:37 +0000
@@ -558,6 +558,20 @@ MYSQL_ERROR *Warning_info::push_warning(
   return cond;
 }
 
+MYSQL_ERROR *Warning_info::push_warning(THD *thd, const MYSQL_ERROR *sql_condition)
+{
+  MYSQL_ERROR *new_condition= push_warning(thd,
+                                           sql_condition->get_sql_errno(),
+                                           sql_condition->get_sqlstate(),
+                                           sql_condition->get_level(),
+                                           sql_condition->get_message_text());
+
+  if (new_condition)
+    new_condition->copy_opt_attributes(sql_condition);
+
+  return new_condition;
+}
+
 /*
   Push the warning to error list if there is still room in the list
 

=== modified file 'sql/sql_error.h'
--- a/sql/sql_error.h	2011-03-09 20:54:55 +0000
+++ b/sql/sql_error.h	2011-03-30 16:04:37 +0000
@@ -383,19 +383,13 @@ public:
   void append_warnings(THD *thd, List<MYSQL_ERROR> *src)
   {
     MYSQL_ERROR *err;
-    MYSQL_ERROR *copy;
     List_iterator_fast<MYSQL_ERROR> it(*src);
     /*
       Don't use ::push_warning() to avoid invocation of condition
       handlers or escalation of warnings to errors.
     */
     while ((err= it++))
-    {
-      copy= Warning_info::push_warning(thd, err->get_sql_errno(), err->get_sqlstate(),
-                                       err->get_level(), err->get_message_text());
-      if (copy)
-        copy->copy_opt_attributes(err);
-    }
+      Warning_info::push_warning(thd, err);
   }
 
   /**
@@ -461,6 +455,9 @@ public:
                             MYSQL_ERROR::enum_warning_level level,
                             const char* msg);
 
+  /** Add a new condition to the current list. */
+  MYSQL_ERROR *push_warning(THD *thd, const MYSQL_ERROR *sql_condition);
+
   /**
     Set the read only status for this statement area.
     This is a privileged operation, reserved for the implementation of

=== modified file 'sql/sql_parse.cc'
--- a/sql/sql_parse.cc	2011-03-17 17:39:31 +0000
+++ b/sql/sql_parse.cc	2011-03-30 16:04:37 +0000
@@ -7335,10 +7335,20 @@ bool parse_sql(THD *thd,
 
   bool mysql_parse_status= MYSQLparse(thd) != 0;
 
-  /* Check that if MYSQLparse() failed, thd->is_error() is set. */
+  /*
+    Check that if MYSQLparse() failed either thd->is_error() is set, or an
+    internal error handler is set.
+
+    The assert will not catch a situation where parsing fails without an
+    error reported if an error handler exists. The problem is that the
+    error handler might have intercepted the error, so thd->is_error() is
+    not set. However, there is no way to be 100% sure here (the error
+    handler might be for other errors than parsing one).
+  */
 
   DBUG_ASSERT(!mysql_parse_status ||
-              (mysql_parse_status && thd->is_error()));
+              (mysql_parse_status && thd->is_error()) ||
+              (mysql_parse_status && thd->get_internal_handler()));
 
   /* Reset parser state. */
 

=== modified file 'sql/sql_show.cc'
--- a/sql/sql_show.cc	2011-03-28 08:10:39 +0000
+++ b/sql/sql_show.cc	2011-03-30 16:04:37 +0000
@@ -3421,6 +3421,45 @@ 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
+{
+public:
+  bool handle_condition(THD *thd,
+                        uint sql_errno,
+                        const char* sqlstate,
+                        MYSQL_ERROR::enum_warning_level level,
+                        const char* msg,
+                        MYSQL_ERROR ** cond_hdl)
+  {
+    if (sql_errno == ER_PARSE_ERROR ||
+        sql_errno == ER_TRG_NO_DEFINER ||
+        sql_errno == ER_TRG_NO_CREATION_CTX)
+      return true;
+
+    return false;
+  }
+};
+
+
+
+/**
   @brief          Fill I_S tables whose data are retrieved
                   from frm files and storage engine
 
@@ -3569,7 +3608,6 @@ int get_all_tables(THD *thd, TABLE_LIST
         acl_get(sctx->host, sctx->ip, sctx->priv_user, db_name->str, 0))
 #endif
     {
-      thd->no_warnings_for_error= 1;
       List<LEX_STRING> table_names;
       int res= make_table_name_list(thd, &table_names, lex,
                                     &lookup_field_vals,
@@ -3618,9 +3656,23 @@ int get_all_tables(THD *thd, TABLE_LIST
             if (!(table_open_method & ~OPEN_FRM_ONLY) &&
                 !with_i_schema)
             {
-              if (!fill_schema_table_from_frm(thd, tables, schema_table, db_name,
-                                              table_name, schema_table_idx,
-                                              can_deadlock))
+              /*
+                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,
+                                                  can_deadlock);
+
+              thd->pop_internal_handler();
+
+              if (!res)
                 continue;
             }
 
@@ -3630,7 +3682,6 @@ int get_all_tables(THD *thd, TABLE_LIST
               Set the parent lex of 'sel' because it is needed by
               sel.init_query() which is called inside make_table_list.
             */
-            thd->no_warnings_for_error= 1;
             sel.parent_lex= lex;
             /* db_name can be changed in make_table_list() func */
             if (!thd->make_lex_string(&orig_db_name, db_name->str,
@@ -6669,6 +6720,84 @@ int make_schema_select(THD *thd, SELECT_
 }
 
 
+/**
+  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)
+{
+  Warning_info wi(thd->query_id);
+  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
 
@@ -6691,7 +6820,6 @@ bool get_schema_tables_result(JOIN *join
   bool result= 0;
   DBUG_ENTER("get_schema_tables_result");
 
-  thd->no_warnings_for_error= 1;
   for (JOIN_TAB *tab= join->join_tab; tab < tmp_join_tab; tab++)
   {  
     if (!tab->table || !tab->table->pos_in_table_list)
@@ -6742,8 +6870,7 @@ bool get_schema_tables_result(JOIN *join
       else
         table_list->table->file->stats.records= 0;
 
-      if (table_list->schema_table->fill_table(thd, table_list,
-                                               tab->select_cond))
+      if (do_fill_table(thd, table_list, tab))
       {
         result= 1;
         join->error= 1;
@@ -6755,7 +6882,6 @@ bool get_schema_tables_result(JOIN *join
       table_list->schema_table_state= executed_place;
     }
   }
-  thd->no_warnings_for_error= 0;
   DBUG_RETURN(result);
 }
 

=== modified file 'sql/sql_trigger.cc'
--- a/sql/sql_trigger.cc	2011-03-10 08:33:54 +0000
+++ b/sql/sql_trigger.cc	2011-03-30 16:04:37 +0000
@@ -1225,13 +1225,12 @@ bool Table_triggers_list::check_n_load(T
 
           DBUG_RETURN(1); // EOM
         }
-        
-        if (!thd->no_warnings_for_error)
-          push_warning_printf(thd, MYSQL_ERROR::WARN_LEVEL_WARN,
-                              ER_TRG_NO_CREATION_CTX,
-                              ER(ER_TRG_NO_CREATION_CTX),
-                              (const char*) db,
-                              (const char*) table_name);
+
+        push_warning_printf(thd, MYSQL_ERROR::WARN_LEVEL_WARN,
+                            ER_TRG_NO_CREATION_CTX,
+                            ER(ER_TRG_NO_CREATION_CTX),
+                            (const char*) db,
+                            (const char*) table_name);
 
         if (!(trg_client_cs_name= alloc_lex_string(&table->mem_root)) ||
             !(trg_connection_cl_name= alloc_lex_string(&table->mem_root)) ||
@@ -1362,12 +1361,12 @@ bool Table_triggers_list::check_n_load(T
             MySQL, which does not support triggers definers. We should emit
             warning here.
           */
-          if (!thd->no_warnings_for_error)
-            push_warning_printf(thd, MYSQL_ERROR::WARN_LEVEL_WARN,
-                                ER_TRG_NO_DEFINER, ER(ER_TRG_NO_DEFINER),
-                                (const char*) db,
-                                (const char*) sp->m_name.str);
-          
+
+          push_warning_printf(thd, MYSQL_ERROR::WARN_LEVEL_WARN,
+                              ER_TRG_NO_DEFINER, ER(ER_TRG_NO_DEFINER),
+                              (const char*) db,
+                              (const char*) sp->m_name.str);
+
           /*
             Set definer to the '' to correct displaying in the information
             schema.


Attachment: [text/bzr-bundle] bzr/alexander.nozdrin@oracle.com-20110330160437-kjhdvrmzy4nn2z1f.bundle
Thread
bzr commit into mysql-trunk branch (alexander.nozdrin:3350) Bug#11763166Alexander Nozdrin30 Mar
  • Re: bzr commit into mysql-trunk branch (alexander.nozdrin:3350) Bug#11763166Davi Arnaut30 Mar