List:Commits« Previous MessageNext Message »
From:Dmitry Lenev Date:July 22 2011 12:35pm
Subject:bzr push into mysql-5.5 branch (Dmitry.Lenev:3476 to 3477) Bug#11754210
View as plain text  
 3477 Dmitry Lenev	2011-07-22
      Fix for bug #11754210 - "45777: CHECK TABLE DOESN'T
      SHOW ALL PROBLEMS FOR MERGE TABLE COMPLIANCE IN 5.1".
      
      The problem was that CHECK/REPAIR TABLE for a MERGE table which
      had several children missing or in wrong engine reported only
      issue with the first such table in its result-set. While in 5.0
      this statement returned the whole list of problematic tables.
      
      Ability to report problems for all children was lost during
      significant refactorings of MERGE code which were done as part
      of work on 5.1 and 5.5 releases.
      
      This patch restores status quo ante refactorings by changing
      code in such a way that:
      1) Failure to open child table due to its absence during CHECK/
         REPAIR TABLE for a MERGE table is not reported immediately
         when its absence is discovered in open_tables(). Instead
         handling/error reporting in such a situation is postponed
         until the moment when children are attached.
      2) Code performing attaching of children no longer stops when
         it encounters first problem with one of the children during
         CHECK/REPAIR TABLE. Instead it continues iteration through
         the child list until all problems caused by child absence/
         wrong engine are reported.
      
      Note that even after this change problem with mismatch of
      child/parent definition won't be reported if there is also
      another child missing, but this is how it was in 5.0 as well.
     @ mysql-test/r/merge.result
        Added test case for bug #11754210 - "45777: CHECK TABLE DOESN'T
        SHOW ALL PROBLEMS FOR MERGE TABLE COMPLIANCE IN 5.1".
        Adjusted results of existing tests to the fact that CHECK/REPAIR
        TABLE statements now try to report problems about missing table/
        wrong engine for all underlying tables, and to the fact that
        mismatch of parent/child definitions is always reported as an
        error and not a warning.
     @ mysql-test/t/merge.test
        Added test case for bug #11754210 - "45777: CHECK TABLE DOESN'T
        SHOW ALL PROBLEMS FOR MERGE TABLE COMPLIANCE IN 5.1".
     @ sql/sql_base.cc
        Changed code responsible for opening tables to ignore the fact
        that underlying tables of a MERGE table are missing, if this
        table is opened for CHECK/REPAIR TABLE.
        The absence of underlying tables in this case is now detected and
        appropriate error is reported at the point when child tables are
        attached. At this point we can produce full list of problematic
        child tables/errors to be returned as part of CHECK/REPAIR TABLE
        result-set.
     @ storage/myisammrg/ha_myisammrg.cc
        Changed myisammrg_attach_children_callback() to handle new
        situation, when during CHECK/REPAIR TABLE we do not report 
        error about missing child immediately when this fact is 
        discovered during open_tables() but postpone error-reporting
        till the time when children are attached. 
        Also this callback is now responsible for pushing an error
        mentioning problematic child table to the list of errors to 
        be reported by CHECK/REPAIR TABLE statements.
        Finally, since now myrg_attach_children() no longer relies on
        return value from callback to determine the end of the children
        list, callback no longer needs to set my_errno value and can
        be simplified.
        
        Changed myrg_print_wrong_table() to always report a problem
        with child table as an error and not as a warning. This makes
        reporting for different types of issues with child tables
        more consistent and compatible with 5.0 behavior.
     @ storage/myisammrg/myrg_open.c
        Changed code in myrg_attach_children() not to abort on the
        first problem with a child table when attaching children to
        parent MERGE table during CHECK/REPAIR TABLE statement 
        execution. This allows CHECK/REPAIR TABLE to report problems 
        about absence/wrong engine for all underlying tables as
        part of their result-set.

    modified:
      mysql-test/r/merge.result
      mysql-test/t/merge.test
      sql/sql_base.cc
      storage/myisammrg/ha_myisammrg.cc
      storage/myisammrg/myrg_open.c
 3476 Sunanda Menon	2011-07-22
      Bug #12561297: Added the MySQL embedded binary

    modified:
      support-files/mysql.spec.sh
=== modified file 'mysql-test/r/merge.result'
--- a/mysql-test/r/merge.result	2010-11-30 17:53:11 +0000
+++ b/mysql-test/r/merge.result	2011-07-22 12:31:10 +0000
@@ -904,7 +904,8 @@ SELECT * FROM tm1;
 ERROR HY000: Unable to open underlying table which is differently defined or of non-MyISAM type or doesn't exist
 CHECK TABLE tm1;
 Table	Op	Msg_type	Msg_text
