List:Commits« Previous MessageNext Message »
From:Dmitry Lenev Date:May 25 2009 8:19pm
Subject:bzr commit into mysql-6.1-fk branch (dlenev:2740) WL#148
View as plain text  
#At file:///home/dlenev/src/bzr/mysql-6.1-mil14/ based on revid:dlenev@stripped

 2740 Dmitry Lenev	2009-05-26
      WL#148 "Foreign keys".
      
      Milestone 14 "DDL checks and changes: DROP, TRUNCATE, RENAME".
      
      After-review changes in progress. Extracted DROP TABLE-specific
      prelocking process into separate function. Also now DROP TABLE
      uses the same approach for checking if table's foreign keys
      allow impending operation as TRUNCATE and RENAME TABLE.

    modified:
      sql/fk_dd.cc
      sql/fk_dd.h
      sql/mysql_priv.h
      sql/sql_class.cc
      sql/sql_class.h
      sql/sql_delete.cc
      sql/sql_rename.cc
      sql/sql_table.cc
=== modified file 'sql/fk_dd.cc'
--- a/sql/fk_dd.cc	2009-05-15 12:46:44 +0000
+++ b/sql/fk_dd.cc	2009-05-25 20:19:16 +0000
@@ -2708,3 +2708,116 @@ bool fk_take_mdl_on_fk_names(THD *thd, T
 
   return FALSE;
 }
+
+
+/**
+  Check if table which is going to be affected by the statement has certain
+  property (related to its foreign keys) which makes it illegal for use in
+  this statement.
+
+  @param thd            Thread context.
+  @param table          TABLE_LIST element for the table to be affected.
+  @param error_silencer Object which specifies how errors during opening of
+                        table definition should be handled.
+  @param checker        Object which specifies what property should be
+                        checked and what error should be emitted if table
+                        has this property.
+
+  @retval FALSE  Tables does not have property specified by 'checker'.
+                 Statement can proceed.
+  @retval TRUE   It is either illegal to use this table in the statement
+                 (an appropriate error was emitted) or we have failed to
+                 open table definition.
+*/
+
+bool
+fk_check_if_illegal_statement(THD *thd, TABLE_LIST *table,
+                              Silence_open_table_def_error *error_silencer,
+                              Table_property_checker *checker)
+{
+  char key[MAX_DBKEY_LENGTH];
+  uint key_length;
+  TABLE_SHARE *share;
+  int error;
+  bool result= FALSE;
+
+
+  key_length= create_table_def_key(thd, key, table, 0);
+
+  thd->push_internal_handler(error_silencer);
+  pthread_mutex_lock(&LOCK_open);
+  share= get_table_share_with_create(thd, table, key, key_length, OPEN_VIEW,
+                                     &error);
+  pthread_mutex_unlock(&LOCK_open);
+  thd->pop_internal_handler();
+
+  if (share)
+  {
+    result= checker->check(share);
+
+    pthread_mutex_lock(&LOCK_open);
+    release_table_share(share);
+    pthread_mutex_unlock(&LOCK_open);
+    return result;
+  }
+  else
+  {
+    return error_silencer->has_unhandled_errors();
+  }
+}
+
+
+/**
+  Check if table which is going to be affected by the statement has
+  any foreign keys associated with it and if yes emit an error that
+  it is illegal to use such table in the statement.
+
+  @param share      TABLE_SHARE for the table to be checked.
+
+  @retval FALSE  There are no foreign keys associated. Statement can proceed.
+  @retval TRUE   There are foreign keys, error was emitted.
+*/
+
+bool Stmt_illegal_if_fks_checker::check(TABLE_SHARE *share)
+{
+  if (! share->fkeys.fkeys_child.is_empty() ||
+      ! share->fkeys.fkeys_parent.is_empty())
+  {
+    my_error(ER_FK_STATEMENT_ILLEGAL, MYF(0), m_statement, share->table_name.str);
+    return TRUE;
+  }
+  return FALSE;
+}
+
+
+/**
+  Check if table which is going to be affected by TRUNCATE TABLE
+  is a parent table in some non-self-referencing foreign key and
+  if yes emit an error that it is illegal to truncate such table.
+
+  @param share      TABLE_SHARE for the table to be checked.
+
+  @retval FALSE  This table is not parent in a non-self-referencing foreign
+                 key. Statement can proceed.
+  @retval TRUE   This table is parent in a non-self-referencing foreign key,
+                 error was emitted.
+*/
+
+bool Truncate_illegal_if_parent_checker::check(TABLE_SHARE *share)
+{
+  List_iterator<Foreign_key_parent_share> fk_it(share->fkeys.fkeys_parent);
+  Foreign_key_parent_share *fk_s;
+
+  while ((fk_s= fk_it++))
+  {
+    if (share->db.length != fk_s->child_table_db.length ||
+        strcmp(share->db.str, fk_s->child_table_db.str) ||
+        share->table_name.length != fk_s->child_table_name.length ||
+        strcmp(share->table_name.str, fk_s->child_table_name.str))
+    {
+      my_error(ER_FK_TRUNCATE_ILLEGAL, MYF(0));
+      return TRUE;
+    }
+  }
+  return FALSE;
+}

