List:Commits« Previous MessageNext Message »
From:Alexander Nozdrin Date:April 30 2009 3:31pm
Subject:bzr commit into mysql-6.0-falcon branch (alik:2768) Bug#43138
View as plain text  
#At file:///mnt/raid/alik/MySQL/bzr/bug43138/6.0-rt-bug43138.3/ based on revid:alik@stripped

 2768 Alexander Nozdrin	2009-04-30
      Fix for Bug#43138: DROP DATABASE failure does not clean up message list.
      
      The problem was that the high-level function mysql_rm_db() invoked
      low-level mysql_rm_table_part2(), which reported low-level error
      (Unknown table) if SE refused to delete a table. Also when
      mysql_rm_table_part2() reported an error, it didn't add corresponding
      warning into the list (because it is used from other places where such
      behaviour is required).
      
      The fix is to
        1. Remove no_warnings_for_error usage from sql_table.cc
        2. Improve internal error handler support in THD, so that
           a stack of error handlers is allowed.
        3. Create an internal error handler (Drop_table_error_handler)
           to silence useless warnings.
        4. Use the handler in DROP DATABASE and DROP TABLE statements.

    modified:
      mysql-test/r/drop.result
      mysql-test/t/drop.test
      mysql-test/t/myisam-system.test
      sql/sql_class.cc
      sql/sql_class.h
      sql/sql_db.cc
      sql/sql_table.cc
=== modified file 'mysql-test/r/drop.result'
--- a/mysql-test/r/drop.result	2008-12-24 10:48:24 +0000
+++ b/mysql-test/r/drop.result	2009-04-30 15:31:30 +0000
@@ -141,3 +141,28 @@ Error	1146	Table 'mysql.proc' doesn't ex
 # --
 
 End of 5.1 tests
+
+# --
+# -- Bug#43138: DROP DATABASE failure does not clean up message list.
+# --
+
+DROP DATABASE IF EXISTS mysql_test;
+
+CREATE DATABASE mysql_test;
+CREATE TABLE mysql_test.t1(a INT);
+CREATE TABLE mysql_test.t2(b INT);
+CREATE TABLE mysql_test.t3(c INT);
+
+SET SESSION DEBUG = "+d,bug43138";
+
+DROP DATABASE mysql_test;
+Warnings:
+Error	1051	Unknown table 't3'
+Error	1051	Unknown table 't2'
+Error	1051	Unknown table 't1'
+
+SET SESSION DEBUG = "-d,bug43138";
+
+# --
+# -- End of Bug#43138.
+# --

=== modified file 'mysql-test/t/drop.test'
--- a/mysql-test/t/drop.test	2009-03-04 13:48:55 +0000
+++ b/mysql-test/t/drop.test	2009-04-30 15:31:30 +0000
@@ -232,3 +232,36 @@ DROP DATABASE mysql_test;
 
 --echo
 --echo End of 5.1 tests
+
+###########################################################################
+--echo
+--echo # --
+--echo # -- Bug#43138: DROP DATABASE failure does not clean up message list.
+--echo # --
+--echo
+
+--disable_warnings
+DROP DATABASE IF EXISTS mysql_test;
+--enable_warnings
+
+--echo
+CREATE DATABASE mysql_test;
+CREATE TABLE mysql_test.t1(a INT);
+CREATE TABLE mysql_test.t2(b INT);
+CREATE TABLE mysql_test.t3(c INT);
+
+--echo
+SET SESSION DEBUG = "+d,bug43138";
+
+--echo
+DROP DATABASE mysql_test;
+
+--echo
+SET SESSION DEBUG = "-d,bug43138";
+
+--echo
+--echo # --
+--echo # -- End of Bug#43138.
+--echo # --
+
+###########################################################################

=== modified file 'mysql-test/t/myisam-system.test'
--- a/mysql-test/t/myisam-system.test	2007-12-12 17:19:24 +0000
+++ b/mysql-test/t/myisam-system.test	2009-04-30 15:31:30 +0000
@@ -12,11 +12,11 @@ let $MYSQLD_DATADIR= `select @@datadir`;
 drop table if exists t1;
 create table t1 (a int) engine=myisam;
 --remove_file $MYSQLD_DATADIR/test/t1.MYI
---error 1051,6
+--error ER_BAD_TABLE_ERROR,6
 drop table t1;
 create table t1 (a int) engine=myisam;
 --remove_file $MYSQLD_DATADIR/test/t1.MYD