-test.tm1	check	Error	Table 'test.t1' doesn't exist
+test.tm1	check	Error	Table 'test.t1' is differently defined or of non-MyISAM type or doesn't exist
+test.tm1	check	Error	Table 'test.t2' is differently defined or of non-MyISAM type or doesn't exist
 test.tm1	check	Error	Unable to open underlying table which is differently defined or of non-MyISAM type or doesn't exist
 test.tm1	check	error	Corrupt
 CREATE TABLE t1(a INT);
@@ -912,7 +913,7 @@ SELECT * FROM tm1;
 ERROR HY000: Unable to open underlying table which is differently defined or of non-MyISAM type or doesn't exist
 CHECK TABLE tm1;
 Table	Op	Msg_type	Msg_text
-test.tm1	check	Error	Table 'test.t2' doesn't exist
+test.tm1	check	Error	Table 'test.t2' is differently defined or of non-MyISAM type or doesn't exist
 test.tm1	check	Error	Unable to open underlying table which is differently defined or of non-MyISAM type or doesn't exist
 test.tm1	check	error	Corrupt
 CREATE TABLE t2(a BLOB);
@@ -920,7 +921,7 @@ SELECT * FROM tm1;
 ERROR HY000: Unable to open underlying table which is differently defined or of non-MyISAM type or doesn't exist
 CHECK TABLE tm1;
 Table	Op	Msg_type	Msg_text
-test.tm1	check	Warning	Table 'test.t2' is differently defined or of non-MyISAM type or doesn't exist
+test.tm1	check	Error	Table 'test.t2' is differently defined or of non-MyISAM type or doesn't exist
 test.tm1	check	Error	Unable to open underlying table which is differently defined or of non-MyISAM type or doesn't exist
 test.tm1	check	error	Corrupt
 ALTER TABLE t2 MODIFY a INT;
@@ -3634,7 +3635,7 @@ test.t1	analyze	Error	Unable to open und
 test.t1	analyze	error	Corrupt
 CHECK TABLE t1;
 Table	Op	Msg_type	Msg_text
-test.t1	check	Error	Table 'test.t_not_exists' doesn't exist
+test.t1	check	Error	Table 'test.t_not_exists' is differently defined or of non-MyISAM type or doesn't exist
 test.t1	check	Error	Unable to open underlying table which is differently defined or of non-MyISAM type or doesn't exist
 test.t1	check	error	Corrupt
 CHECKSUM TABLE t1;
@@ -3650,7 +3651,7 @@ test.t1	optimize	Error	Unable to open un
 test.t1	optimize	error	Corrupt
 REPAIR TABLE t1;
 Table	Op	Msg_type	Msg_text
-test.t1	repair	Error	Table 'test.t_not_exists' doesn't exist
+test.t1	repair	Error	Table 'test.t_not_exists' is differently defined or of non-MyISAM type or doesn't exist
 test.t1	repair	Error	Unable to open underlying table which is differently defined or of non-MyISAM type or doesn't exist
 test.t1	repair	error	Corrupt
 REPAIR TABLE t1 USE_FRM;
@@ -3676,4 +3677,37 @@ ALTER TABLE t1 engine=myisam;
 ERROR HY000: Table 't1' was locked with a READ lock and can't be updated
 UNLOCK TABLES;
 DROP TABLE m1, t1;
-End of 6.0 tests
+#
+# Test for bug #11754210 - "45777: CHECK TABLE DOESN'T SHOW ALL
+#                           PROBLEMS FOR MERGE TABLE COMPLIANCE IN 5.1"
+#
+drop tables if exists t1, t2, t3, t4, m1;
+create table t1(id int) engine=myisam;
+create view t3 as select 1 as id;
+create table t4(id int) engine=memory;
+create table m1(id int) engine=merge union=(t1,t2,t3,t4);
+select * from m1;
+ERROR HY000: Unable to open underlying table which is differently defined or of non-MyISAM type or doesn't exist
+# The below CHECK and REPAIR TABLE statements should
+# report all problems with underlying tables: 
+# - absence of 't2',
+# - missing base table for 't3',
+# - wrong engine of 't4'.
+check table m1;
+Table	Op	Msg_type	Msg_text
+test.m1	check	Error	Table 'test.t2' is differently defined or of non-MyISAM type or doesn't exist
+test.m1	check	Error	Table 'test.t3' is differently defined or of non-MyISAM type or doesn't exist
+test.m1	check	Error	Table 'test.t4' is differently defined or of non-MyISAM type or doesn't exist
+test.m1	check	Error	Unable to open underlying table which is differently defined or of non-MyISAM type or doesn't exist
+test.m1	check	error	Corrupt
+repair table m1;
+Table	Op	Msg_type	Msg_text
+test.m1	repair	Error	Table 'test.t2' is differently defined or of non-MyISAM type or doesn't exist
+test.m1	repair	Error	Table 'test.t3' is differently defined or of non-MyISAM type or doesn't exist
+test.m1	repair	Error	Table 'test.t4' is differently defined or of non-MyISAM type or doesn't exist
+test.m1	repair	Error	Unable to open underlying table which is differently defined or of non-MyISAM type or doesn't exist
+test.m1	repair	error	Corrupt
+# Clean-up.
+drop tables m1, t1, t4;
+drop view t3;
+End of 5.5 tests