=== modified file 'sql/fk_dd.h'
--- a/sql/fk_dd.h	2009-05-15 12:46:44 +0000
+++ b/sql/fk_dd.h	2009-05-25 20:19:16 +0000
@@ -213,4 +213,58 @@ protected:
 };
 
 
+/**
+  Base class for helper classes specifying check to be carried out
+  and error to be emitted by fk_check_if_illegal_statement().
+*/
+
+class Table_property_checker
+{
+public:
+
+  virtual ~Table_property_checker() {}
+
+  virtual bool check(TABLE_SHARE *share) = 0;
+};
+
+
+/**
+  Helper class which allows to check if table participates in some
+  foreign keys and therefore it is illegal to use this table in
+  this statement.
+*/
+
+class Stmt_illegal_if_fks_checker : public Table_property_checker
+{
+public:
+
+  Stmt_illegal_if_fks_checker(const char *statement)
+    : m_statement(statement)
+  {}
+
+  virtual bool check(TABLE_SHARE *share);
+
+private:
+  const char *m_statement;
+};
+
+
+/**
+  Helper class which allows to check if table on which TRUNCATE TABLE
+  is going to be performed is parent table in some non-self-referencing
+  foreign key and therefore it is illegal to truncate it.
+*/
+
+class Truncate_illegal_if_parent_checker : public Table_property_checker
+{
+public:
+
+  virtual bool check(TABLE_SHARE *share);
+};
+
+bool
+fk_check_if_illegal_statement(THD *thd, TABLE_LIST *table,
+                              Silence_open_table_def_error *error_silencer,
+                              Table_property_checker *checker);
+
 #endif

=== modified file 'sql/mysql_priv.h'
--- a/sql/mysql_priv.h	2009-05-19 05:21:08 +0000
+++ b/sql/mysql_priv.h	2009-05-25 20:19:16 +0000
@@ -1079,12 +1079,6 @@ bool mysql_rm_db(THD *thd,char *db,bool 
 bool mysql_upgrade_db(THD *thd, LEX_STRING *old_db);
 void mysql_binlog_send(THD* thd, char* log_ident, my_off_t pos, ushort flags);
 void mysql_client_binlog_statement(THD *thd);
-bool fk_stmt_illegal_if_has_fks(TABLE_SHARE *share, const char *statement);
-bool fk_truncate_illegal_if_parent(TABLE_SHARE *share, const char *statement);
-bool
-fk_check_if_illegal_statement(THD *thd, TABLE_LIST *table,
-                              bool (*check_func)(TABLE_SHARE *, const char *),
-                              const char *statement);
 bool mysql_rm_table(THD *thd,TABLE_LIST *tables, my_bool if_exists,
                     my_bool drop_temporary);
 int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists,

=== modified file 'sql/sql_class.cc'
--- a/sql/sql_class.cc	2009-05-05 04:45:37 +0000
+++ b/sql/sql_class.cc	2009-05-25 20:19:16 +0000
@@ -466,6 +466,62 @@ bool Drop_table_error_handler::handle_co
 }
 
 
+/**
+  Do not silence any errors.
+*/
+
+bool
+Silence_no_error::handle_condition(THD *, uint, const char *,
+                                   MYSQL_ERROR::enum_warning_level,
+                                   const char *,
+                                   MYSQL_ERROR **condition)
+{
+  *condition= NULL;
+  m_has_unhandled_errors++;
+  return FALSE;
+}
+
+
+/**
+  Silence ER_NO_SUCH_TABLE errors, e.g. during opening of table definition.
+*/
+
+bool
+Silence_no_such_table_error::handle_condition(THD *,
+                                              uint sql_errno,
+                                              const char *,
+                                              MYSQL_ERROR::enum_warning_level,
+                                              const char *,
+                                              MYSQL_ERROR **condition)
+{
+  *condition= NULL;
+  if (sql_errno == ER_NO_SUCH_TABLE)
+    return TRUE;
+  m_has_unhandled_errors++;
+  return FALSE;
+}
+
+
+/**
+  Silence all non-fatal, non-transient errors which can occur, e.g. during
+  opening of table definition.
+*/
+
+bool Silence_non_fatal_non_transient_error::
+handle_condition(THD *thd, uint sql_errno, const char *,
+                 MYSQL_ERROR::enum_warning_level, const char *,
+                 MYSQL_ERROR **condition)
+{
+  *condition= NULL;
+  if (thd->is_fatal_error || sql_errno == ER_CANT_OPEN_FILE)
+  {
+    m_has_unhandled_errors++;
+    return FALSE;
+  }
+  return TRUE;
+}
+
+
 THD::THD()
    :Statement(&main_lex, &main_mem_root, CONVENTIONAL_EXECUTION,
               /* statement id */ 0),