---error 1105,6,29
+--error ER_BAD_TABLE_ERROR,6,29
 drop table t1;
---error 1051
+--error ER_BAD_TABLE_ERROR
 drop table t1;

=== modified file 'sql/sql_class.cc'
--- a/sql/sql_class.cc	2009-04-03 15:14:49 +0000
+++ b/sql/sql_class.cc	2009-04-30 15:31:30 +0000
@@ -388,6 +388,32 @@ char *thd_security_context(THD *thd, cha
 }
 
 
+/**
+  Implementation of Internal_error_handler::handle_condition().
+  The reason in having this implementation is to silence technical low-level
+  warnings during DROP TABLE operation. Currently we don't want to expose
+  the following warnings during DROP TABLE:
+    - Some of table files are missed or invalid (the table is going to be
+      deleted anyway, so why bother that something was missed);
+    - A trigger associated with the table does not have DEFINER (One of the
+      MySQL specifics now is that triggers are loaded for the table being
+      dropped. So, we may have a warning that trigger does not have DEFINER
+      attribute during DROP TABLE operation).
+
+  @return TRUE if the condition is handled.
+*/
+bool Drop_table_error_handler::handle_condition(THD *thd,
+                                                uint sql_errno,
+                                                const char *sqlstate,
+                                                MYSQL_ERROR::enum_warning_level
+                                                level,
+                                                const char *msg,
+                                                MYSQL_ERROR **condition)
+{
+  return sql_errno == EE_DELETE || sql_errno == ER_TRG_NO_DEFINER;
+}
+
+
 THD::THD()
    :Statement(&main_lex, &main_mem_root, CONVENTIONAL_EXECUTION,
               /* statement id */ 0),
@@ -548,12 +574,15 @@ THD::THD()
 
 void THD::push_internal_handler(Internal_error_handler *handler)
 {
-  /*
-    TODO: The current implementation is limited to 1 handler at a time only.
-    THD and sp_rcontext need to be modified to use a common handler stack.
-  */
-  DBUG_ASSERT(m_internal_handler == NULL);
-  m_internal_handler= handler;
+  if (m_internal_handler)
+  {
+    handler->m_prev_internal_handler= m_internal_handler;
+    m_internal_handler= handler;
+  }
+  else
+  {
+    m_internal_handler= handler;
+  }
 }
 
 
@@ -563,17 +592,23 @@ bool THD::handle_condition(uint sql_errn
                            const char* msg,
                            MYSQL_ERROR ** cond_hdl)
 {
-  if (m_internal_handler)
+  if (!m_internal_handler)
   {
-    return m_internal_handler->handle_condition(this,
-                                                sql_errno,
-                                                sqlstate,
-                                                level,
-                                                msg,
-                                                cond_hdl);
+    *cond_hdl= NULL;
+    return FALSE;
+  }
+
+  for (Internal_error_handler *error_handler= m_internal_handler;
+       error_handler;
+       error_handler= m_internal_handler->m_prev_internal_handler)
+  {
+    if (error_handler-> handle_condition(this, sql_errno, sqlstate, level, msg,
+                                         cond_hdl))
+    {
+      return TRUE;
+    }
   }
 
-  *cond_hdl= NULL;
   return FALSE;
 }
 
