List:Commits« Previous MessageNext Message »
From:Dmitry Lenev Date:July 20 2009 12:52pm
Subject:bzr commit into mysql-6.1-fk branch (dlenev:2742) WL#148
View as plain text  
#At file:///home/dlenev/src/bzr/mysql-6.1-mil13/ based on revid:dlenev@stripped

 2742 Dmitry Lenev	2009-07-20
      WL#148 "Foreign keys".
      
      Milestone 13 "DDL checks and changes: ALTER, CREATE INDEX, DROP INDEX words".
      
      Work in progress. Avoid creation of extra TABLE instance for performing
      integrity checks when handling ALTER TABLE ADD CONSTRAINT when it is not
      necessary.

    modified:
      sql/fk.cc
      sql/fk.h
      sql/sql_table.cc
=== modified file 'sql/fk.cc'
--- a/sql/fk.cc	2009-07-17 06:30:13 +0000
+++ b/sql/fk.cc	2009-07-20 12:52:17 +0000
@@ -526,22 +526,23 @@ Fk_constraint_list::prepare_check_parent
   For ALTER TABLE operation on the table, prepare runtime contexts for foreign
   keys which are added by this statement and which may require checking.
 
-  @param thd         Thread context.
-  @param table       TABLE object representing new version of table being
-                     altered.
-  @param table_2     Second instance of TABLE object for new version of table
-                     being altered (required for checking self-referencing
-                     foreign keys).
-  @param fk_list     List of foreign keys in new version of table, with new
-                     foreign keys marked.
+  @param thd             Thread context.
+  @param table           TABLE object representing new version of table being
+                         altered.
+  @param table_tmp_name  Temporary name for new version of table being altered
+                         (used for creation of additional TABLE object for
+                          table being altered if some self-referencing foreign
+                          keys need to be checked).
+  @param fk_list         List of foreign keys in new version of table, with new
+                         foreign keys marked.
 
   @retval FALSE  Success.
   @retval TRUE   Error, out of resources.
 */
 
 bool