=== modified file 'mysql-test/t/merge.test'
--- a/mysql-test/t/merge.test	2010-10-06 07:56:29 +0000
+++ b/mysql-test/t/merge.test	2011-07-22 12:31:10 +0000
@@ -2798,7 +2798,32 @@ UNLOCK TABLES;
 DROP TABLE m1, t1;
 
 
---echo End of 6.0 tests
+--echo #
+--echo # Test for bug #11754210 - "45777: CHECK TABLE DOESN'T SHOW ALL
+--echo #                           PROBLEMS FOR MERGE TABLE COMPLIANCE IN 5.1"
+--echo #
+--disable_warnings
+drop tables if exists t1, t2, t3, t4, m1;
+--enable_warnings
+create table t1(id int) engine=myisam;
+create view t3 as select 1 as id;
+create table t4(id int) engine=memory;
+create table m1(id int) engine=merge union=(t1,t2,t3,t4);
+--error ER_WRONG_MRG_TABLE
+select * from m1;
+--echo # The below CHECK and REPAIR TABLE statements should
+--echo # report all problems with underlying tables: 
+--echo # - absence of 't2',
+--echo # - missing base table for 't3',
+--echo # - wrong engine of 't4'.
+check table m1;
+repair table m1;
+--echo # Clean-up.
+drop tables m1, t1, t4;
+drop view t3;
+
+
+--echo End of 5.5 tests
 
 --disable_result_log
 --disable_query_log

=== modified file 'sql/sql_base.cc'
--- a/sql/sql_base.cc	2011-07-03 23:48:19 +0000
+++ b/sql/sql_base.cc	2011-07-22 12:31:10 +0000
@@ -89,6 +89,69 @@ bool No_such_table_error_handler::safely
   return ((m_handled_errors > 0) && (m_unhandled_errors == 0));
 }
 
+
+/**
+  This internal handler is used to trap ER_NO_SUCH_TABLE and
+  ER_WRONG_MRG_TABLE errors during CHECK/REPAIR TABLE for MERGE
+  tables.
+*/
+
+class Repair_mrg_table_error_handler : public Internal_error_handler
+{
+public:
+  Repair_mrg_table_error_handler()
+    : m_handled_errors(false), m_unhandled_errors(false)
+  {}
+
+  bool handle_condition(THD *thd,
+                        uint sql_errno,
+                        const char* sqlstate,
+                        MYSQL_ERROR::enum_warning_level level,
+                        const char* msg,
+                        MYSQL_ERROR ** cond_hdl);
+
+  /**
+    Returns TRUE if there were ER_NO_SUCH_/WRONG_MRG_TABLE and there
+    were no unhandled errors. FALSE otherwise.
+  */
+  bool safely_trapped_errors()
+  {
+    /*
+      Check for m_handled_errors is here for extra safety.
+      It can be useful in situation when call to open_table()
+      fails because some error which was suppressed by another
+      error handler (e.g. in case of MDL deadlock which we
+      decided to solve by back-off and retry).
+    */
+    return (m_handled_errors && (! m_unhandled_errors));
+  }
+
+private:
+  bool m_handled_errors;
+  bool m_unhandled_errors;
+};
+
+
+bool
+Repair_mrg_table_error_handler::handle_condition(THD *,
+                                                 uint sql_errno,
+                                                 const char*,
+                                                 MYSQL_ERROR::enum_warning_level level,
+                                                 const char*,
+                                                 MYSQL_ERROR ** cond_hdl)
+{
+  *cond_hdl= NULL;
+  if (sql_errno == ER_NO_SUCH_TABLE || sql_errno == ER_WRONG_MRG_TABLE)
+  {
+    m_handled_errors= true;
+    return TRUE;
+  }
+
+  m_unhandled_errors= true;
+  return FALSE;
+}
+
+
 /**
   @defgroup Data_Dictionary Data Dictionary
   @{
@@ -4377,6 +4440,20 @@ open_and_process_table(THD *thd, LEX *le
     thd->pop_internal_handler();
     safe_to_ignore_table= no_such_table_handler.safely_trapped_errors();
   }
+  else if (tables->parent_l && (thd->open_options & HA_OPEN_FOR_REPAIR))
+  {
+    /*
+      Also fail silently for underlying tables of a MERGE table if this
+      table is opened for CHECK/REPAIR TABLE statement. This is needed
+      to provide complete list of problematic underlying tables in
+      CHECK/REPAIR TABLE output.
+    */
+    Repair_mrg_table_error_handler repair_mrg_table_handler;
+    thd->push_internal_handler(&repair_mrg_table_handler);
+    error= open_table(thd, tables, new_frm_mem, ot_ctx);
+    thd->pop_internal_handler();
+    safe_to_ignore_table= repair_mrg_table_handler.safely_trapped_errors();
+  }
   else
     error= open_table(thd, tables, new_frm_mem, ot_ctx);
 