=== modified file 'sql/sql_class.h'
--- a/sql/sql_class.h	2009-05-15 12:06:19 +0000
+++ b/sql/sql_class.h	2009-05-25 20:19:16 +0000
@@ -1572,6 +1572,76 @@ private:
 
 
 /**
+  Base class for internal error handler used for silencing errors
+  which can occur, e.g. during opening of table definition.
+*/
+
+class Silence_open_table_def_error: public Internal_error_handler
+{
+public:
+
+  Silence_open_table_def_error()
+    : m_has_unhandled_errors(0)
+  {}
+
+  bool has_unhandled_errors() { return m_has_unhandled_errors != 0; }
+
+protected:
+  uint m_has_unhandled_errors;
+};
+
+
+/**
+  Internal error handler which doesn't silence any errors.
+*/
+
+class Silence_no_error: public Silence_open_table_def_error
+{
+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);
+};
+
+
+/**
+  Internal error handler which allows to silence ER_NO_SUCH_TABLE
+  errors, e.g. during opening of table definition.
+*/
+
+class Silence_no_such_table_error: public Silence_open_table_def_error
+{
+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);
+};
+
+
+/**
+  Internal error handler which allows to silence all non-fatal, non-transient
+  errors which can occur, e.g. during opening of table definition.
+*/
+
+class Silence_non_fatal_non_transient_error: public Silence_open_table_def_error
+{
+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);
+};
+
+
+/**
   Tables that were locked with LOCK TABLES statement.
 
   Encapsulates a list of TABLE_LIST instances for tables

=== modified file 'sql/sql_delete.cc'
--- a/sql/sql_delete.cc	2009-05-19 05:21:08 +0000
+++ b/sql/sql_delete.cc	2009-05-25 20:19:16 +0000
@@ -26,6 +26,7 @@
 #include "sql_trigger.h"
 #include "transaction.h"
 #include "fk.h"
+#include "fk_dd.h"
 
 
 /**
@@ -153,11 +154,19 @@ bool mysql_delete(THD *thd, TABLE_LIST *
   }
 
   if (thd->lex->sql_command == SQLCOM_TRUNCATE &&
-      opt_fk_all_engines &&
-      fk_truncate_illegal_if_parent(table->s, "TRUNCATE"))
+      opt_fk_all_engines)
   {
-    MYSQL_DELETE_DONE(1, 0);
-    DBUG_RETURN(TRUE);
+    /*
+      Check that we are not truncating table which is
+      parent for some other table.
+    */
+    Truncate_illegal_if_parent_checker fk_checker;
+
+    if (fk_checker.check(table->s))
+    {
+      MYSQL_DELETE_DONE(1, 0);
+      DBUG_RETURN(TRUE);
+    }
   }
 
   thd_proc_info(thd, "init");
@@ -1207,13 +1216,21 @@ bool mysql_truncate(THD *thd, TABLE_LIST
       DBUG_RETURN(TRUE);
     }
 
-    if (opt_fk_all_engines &&
-        fk_check_if_illegal_statement(thd, table_list,
-                                      fk_truncate_illegal_if_parent,
-                                      "TRUNCATE"))
+    if (opt_fk_all_engines)
     {
-      error= 1;
-      goto end;
+      /*
+        Check that we are not going to truncate table which is parent
+        of some other table.
+      */
+      Truncate_illegal_if_parent_checker fk_checker;
+      Silence_no_error no_error_silencer;
+
+      if (fk_check_if_illegal_statement(thd, table_list, &no_error_silencer,
+                                        &fk_checker))
+      {
+        error= 1;
+        goto end;
+      }
     }
 
     pthread_mutex_lock(&LOCK_open);

=== modified file 'sql/sql_rename.cc'
--- a/sql/sql_rename.cc	2009-05-19 05:21:08 +0000
+++ b/sql/sql_rename.cc	2009-05-25 20:19:16 +0000
@@ -19,6 +19,7 @@
 
 #include "mysql_priv.h"
 #include "sql_trigger.h"