@@ -581,7 +616,7 @@ bool THD::handle_condition(uint sql_errn
 void THD::pop_internal_handler()
 {
   DBUG_ASSERT(m_internal_handler != NULL);
-  m_internal_handler= NULL;
+  m_internal_handler= m_internal_handler->m_prev_internal_handler;
 }
 
 void THD::raise_error(uint sql_errno)

=== modified file 'sql/sql_class.h'
--- a/sql/sql_class.h	2009-04-14 23:53:44 +0000
+++ b/sql/sql_class.h	2009-04-30 15:31:30 +0000
@@ -1110,7 +1110,10 @@ show_system_thread(enum_thread_type thre
 class Internal_error_handler
 {
 protected:
-  Internal_error_handler() {}
+  Internal_error_handler() :
+    m_prev_internal_handler(NULL)
+  { }
+
   virtual ~Internal_error_handler() {}
 
 public:
@@ -1145,6 +1148,36 @@ public:
                                 const char* msg,
                                 MYSQL_ERROR ** cond_hdl) = 0;
 
+private:
+  Internal_error_handler *m_prev_internal_handler;
+  friend class THD;
+};
+
+
+/**
+  This class is an internal error handler implementation for DROP DATABASE
+  and DROP TABLE statements. The thing is that there may be warnings during
+  execution of these statements, which should not be exposed to the user.
+  This class is intended to silence such warnings.
+*/
+
+class Drop_table_error_handler : public Internal_error_handler
+{
+public:
+  Drop_table_error_handler(Internal_error_handler *err_handler)
+    :m_err_handler(err_handler)
+  { }
+
+public:
+  virtual bool handle_condition(THD *thd,
+                                uint sql_errno,
+                                const char *sqlstate,
+                                MYSQL_ERROR::enum_warning_level level,
+                                const char *msg,
+                                MYSQL_ERROR **condition);
+
+private:
+  Internal_error_handler *m_err_handler;
 };
 
 /**
@@ -2282,6 +2315,9 @@ public:
   thd_scheduler scheduler;
 
 public:
+  inline Internal_error_handler *get_internal_handler()
+  { return m_internal_handler; }
+
   /**
     Add an internal error handler to the thread execution context.
     @param handler the exception handler to add

=== modified file 'sql/sql_db.cc'
--- a/sql/sql_db.cc	2009-03-17 20:26:16 +0000
+++ b/sql/sql_db.cc	2009-04-30 15:31:30 +0000
@@ -913,6 +913,9 @@ bool mysql_rm_db(THD *thd,char *db,bool 
   }
   else
   {
+    Drop_table_error_handler err_handler(thd->get_internal_handler());
+    thd->push_internal_handler(&err_handler);
+
     error= -1;
     /*
       We temporarily disable the binary log while dropping the objects
@@ -945,6 +948,8 @@ bool mysql_rm_db(THD *thd,char *db,bool 
       error = 0;
       reenable_binlog(thd);
     }
+
+    thd->pop_internal_handler();
   }
   if (!silent && deleted>=0)
   {

=== modified file 'sql/sql_table.cc'
--- a/sql/sql_table.cc	2009-04-03 15:14:49 +0000
+++ b/sql/sql_table.cc	2009-04-30 15:31:30 +0000
@@ -1498,6 +1498,8 @@ bool mysql_rm_table(THD *thd,TABLE_LIST 
                     my_bool drop_temporary)
 {
   bool error, need_start_waiting= FALSE;
+  Drop_table_error_handler err_handler(thd->get_internal_handler());
+
   DBUG_ENTER("mysql_rm_table");
 
   /* mark for close and remove all cached entries */
@@ -1509,7 +1511,9 @@ bool mysql_rm_table(THD *thd,TABLE_LIST 
       DBUG_RETURN(TRUE);
   }
 
+  thd->push_internal_handler(&err_handler);
   error= mysql_rm_table_part2(thd, tables, if_exists, drop_temporary, 0, 0);
+  thd->pop_internal_handler();
 
   if (need_start_waiting)
     start_waiting_global_read_lock(thd);
@@ -1649,9 +1653,6 @@ int mysql_rm_table_part2(THD *thd, TABLE
     }
   }
 
-  /* Don't give warnings for not found errors, as we already generate notes */
-  thd->no_warnings_for_error= 1;
-
   for (table= tables; table; table= table->next_local)
   {
     char *db=table->db;
@@ -1793,11 +1794,18 @@ int mysql_rm_table_part2(THD *thd, TABLE
         wrong_tables.append(',');
       wrong_tables.append(String(table->table_name,system_charset_info));
     }
+
+    DBUG_EXECUTE_IF("bug43138",
+                    my_printf_error(ER_BAD_TABLE_ERROR,
+                                    ER(ER_BAD_TABLE_ERROR), MYF(0),
+                                    table->table_name););
+
     DBUG_PRINT("table", ("table: %p  s: %p", table->table,
                          table->table ? table->table->s : (TABLE_SHARE *)-1));
   }
   thd->thread_specific_used|= tmp_table_deleted;
   error= 0;
+
   if (wrong_tables.length())
   {
     if (!foreign_key_error)
@@ -1890,7 +1898,6 @@ err:
   }
 
 end:
-  thd->no_warnings_for_error= 0;
   DBUG_RETURN(error);
 }
 


Attachment: [text/bzr-bundle] bzr/alik@sun.com-20090430153130-950uzfbgz57t15gd.bundle
Thread
bzr commit into mysql-6.0-falcon branch (alik:2768) Bug#43138Alexander Nozdrin30 Apr