=== modified file 'storage/myisammrg/ha_myisammrg.cc'
--- a/storage/myisammrg/ha_myisammrg.cc	2011-07-03 23:48:19 +0000
+++ b/storage/myisammrg/ha_myisammrg.cc	2011-07-22 12:31:10 +0000
@@ -159,9 +159,14 @@ extern "C" void myrg_print_wrong_table(c
   buf[db.length]= '.';
   memcpy(buf + db.length + 1, name.str, name.length);
   buf[db.length + name.length + 1]= 0;
-  push_warning_printf(current_thd, MYSQL_ERROR::WARN_LEVEL_WARN,
-                      ER_ADMIN_WRONG_MRG_TABLE, ER(ER_ADMIN_WRONG_MRG_TABLE),
-                      buf);
+  /*
+    Push an error to be reported as part of CHECK/REPAIR result-set.
+    Note that calling my_error() from handler is a hack which is kept
+    here to avoid refactoring. Normally engines should report errors
+    through return value which will be interpreted by caller using
+    handler::print_error() call.
+  */
+  my_error(ER_ADMIN_WRONG_MRG_TABLE, MYF(0), buf);
 }
 
 
@@ -593,8 +598,7 @@ public:
 
   @return       pointer to open MyISAM table structure
     @retval     !=NULL                  OK, returning pointer
-    @retval     NULL, my_errno == 0     Ok, no more child tables
-    @retval     NULL, my_errno != 0     error
+    @retval     NULL,                   Error.
 
   @detail
     This function retrieves the MyISAM table handle from the
@@ -614,17 +618,33 @@ extern "C" MI_INFO *myisammrg_attach_chi
   MI_INFO       *myisam= NULL;
   DBUG_ENTER("myisammrg_attach_children_callback");
 
-  if (!child_l)
-  {
-    DBUG_PRINT("myrg", ("No more children to attach"));
-    my_errno= 0; /* Ok, no more child tables. */
-    goto end;
-  }
+  /*
+    Number of children in the list and MYRG_INFO::tables_count,
+    which is used by caller of this function, should always match.
+  */
+  DBUG_ASSERT(child_l);
+
   child= child_l->table;
   /* Prepare for next child. */
   param->next();
 
   /*
+    When MERGE table is opened for CHECK or REPAIR TABLE statements,
+    failure to open any of underlying tables is ignored until this moment
+    (this is needed to provide complete list of the problematic underlying
+    tables in CHECK/REPAIR TABLE output).
+    Here we detect such a situation and report an appropriate error.
+  */
+  if (! child)
+  {
+    DBUG_PRINT("error", ("failed to open underlying table '%s'.'%s'",
+                         child_l->db, child_l->table_name));
+    /* This should only happen inside of CHECK/REPAIR TABLE. */
+    DBUG_ASSERT(current_thd->open_options & HA_OPEN_FOR_REPAIR);
+    goto end;
+  }
+
+  /*
     Do a quick compatibility check. The table def version is set when
     the table share is created. The child def version is copied
     from the table def version after a successful compatibility check.
@@ -653,7 +673,6 @@ extern "C" MI_INFO *myisammrg_attach_chi
   {
     DBUG_PRINT("error", ("temporary table mismatch parent: %d  child: %d",
                          parent->s->tmp_table, child->s->tmp_table));
-    my_errno= HA_ERR_WRONG_MRG_TABLE_DEF;
     goto end;
   }
 
@@ -664,12 +683,27 @@ extern "C" MI_INFO *myisammrg_attach_chi
     DBUG_PRINT("error", ("no MyISAM handle for child table: '%s'.'%s' 0x%lx",
                          child->s->db.str, child->s->table_name.str,
                          (long) child));
-    my_errno= HA_ERR_WRONG_MRG_TABLE_DEF;
   }
-  DBUG_PRINT("myrg", ("MyISAM handle: 0x%lx  my_errno: %d",
-                      my_errno ? 0L : (long) myisam, my_errno));
+
+  DBUG_PRINT("myrg", ("MyISAM handle: 0x%lx", (long) myisam));
 
  end:
+
+  if (!myisam &&
+      (current_thd->open_options & HA_OPEN_FOR_REPAIR))
+  {
+    char buf[2*NAME_LEN + 1 + 1];
+    strxnmov(buf, sizeof(buf) - 1, child_l->db, ".", child_l->table_name, NULL);
+    /*
+      Push an error to be reported as part of CHECK/REPAIR result-set.
+      Note that calling my_error() from handler is a hack which is kept
+      here to avoid refactoring. Normally engines should report errors
+      through return value which will be interpreted by caller using
+      handler::print_error() call.
+    */
+    my_error(ER_ADMIN_WRONG_MRG_TABLE, MYF(0), buf);
+  }
+
   DBUG_RETURN(myisam);
 }
 