-Fk_constraint_list::prepare_alter_check_parent(THD *thd,
-                                               TABLE *table, TABLE *table_2,
+Fk_constraint_list::prepare_alter_check_parent(THD *thd, TABLE *table,
+                                               const char *table_tmp_name,
                                                List<Foreign_key_child> &fk_list)
 {
   List_iterator<Foreign_key_child_share> fk_s_it(table->s->fkeys.fkeys_child);
@@ -580,7 +581,40 @@ Fk_constraint_list::prepare_alter_check_
         new (thd->mem_root) Foreign_key_child_rcontext(thd, table, fk_s, this);
 
       if (fk_s->is_self_reference)
-        parent_table= table_2;
+      {
+        /*
+          To perform checks for self-referencing foreign key constraints on
+          table being altered we have to create an additional TABLE object
+          for its new version. To do this we approach similar to one used
+          in open_temporary_table() with the exception that here we already
+          have properly loaded TABLE_SHARE object for table.
+
+          TODO: At some point in future we will probably use a cursor created
+                through cursor API instead of full-blown TABLE instance.
+        */
+        if (! m_extra_table)
+        {
+          if (! (m_extra_table= (TABLE*) my_malloc(sizeof(*m_extra_table),
+                                                   MYF(MY_WME))))
+            return TRUE;
+
+          if (open_table_from_share(thd, table->s, table_tmp_name,
+                                    (uint) (HA_OPEN_KEYFILE | HA_OPEN_RNDFILE |
+                                            HA_GET_INDEX),
+                                    (READ_KEYINFO | COMPUTE_TYPES |
+                                     EXTRA_RECORD),
+                                    ha_open_options, m_extra_table, OTM_OPEN))
+          {
+            my_free((char*) m_extra_table, MYF(0));
+            m_extra_table= 0;
+            return TRUE;
+          }
+
+          m_extra_table->reginfo.lock_type= TL_READ_NO_INSERT;
+          m_extra_table->pos_in_table_list= 0;
+        }
+        parent_table= m_extra_table;
+      }
       else
         parent_table= fk_def->parent_table->table;
 
@@ -597,6 +631,60 @@ Fk_constraint_list::prepare_alter_check_
 
 
 /**
+   Set external lock for extra TABLE instance which was created for performing
+   checks for self-referencing foreign keys during ALTER TABLE.
+
+   @param thd  Thread context.
+
+   @note Some engines use handler::ha_external_lock() called by this method as
+         a mark of statement boundary and to set up some structures needed for
+         statement execution.
+
+   @retval FALSE  Success.
+   @retval TRUE   Error.
+*/
+
+bool Fk_constraint_list::lock_extra_table(THD *thd)
+{
+  int error;
+
+  if (m_extra_table &&
+      (error= m_extra_table->file->ha_external_lock(thd, F_RDLCK)))
+  {
+    m_extra_table->file->print_error(error, MYF(0));
+    return TRUE;
+  }
+  return FALSE;
+}
+
+
+/**
+   "Unlock" extra TABLE instance which was created for performing checks for
+   self-referencing foreign keys during ALTER TABLE.
+
+   @param thd  Thread context.
+
+   @sa Fk_constraint_list::lock_extra_table()
+
+   @retval FALSE  Success.
+   @retval TRUE   Error.
+*/
+
+bool Fk_constraint_list::unlock_extra_table(THD *thd)
+{
+  int error;
+
+  if (m_extra_table &&
+      (error= m_extra_table->file->ha_external_lock(thd, F_UNLCK)))
+  {
+    m_extra_table->file->print_error(error, MYF(0));
+    return TRUE;
+  }
+  return FALSE;
+}
+
+
+/**
   For UPDATE or DELETE operation on the table prepare runtime contexts
   for all foreign keys for which this table is a parent and which may
   require checks or cascading actions.
@@ -687,6 +775,12 @@ Fk_constraint_list::~Fk_constraint_list(
 
   while ((parent_ctx= cascade_it++))
     delete parent_ctx;
+
+  if (m_extra_table)
+  {
+    close_temporary(m_extra_table, FALSE, FALSE);
+    my_free((char*) m_extra_table, MYF(0));
+  }
 }
 
 

=== modified file 'sql/fk.h'
--- a/sql/fk.h	2009-07-17 06:30:13 +0000
+++ b/sql/fk.h	2009-07-20 12:52:17 +0000
@@ -38,7 +38,7 @@ public:
   Fk_constraint_list(trg_event_type action_type_arg,
                      Fk_constraint_list *parent_fk_list)
     : m_action_type(action_type_arg),
-      m_table(NULL),
+      m_table(NULL), m_extra_table(NULL),
       m_parent_fk_list(parent_fk_list)
   { }
 
@@ -49,8 +49,8 @@ public:
   bool prepare_check_parent(THD *thd, TABLE *table,
                             MY_BITMAP *changed_column_map);
 
-  bool prepare_alter_check_parent(THD *thd,
-                                  TABLE *table, TABLE *table_2,
+  bool prepare_alter_check_parent(THD *thd, TABLE *table,
+                                  const char *table_tmp_name,
                                   List<Foreign_key_child> &fk_list);
 
   bool prepare_check_child(THD *thd, TABLE *table,
@@ -111,6 +111,9 @@ public:
 
   bool has_triggers();
 
+  bool lock_extra_table(THD *thd);
+  bool unlock_extra_table(THD *thd);
+
 private:
   /** Implementation. */
 
@@ -135,6 +138,13 @@ private:
   */
   TABLE *m_table;
   /**
+    Additional TABLE instance which is created for performing checks for
+    self-referencing foreign keys during ALTER TABLE. Unlike instance
+    from 'm_table' member it is created within method of this class and
+    it is responsibility of this class to destroy it at its end-of-life.
+  */
+  TABLE *m_extra_table;
+  /**
     List of foreign key constraints in which the subject table
     participates as a child.
   */

=== modified file 'sql/sql_table.cc'
--- a/sql/sql_table.cc	2009-07-17 06:30:13 +0000
+++ b/sql/sql_table.cc	2009-07-20 12:52:17 +0000
@@ -9352,7 +9352,7 @@ bool mysql_alter_table(THD *thd,char *ne
                        Alter_info *alter_info,
                        uint order_num, ORDER *order, bool ignore)
 {
-  TABLE *table, *new_table= 0, *new_table_2= 0;
+  TABLE *table, *new_table= 0;
   MDL_request target_mdl_request;
   bool has_target_mdl_lock= FALSE;
   int error= 0;
@@ -10105,29 +10105,6 @@ view_err:
                          FN_IS_TMP);
     /* Open our intermediate table */
     new_table= open_temporary_table(thd, path, new_db, tmp_name, 1, OTM_OPEN);
-
-    /*
-      TODO/FIXME: - Do this only if needed...
-                  - Move code to separate function...
-                  - Find out what should be done re locking...
-                  - Clarify what happens in case of error...
-
-      A death-defying stunt - creation of second instance of TABLE object
-      for a temporary table.
-
-      QQ: What is the correct approach to this ? May be we should rely on the
-          new interface for cursors ? Should we call mysql_lock_tables() and
-          such?
-    */
-    if (! (new_table_2= (TABLE*) my_malloc(sizeof(*new_table_2), MYF(MY_WME))) ||
-        open_table_from_share(thd, new_table->s, tmp_name,
-                              (uint) (HA_OPEN_KEYFILE | HA_OPEN_RNDFILE | HA_GET_INDEX),
-                              (READ_KEYINFO | COMPUTE_TYPES | EXTRA_RECORD),
-                              ha_open_options, new_table_2, OTM_OPEN))
-      goto err_new_table_cleanup;
-
-    new_table_2->reginfo.lock_type= TL_WRITE;
-    new_table_2->pos_in_table_list= 0;
   }
   if (!new_table)
     goto err_new_table_cleanup;
@@ -10157,7 +10134,7 @@ view_err:
   {
     Fk_constraint_list fk_list(TRG_EVENT_INSERT, NULL);
 
-    if (fk_list.prepare_alter_check_parent(thd, new_table, new_table_2,
+    if (fk_list.prepare_alter_check_parent(thd, new_table, tmp_name,
                                            alter_info->foreign_key_list))
     {
       error= 1;
@@ -10213,11 +10190,6 @@ view_err:
     not delete it! Even altough MERGE tables do not have their children
     attached here it is safe to call close_temporary_table().
   */
-  if (new_table_2)
-  {
-    close_temporary(new_table_2, 0, 0);
-    new_table_2= 0;
-  }
   close_temporary_table(thd, new_table, 1, 0);
   new_table= 0;
 
@@ -10396,11 +10368,6 @@ end_temporary:
   DBUG_RETURN(FALSE);
 
 err_new_table_cleanup:
-  if (new_table_2)
-  {
-    close_temporary(new_table_2, 0, 0);
-    new_table_2= 0;
-  }
   if (new_table)
   {
     /* close_temporary_table() frees the new_table pointer. */
@@ -10541,6 +10508,10 @@ copy_data_between_tables(TABLE *from,TAB
     goto err;
   errpos= 2;
 
+  if (fk_list->lock_extra_table(thd))
+    goto err;
+  errpos= 3;
+
   /* We need external lock before we can disable/enable keys */
   alter_table_manage_keys(to, from->file->indexes_are_disabled(), keys_onoff);
 
@@ -10551,7 +10522,7 @@ copy_data_between_tables(TABLE *from,TAB
 
   from->file->info(HA_STATUS_VARIABLE | HA_STATUS_NO_LOCK);
   to->file->ha_start_bulk_insert(from->file->stats.records);
-  errpos= 3;
+  errpos= 4;
 
   copy_end=copy;
   for (Field **ptr=to->field ; *ptr ; ptr++)
@@ -10614,7 +10585,7 @@ copy_data_between_tables(TABLE *from,TAB
   /* Tell handler that we have values for all columns in the to table */
   to->use_all_columns();
   init_read_record(&info, thd, from, (SQL_SELECT *) 0, 1, 1, FALSE);
-  errpos= 4;
+  errpos= 5;
   if (ignore)
     to->file->extra(HA_EXTRA_IGNORE_DUP_KEY);
   thd->warning_info->reset_current_row_for_warning();
@@ -10683,14 +10654,14 @@ copy_data_between_tables(TABLE *from,TAB
   }
 
 err:
-  if (errpos >= 4)
+  if (errpos >= 5)
     end_read_record(&info);
   free_io_cache(from);
   delete [] copy;
 
   if (error > 0)
     to->file->extra(HA_EXTRA_PREPARE_FOR_DROP);
-  if (errpos >= 3 && to->file->ha_end_bulk_insert(error > 1) && error <= 0)
+  if (errpos >= 4 && to->file->ha_end_bulk_insert(error > 1) && error <= 0)
   {
     to->file->print_error(my_errno,MYF(0));
     error= 1;
@@ -10717,6 +10688,9 @@ err:
   *copied= found_count;
   *deleted=delete_count;
   to->file->ha_release_auto_increment();
+
+  if (errpos >= 3 && fk_list->unlock_extra_table(thd))
+    error= 1;
   if (errpos >= 2 && to->file->ha_external_lock(thd,F_UNLCK))
     error=1;
   if (error < 0 && to->file->extra(HA_EXTRA_PREPARE_FOR_RENAME))


Attachment: [text/bzr-bundle] bzr/dlenev@mysql.com-20090720125217-ovx8p56rqcowdgv4.bundle
Thread
bzr commit into mysql-6.1-fk branch (dlenev:2742) WL#148Dmitry Lenev20 Jul