+#include "fk_dd.h"
 
 
 static TABLE_LIST *rename_tables(THD *thd, TABLE_LIST *table_list,
@@ -146,12 +147,23 @@ bool mysql_rename_tables(THD *thd, TABLE
 
   if (opt_fk_all_engines)
   {
+    /*
+      We have to silence ER_NO_SUCH_TABLE error to allow execution of
+      statements like "RENAME TABLES t1 TO t3, t2 TO t1, t3 TO t1"
+      (i.e. statements in which some table being renamed is not yet
+      exists at the start). It does not make sense to ignore other
+      errors as in this case further execution of the RENAME statement
+      is likely to fail anyway.
+    */
+    Stmt_illegal_if_fks_checker fk_checker("RENAME TABLE");
+    Silence_no_such_table_error no_such_table_error_silencer;
+
     for (ren_table= table_list; ren_table;
          ren_table= ren_table->next_local->next_local)
     {
       if (fk_check_if_illegal_statement(thd, ren_table,
-                                        fk_stmt_illegal_if_has_fks,
-                                        "RENAME TABLE"))
+                                        &no_such_table_error_silencer,
+                                        &fk_checker))
       {
         error= 1;
         goto err_unlock;

=== modified file 'sql/sql_table.cc'
--- a/sql/sql_table.cc	2009-05-23 06:58:20 +0000
+++ b/sql/sql_table.cc	2009-05-25 20:19:16 +0000
@@ -1533,172 +1533,6 @@ bool mysql_rm_table(THD *thd,TABLE_LIST 
 
 
 /**
-  Check if table which is going to be affected by the statement has
-  any foreign keys associated with it and if yes emit an error that
-  it is illegal to use such table in the statement.
-
-  @param share      TABLE_SHARE for the table to be affected.
-  @param statement  Name of the statement to be used in the error message.
-
-  @retval FALSE  There are no foreign keys associated. Statement can proceed.
-  @retval TRUE   There are foreign keys, error was emitted.
-*/
-
-bool fk_stmt_illegal_if_has_fks(TABLE_SHARE *share, const char *statement)
-{
-  if (! share->fkeys.fkeys_child.is_empty() ||
-      ! share->fkeys.fkeys_parent.is_empty())
-  {
-    my_error(ER_FK_STATEMENT_ILLEGAL, MYF(0), statement, share->table_name.str);
-    return TRUE;
-  }
-  return FALSE;
-}
-
-
-/**
-  Check if table which is going to be affected by the statement is a
-  parent table in some non-self-referencing foreign key and if yes
-  emit an error that it is illegal to use such table in TRUNCATE TABLE.
-
-  @param share      TABLE_SHARE for the table to be affected.
-  @param statement  Name of the statement to be used in the error message.
-
-  @retval FALSE  This table is not parent in a non-self-referencing foreign
-                 key. Statement can proceed.
-  @retval TRUE   There are foreign keys, error was emitted.
-*/
-
-bool fk_truncate_illegal_if_parent(TABLE_SHARE *share, const char *statement)
-{
-  List_iterator<Foreign_key_parent_share> fk_it(share->fkeys.fkeys_parent);
-  Foreign_key_parent_share *fk_s;
-
-  while ((fk_s= fk_it++))
-  {
-    if (share->db.length != fk_s->child_table_db.length ||
-        strcmp(share->db.str, fk_s->child_table_db.str) ||
-        share->table_name.length != fk_s->child_table_name.length ||
-        strcmp(share->table_name.str, fk_s->child_table_name.str))
-    {
-      my_error(ER_FK_TRUNCATE_ILLEGAL, MYF(0));
-      return TRUE;
-    }
-  }
-  return FALSE;
-}
-
-
-/**
-  Silence ER_NO_SUCH_TABLE error during opening of table definition.
-*/
-
-class Silence_no_such_table_error : public Internal_error_handler
-{
-public:
-   Silence_no_such_table_error()
-    : m_unhandled_errors(0)
-  {}
-
-  virtual ~Silence_no_such_table_error() {}
-
-  virtual bool handle_condition(THD *thd,
-                                uint sql_errno,
-                                const char *sqlstate,
-                                MYSQL_ERROR::enum_warning_level level,
-                                const char *msg,
-                                MYSQL_ERROR **condition);
-
-  bool not_trapped_errors() { return m_unhandled_errors != 0; }
-
-private:
-  int m_unhandled_errors;
-};
-
-
-bool
-Silence_no_such_table_error::handle_condition(THD *,
-                                              uint sql_errno,
-                                              const char *,
-                                              MYSQL_ERROR::enum_warning_level,
-                                              const char *,
-                                              MYSQL_ERROR **condition)
-{
-  *condition= NULL;
-
-  if (sql_errno == ER_NO_SUCH_TABLE)
-  {
-    return TRUE;
-  }
-
-  m_unhandled_errors++;
-  return FALSE;
-}
-
-
-/**
-  Check if table which is going to be affected by the statement has certain
-  property (related to its foreign keys) and if yes emit an appropriate error
-  that it is illegal to use such table in the statement.
-
-  @param thd        Thread context.
-  @param table      TABLE_LIST element for the table to be affected.
-  @param check_func Function which checks if table has certain property
-                    which makes it illegal to use within particular
-                    statement and emits appropriate error message.
-  @param statement  Name of the statement to be used in the error message.
-
-  @retval FALSE  There are no foreign keys associated. Statement can proceed.
-  @retval TRUE   There are foreign keys, error was emitted.
-*/
-
-bool
-fk_check_if_illegal_statement(THD *thd, TABLE_LIST *table,
-                              bool (*check_func)(TABLE_SHARE *, const char *),
-                              const char *statement)
-{
-  char key[MAX_DBKEY_LENGTH];
-  uint key_length;
-  Silence_no_such_table_error no_such_table_error_silencer;
-  TABLE_SHARE *share;
-  int error;
-  bool result= FALSE;
-
-
-  key_length= create_table_def_key(thd, key, table, 0);
-
-  /*
-    We have to silence ER_NO_SUCH_TABLE error to allow execution of statements
-    like "RENAME TABLES t1 TO t3, t2 TO t1, t3 TO t1" (i.e. statements in which
-    some table being renamed is not yet exists at the start).
-    It does not make sense to ignore other errors as in this case further
-    execution of statement calling fk_illegal_statement_if_has_fks() is
-    likely to fail anyway.
-  */
-  thd->push_internal_handler(& no_such_table_error_silencer);
-  pthread_mutex_lock(&LOCK_open);
-  share= get_table_share_with_create(thd, table, key, key_length, OPEN_VIEW,
-                                     &error);
-  pthread_mutex_unlock(&LOCK_open);
-  thd->pop_internal_handler();
-
-  if (share)
-  {
-    result= (*check_func)(share, statement);
-
-    pthread_mutex_lock(&LOCK_open);
-    release_table_share(share);
-    pthread_mutex_unlock(&LOCK_open);
-    return result;
-  }
-  else
-  {
-    return no_such_table_error_silencer.not_trapped_errors();
-  }
-}
-
-
-/**
   Create TABLE_LIST object representing a parent table for the child
   table being dropped and add it to the table list.
 
@@ -1714,10 +1548,10 @@ fk_check_if_illegal_statement(THD *thd, 
   @retval TRUE   Failure (OOM).
 */
 
-static bool fk_drop_table_add_parent_table(THD *thd,
-                                           TABLE_LIST **local_table_list,
-                                           const LEX_STRING &db,
-                                           const LEX_STRING &table_name)
+static bool drop_table_add_parent_table(THD *thd,
+                                        TABLE_LIST **local_table_list,
+                                        const LEX_STRING &db,
+                                        const LEX_STRING &table_name)
 {
   TABLE_LIST *table;
 
@@ -1848,51 +1682,6 @@ private:
 
 
 /**
-  Silence all non-fatal, non-transient errors which can occur during
-  opening of table definition.
-*/
-
-class Silence_non_fatal_non_transient_error : public Internal_error_handler
-{
-public:
-   Silence_non_fatal_non_transient_error()
-    : m_unhandled_errors(0)
-  {}
-
-  virtual ~Silence_non_fatal_non_transient_error() {}
-
-  virtual bool handle_condition(THD *thd,
-                                uint sql_errno,
-                                const char *sqlstate,
-                                MYSQL_ERROR::enum_warning_level level,
-                                const char *msg,
-                                MYSQL_ERROR **condition);
-
-  bool not_trapped_errors() { return m_unhandled_errors != 0; }
-
-private:
-  int m_unhandled_errors;
-};
-
-
-bool Silence_non_fatal_non_transient_error::
-handle_condition(THD *thd, uint sql_errno, const char *,
-                 MYSQL_ERROR::enum_warning_level, const char *,
-                 MYSQL_ERROR **condition)
-{
-  *condition= NULL;
-
-  if (thd->is_fatal_error || sql_errno == ER_CANT_OPEN_FILE)
-  {
-    m_unhandled_errors++;
-    return FALSE;
-  }
-
-  return TRUE;
-}
-
-
-/**
   Prepare context for drop table operation by preparing Parent_info
   objects describing related tables from which foreign keys involving
   table being dropped were removed.
@@ -1926,7 +1715,7 @@ Foreign_key_ddl_rcontext::prepare_drop_t
     Don't drop foreign keys from .FRMs of temporary tables which are
     left from aborted ALTER TABLE.
 
-    See comment in fk_check_tables_before_drop() function.
+    See comment in drop_table_check_locks() function.
   */
   if (drop_table->internal_tmp_table)
     return FALSE;
@@ -1944,7 +1733,7 @@ Foreign_key_ddl_rcontext::prepare_drop_t
     in order to avoid errors when attempting to drop non-existing table
     or table with corrupted .FRMs.
 
-    @sa Comment in fk_check_tables_before_drop().
+    @sa Comment in drop_table_check_locks().
   */
   m_thd->push_internal_handler(&non_fatal_error_silencer);
   pthread_mutex_lock(&LOCK_open);
@@ -1954,7 +1743,7 @@ Foreign_key_ddl_rcontext::prepare_drop_t
 
   if (!share)
   {
-    return non_fatal_error_silencer.not_trapped_errors();
+    return non_fatal_error_silencer.has_unhandled_errors();
   }
 
   List_iterator<Foreign_key_child_share> fkeys_it(share->fkeys.fkeys_child);
@@ -2117,7 +1906,7 @@ Foreign_key_ddl_rcontext::prepare_drop_t
         This also covers cases when table needs repair or its definition
         in the engine has changed.
       */
-      if (no_such_table_error_silencer.not_trapped_errors())
+      if (no_such_table_error_silencer.has_unhandled_errors())
         return TRUE;
       /* In case of ER_NO_SUCH_TABLE simply remove table from the list. */
       tab_it.remove();
@@ -2200,10 +1989,6 @@ void Foreign_key_ddl_rcontext::cleanup()
   @param parents   Additional tables to lock.
   @param fk_names  List of foreign key names to lock.
 
-  @note Unlike for tables from the first list for tables from
-        'parents' list this functions assumes that meta-data lock
-        requests objects are allocated by caller.
-
   @retval FALSE  Success.
   @retval TRUE   Failure.
 */
@@ -2364,11 +2149,9 @@ err_tables:
 
 
 /**
-  Check if among tables being dropped there are tables which participate
-  as a parent in foreign key and for which corresponding child table is
-  not dropped. Also find out if we have all necessary metadata locks for
-  this operation and create list of tables and foreign key names to be
-  additionally locked if not.
+  Find out if we have all necessary metadata locks for dropping tables
+  and create list of tables and foreign key names to be additionally
+  locked if not.
 
   @param[in]  thd                    Thread context.
   @param[in]  tables                 List of tables to be dropped.
@@ -2389,13 +2172,12 @@ err_tables:
 */
 
 static bool
-fk_check_tables_before_drop(THD *thd,
-                            TABLE_LIST *tables,
-                            TABLE_LIST **parents_to_lock,
-                            List<Foreign_key_name> *fk_names_to_lock,
-                            bool *has_not_locked_parent_or_fk_name)
+drop_table_check_locks(THD *thd, TABLE_LIST *tables,
+                       TABLE_LIST **parents_to_lock,
+                       List<Foreign_key_name> *fk_names_to_lock,
+                       bool *has_not_locked_parent_or_fk_name)
 {
-  TABLE_LIST *table, *tab;
+  TABLE_LIST *table;
   TABLE_SHARE *share;
   char key[MAX_DBKEY_LENGTH];
   uint key_length;
@@ -2449,7 +2231,7 @@ fk_check_tables_before_drop(THD *thd,
     pthread_mutex_unlock(&LOCK_open);
     thd->pop_internal_handler();
 
-    if (!share && non_fatal_error_silencer.not_trapped_errors())
+    if (!share && non_fatal_error_silencer.has_unhandled_errors())
       return TRUE;
 
     if (share)
@@ -2458,26 +2240,8 @@ fk_check_tables_before_drop(THD *thd,
                                                          fkeys.fkeys_parent);
       Foreign_key_parent_share *fk_p_s;
 
-      /*
-        For all foreign keys in which table to be dropped serves as parent
-        check that we also going to drop child table.
-      */
       while ((fk_p_s= fkeys_p_it++))
       {
-        if (! (tab= find_table_in_local_list(tables, fk_p_s->child_table_db.str,
-                                             fk_p_s->child_table_name.str)) ||
-            tab->drop_table_rctx->is_temporary)
-        {
-          my_error(ER_FK_CHILD_TABLE_EXISTS, MYF(0), fk_p_s->name.str,
-                   thd->lex->sql_command == SQLCOM_DROP_DB ? "DROP DATABASE" :
-                                                             "DROP TABLE",
-                   table->table_name);
-          pthread_mutex_lock(&LOCK_open);
-          release_table_share(share);
-          pthread_mutex_unlock(&LOCK_open);
-          return TRUE;
-        }
-
         /*
           Adding name of foreign key to the list of names to be locked
           when processing Foreign_key_parent_share is needed in order
@@ -2523,9 +2287,9 @@ fk_check_tables_before_drop(THD *thd,
                                   table->db, fk_c_s->name.str)))
           *has_not_locked_parent_or_fk_name= TRUE;
 
-        if (fk_drop_table_add_parent_table(thd, parents_to_lock,
-                                           fk_c_s->parent_table_db,
-                                           fk_c_s->parent_table_name))
+        if (drop_table_add_parent_table(thd, parents_to_lock,
+                                        fk_c_s->parent_table_db,
+                                        fk_c_s->parent_table_name))
         {
           pthread_mutex_lock(&LOCK_open);
           release_table_share(share);
@@ -2559,12 +2323,10 @@ fk_check_tables_before_drop(THD *thd,
 
 
 /**
-  Check if among tables being dropped there are tables which participate
-  as a parent in foreign key and for which corresponding child table is
-  not dropped. Also find out if we have all necessary table-level locks
-  on parent tables for this operation. Create list of parent tables which
-  are not among tables to be dropped and list of foreign key names with
-  appropriate metadata locks.
+  Find out if we have all necessary table-level locks on parent tables for
+  tables to be dropped. Create list of parent tables which are not among
+  tables to be dropped and list of foreign key names with appropriate
+  metadata locks.
 
   @param[in]  thd              Thread context.
   @param[in]  tables           List of tables to be dropped.
@@ -2579,9 +2341,9 @@ fk_check_tables_before_drop(THD *thd,
 */
 
 static bool
-fk_check_tables_before_drop(THD *thd,
-                            TABLE_LIST *tables, TABLE_LIST **parents_locked,
-                            List<Foreign_key_name> *fk_names_locked)
+drop_table_check_locks(THD *thd, TABLE_LIST *tables,
+                       TABLE_LIST **parents_locked,
+                       List<Foreign_key_name> *fk_names_locked)
 {
   TABLE_LIST *table, *tab;
   TABLE_SHARE *share;
@@ -2595,24 +2357,6 @@ fk_check_tables_before_drop(THD *thd,
 
     share= table->table->s;
 
-    List_iterator<Foreign_key_parent_share> fkeys_p_it(share->
-                                                       fkeys.fkeys_parent);
-    Foreign_key_parent_share *fk_p_s;
-
-    while ((fk_p_s= fkeys_p_it++))
-    {
-      if (! (tab= find_table_in_local_list(tables, fk_p_s->child_table_db.str,
-                                           fk_p_s->child_table_name.str)) ||
-          tab->drop_table_rctx->is_temporary)
-      {
-        my_error(ER_FK_CHILD_TABLE_EXISTS, MYF(0), fk_p_s->name.str,
-                 thd->lex->sql_command == SQLCOM_DROP_DB ? "DROP DATABASE" :
-                                                           "DROP TABLE",
-                 table->table_name);
-        return TRUE;
-      }
-    }
-
     List_iterator<Foreign_key_child_share> fkeys_c_it(share->fkeys.fkeys_child);
     Foreign_key_child_share *fk_c_s;
 
@@ -2626,9 +2370,9 @@ fk_check_tables_before_drop(THD *thd,
                                      fk_c_s->parent_table_db.str,
                                      fk_c_s->parent_table_name.str))
       {
-        if (fk_drop_table_add_parent_table(thd, parents_locked,
-                                           fk_c_s->parent_table_db,
-                                           fk_c_s->parent_table_name))
+        if (drop_table_add_parent_table(thd, parents_locked,
+                                        fk_c_s->parent_table_db,
+                                        fk_c_s->parent_table_name))
           return TRUE;
 
         if (! ((*parents_locked)->table=
@@ -2660,6 +2404,117 @@ fk_check_tables_before_drop(THD *thd,
 }
 
 
+/**
+  Get all necessary metadata locks for dropping tables (including
+  modification of .FRMs of parent tables), create list of tables
+  which were locked in addition to tables being dropped and similar
+  list of foreign key names.
+
+  @param[in]  thd              Thread context.
+  @param[in]  tables           List of tables to be dropped.
+  @param[out] parents_locked   List of parent tables which were additionally
+                               locked.
+  @param[out] fk_names_locked  List of foreign key names which were locked.
+
+  @retval FALSE  Success.
+  @retval TRUE   Failure (e.g OOM, connection/statement was killed).
+*/
+
+static bool drop_table_lock_tables(THD *thd, TABLE_LIST *tables,
+                                   TABLE_LIST **parents_locked,
+                                   List<Foreign_key_name> *fk_names_locked)
+{
+  TABLE_LIST *parents_to_lock= NULL;
+  List<Foreign_key_name> fk_names_to_lock;
+  bool has_not_locked_parent_or_fk_name;
+
+  while (1)
+  {
+    if (lock_dropped_and_parent_tables(thd, tables, parents_to_lock,
+                                       fk_names_to_lock))
+      return TRUE;
+    *parents_locked= parents_to_lock;
+    *fk_names_locked= fk_names_to_lock;
+
+    if (opt_fk_all_engines)
+    {
+      if (drop_table_check_locks(thd, tables,
+                                 &parents_to_lock, &fk_names_to_lock,
+                                 &has_not_locked_parent_or_fk_name))
+        return TRUE;
+
+      if (has_not_locked_parent_or_fk_name)
+      {
+        thd->mdl_context.release_all_locks();
+        thd->mdl_context.remove_all_requests();
+        /* Retry acquiring all necessary metadata locks. */
+        continue;
+      }
+    }
+    break;
+  }
+  return FALSE;
+}
+
+
+/**
+  Helper class which allows to perform check that DROP TABLE/DATABASE
+  statement doesn't try to drop table which is parent for some other
+  table not to be dropped.
+*/
+
+class Drop_table_illegal_if_parent_checker : public Table_property_checker
+{
+public:
+
+  Drop_table_illegal_if_parent_checker(const char *statement,
+                                       TABLE_LIST *tables)
+    : m_statement(statement), m_dropped_tables(tables)
+  {}
+
+  virtual bool check(TABLE_SHARE *share);
+
+private:
+  const char *m_statement;
+  TABLE_LIST *m_dropped_tables;
+};
+
+
+/**
+  Check if table which is going to be dropped is a parent table for
+  some table which is not dropped. If yes emit an error that it is
+  illegal to do that in DROP TABLE/DATABASE statement.
+
+  @param share      TABLE_SHARE for the table to be checked.
+
+  @retval FALSE  This table is not parent for some table which is not
+                 to be dropped. Statement can proceed.
+  @retval TRUE   This table is parent table for some table which is
+                 not to be dropped. Error was emitted.
+*/
+
+bool Drop_table_illegal_if_parent_checker::check(TABLE_SHARE *share)
+{
+  List_iterator<Foreign_key_parent_share> fk_it(share->fkeys.fkeys_parent);
+  Foreign_key_parent_share *fk_s;
+  TABLE_LIST *tab;
+
+  while ((fk_s= fk_it++))
+  {
+    if (! (tab= find_table_in_local_list(m_dropped_tables,
+                                         fk_s->child_table_db.str,
+                                         fk_s->child_table_name.str)) ||
+        tab->drop_table_rctx->is_temporary)
+    {
+      my_error(ER_FK_CHILD_TABLE_EXISTS, MYF(0), fk_s->name.str, m_statement,
+               share->table_name.str);
+      return TRUE;
+    }
+  }
+  return FALSE;
+}
+
+
 /*
   Execute the drop of a normal or temporary table
 
@@ -2702,9 +2557,8 @@ int mysql_rm_table_part2(THD *thd, TABLE
   int non_temp_tables_count= 0;
   bool some_tables_deleted=0, tmp_table_deleted=0, foreign_key_error=0;
   String built_query;
-  bool has_not_locked_parent_or_fk_name;
-  TABLE_LIST *parents_to_lock= NULL, *parents_locked= NULL;
-  List<Foreign_key_name> fk_names_to_lock, fk_names_locked;
+  TABLE_LIST *parents_locked= NULL;
+  List<Foreign_key_name> fk_names_locked;
   List_iterator<Foreign_key_name> fk_names_it(fk_names_locked);
   Foreign_key_name *fk_name;
   DBUG_ENTER("mysql_rm_table_part2");
@@ -2779,25 +2633,36 @@ int mysql_rm_table_part2(THD *thd, TABLE
 
     if (!thd->locked_tables_mode)
     {
-retry_lock:
-      if (lock_dropped_and_parent_tables(thd, tables,
-                                         parents_to_lock, fk_names_to_lock))
+      if (drop_table_lock_tables(thd, tables, &parents_locked,
+                                 &fk_names_locked))
         DBUG_RETURN(1);
-      parents_locked= parents_to_lock;
-      fk_names_locked= fk_names_to_lock;
 
       if (opt_fk_all_engines)
       {
-        if (fk_check_tables_before_drop(thd, tables,
-                                        &parents_to_lock, &fk_names_to_lock,
-                                        &has_not_locked_parent_or_fk_name))
-          DBUG_RETURN(1);
+        /*
+          For all foreign keys in which table to be dropped serves as parent
+          check that we also going to drop child table.
+
+          Ignore ER_NO_SUCH_TABLE and other non-fatal, non-transient errors
+          in order to be able to drop non-existing or corrupted tables.
+        */
+        Drop_table_illegal_if_parent_checker checker(((thd->lex->sql_command ==
+                                                       SQLCOM_DROP_DB) ?
+                                                      "DROP DATABASE" :
+                                                      "DROP TABLE"),
+                                                     tables);
+        Silence_non_fatal_non_transient_error non_fatal_error_silencer;
 
-        if (has_not_locked_parent_or_fk_name)
+        for (table= tables; table; table= table->next_local)
         {
-          thd->mdl_context.release_all_locks();
-          thd->mdl_context.remove_all_requests();
-          goto retry_lock;
+          if (! table->drop_table_rctx->is_temporary &&
+              ! table->internal_tmp_table)
+          {
+            if (fk_check_if_illegal_statement(thd, table,
+                                              &non_fatal_error_silencer,
+                                              &checker))
+              DBUG_RETURN(1);
+          }
         }
       }
     }
@@ -2820,16 +2685,28 @@ retry_lock:
         }
       }
 
+      if (opt_fk_all_engines)
+      {
+        /*
+          Check that we are not dropping parent tables without dropping
+          child tables.
+        */
+        Drop_table_illegal_if_parent_checker fk_checker("DROP TABLE", tables);
+
+        for (table= tables; table; table= table->next_local)
+          if (fk_checker.check(table->table->s))
+            DBUG_RETURN(1);
+      }
+
       /*
-        Check that we are not dropping parent tables withour dropping child
-        tables and that for all parent tables for child tables being dropped
+        Check that that for all parent tables for child tables being dropped
         we have a write lock so we can safely upgrade meta-data lock before
-        changing their .FRM. Finally, create list of parent tables which are
-        not among tables to be dropped to use for this upgrade.
+        changing their .FRM. Also create list of parent tables which are not
+        among tables to be dropped to use for this upgrade.
       */
       if (opt_fk_all_engines &&
-          fk_check_tables_before_drop(thd, tables, &parents_locked,
-                                      &fk_names_locked))
+          drop_table_check_locks(thd, tables, &parents_locked,
+                                 &fk_names_locked))
         DBUG_RETURN(1);
 
       /*


Attachment: [text/bzr-bundle] bzr/dlenev@mysql.com-20090525201916-9rm2b8e6axf1233b.bundle
Thread
bzr commit into mysql-6.1-fk branch (dlenev:2740) WL#148Dmitry Lenev25 May