@@ -783,12 +817,6 @@ int ha_myisammrg::attach_children(void)
   /* Must call this with children list in place. */
   DBUG_ASSERT(this->table->pos_in_table_list->next_global == this->children_l);
 
-  /*
-    'my_errno' is set by myisammrg_attach_children_callback() in
-    case of an error.
-  */
-  my_errno= 0;
-
   if (myrg_attach_children(this->file, this->test_if_locked |
                            current_thd->open_options,
                            myisammrg_attach_children_callback, &param,

=== modified file 'storage/myisammrg/myrg_open.c'
--- a/storage/myisammrg/myrg_open.c	2011-06-30 15:46:53 +0000
+++ b/storage/myisammrg/myrg_open.c	2011-07-22 12:31:10 +0000
@@ -385,6 +385,7 @@ int myrg_attach_children(MYRG_INFO *m_in
   uint       UNINIT_VAR(key_parts);
   uint       min_keys;
   my_bool    bad_children= FALSE;
+  my_bool    first_child= TRUE;
   DBUG_ENTER("myrg_attach_children");
   DBUG_PRINT("myrg", ("handle_locking: %d", handle_locking));
 
@@ -399,16 +400,26 @@ int myrg_attach_children(MYRG_INFO *m_in
   errpos= 0;
   file_offset= 0;
   min_keys= 0;
-  child_nr= 0;
-  while ((myisam= (*callback)(callback_param)))
+  for (child_nr= 0; child_nr < m_info->tables; child_nr++)
   {
+    if (! (myisam= (*callback)(callback_param)))
+    {
+      if (handle_locking & HA_OPEN_FOR_REPAIR)
+      {
+        /* An appropriate error should've been already pushed by callback. */
+        bad_children= TRUE;
+        continue;
+      }
+      goto bad_children;
+    }
+
     DBUG_PRINT("myrg", ("child_nr: %u  table: '%s'",
                         child_nr, myisam->filename));
-    DBUG_ASSERT(child_nr < m_info->tables);
 
     /* Special handling when the first child is attached. */
-    if (!child_nr)
+    if (first_child)
     {
+      first_child= FALSE;
       m_info->reclength= myisam->s->base.reclength;
       min_keys=  myisam->s->base.keys;
       key_parts= myisam->s->base.key_parts;
@@ -456,14 +467,11 @@ int myrg_attach_children(MYRG_INFO *m_in
     for (idx= 0; idx < key_parts; idx++)
       m_info->rec_per_key_part[idx]+= (myisam->s->state.rec_per_key_part[idx] /
                                        m_info->tables);
-    child_nr++;
   }
 
   if (bad_children)
     goto bad_children;
-  /* Note: callback() resets my_errno, so it is safe to check it here */
-  if (my_errno == HA_ERR_WRONG_MRG_TABLE_DEF)
-    goto err;
+
   if (sizeof(my_off_t) == 4 && file_offset > (ulonglong) (ulong) ~0L)
   {
     my_errno= HA_ERR_RECORD_FILE_FULL;

No bundle (reason: useless for push emails).
Thread
bzr push into mysql-5.5 branch (Dmitry.Lenev:3476 to 3477) Bug#11754210Dmitry Lenev22 Jul