List:Commits« Previous MessageNext Message »
From:Dmitry Lenev Date:February 1 2012 7:32am
Subject:bzr push into mysql-trunk-wl5534 branch (Dmitry.Lenev:3475 to 3476) WL#5534
View as plain text  
 3476 Dmitry Lenev	2012-02-01
      WL#5534 Online ALTER, Phase 1.
      
      Patch #82:
        Made HA_CREATE_INFO and Alter_info part of
        Alter_inplace_info. Documented their state.

    modified:
      sql/ha_partition.cc
      sql/ha_partition.h
      sql/handler.cc
      sql/handler.h
      sql/sql_table.cc
 3475 Dmitry Lenev	2012-01-31
      WL#5534 Online ALTER, Phase 1.
      
      Patch #81:
        - Documented how contents of Alter_inplace_info::key_info_buffer
          are produced.
        - Got rid of redundant calls to mysql_prepare_create_table().

    modified:
      mysql-test/r/archive.result
      mysql-test/r/ctype_utf8mb4.result
      mysql-test/suite/innodb/r/innodb_index_large_prefix.result
      mysql-test/suite/innodb/r/innodb_mysql.result
      mysql-test/suite/innodb/r/innodb_prefix_index_liftedlimit.result
      sql/ha_partition.cc
      sql/handler.h
      sql/sql_table.cc
=== modified file 'sql/ha_partition.cc'
--- a/sql/ha_partition.cc	2012-01-31 14:35:18 +0000
+++ b/sql/ha_partition.cc	2012-02-01 07:31:47 +0000
@@ -6941,8 +6941,6 @@ public:
 
 enum_alter_inplace_result
 ha_partition::check_if_supported_inplace_alter(TABLE *altered_table,
-                                               HA_CREATE_INFO *create_info,
-                                               const Alter_info *alter_info,
                                                Alter_inplace_info *ha_alter_info)
 {
   uint index= 0;
@@ -6969,8 +6967,6 @@ ha_partition::check_if_supported_inplace
   {
     enum_alter_inplace_result p_result=
       m_file[index]->check_if_supported_inplace_alter(altered_table,
-                                                      create_info,
-                                                      alter_info,
                                                       ha_alter_info);
     part_inplace_ctx->handler_ctx_array[index]= ha_alter_info->handler_ctx;
 
@@ -6986,7 +6982,6 @@ ha_partition::check_if_supported_inplace
 
 
 bool ha_partition::prepare_inplace_alter_table(TABLE *altered_table,
-                                               HA_CREATE_INFO *create_info,
                                                Alter_inplace_info *ha_alter_info)
 {
   uint index= 0;
@@ -7001,7 +6996,7 @@ bool ha_partition::prepare_inplace_alter
   for (index= 0; index < m_tot_parts && !error; index++)
   {
     ha_alter_info->handler_ctx= part_inplace_ctx->handler_ctx_array[index];
-    if (m_file[index]->ha_prepare_inplace_alter_table(altered_table, create_info,
+    if (m_file[index]->ha_prepare_inplace_alter_table(altered_table,
                                                       ha_alter_info))
       error= true;
     part_inplace_ctx->handler_ctx_array[index]= ha_alter_info->handler_ctx;
@@ -7013,7 +7008,6 @@ bool ha_partition::prepare_inplace_alter
 
 
 bool ha_partition::inplace_alter_table(TABLE *altered_table,
-                                       HA_CREATE_INFO *create_info,
                                        Alter_inplace_info *ha_alter_info)
 {
   uint index= 0;
@@ -7028,7 +7022,7 @@ bool ha_partition::inplace_alter_table(T
   for (index= 0; index < m_tot_parts && !error; index++)
   {
     ha_alter_info->handler_ctx= part_inplace_ctx->handler_ctx_array[index];
-    if (m_file[index]->ha_inplace_alter_table(altered_table, create_info,
+    if (m_file[index]->ha_inplace_alter_table(altered_table,
                                               ha_alter_info))
       error= true;
     part_inplace_ctx->handler_ctx_array[index]= ha_alter_info->handler_ctx;
@@ -7047,7 +7041,6 @@ bool ha_partition::inplace_alter_table(T
   (As X-lock will be held here if new indexes are to be committed)
 */
 bool ha_partition::commit_inplace_alter_table(TABLE *altered_table,
-                                              HA_CREATE_INFO *create_info,
                                               Alter_inplace_info *ha_alter_info,
                                               bool commit)
 {
@@ -7065,7 +7058,7 @@ bool ha_partition::commit_inplace_alter_
   for (index= 0; index < m_tot_parts; index++)
   {
     ha_alter_info->handler_ctx= part_inplace_ctx->handler_ctx_array[index];
-    if (m_file[index]->ha_commit_inplace_alter_table(altered_table, create_info,
+    if (m_file[index]->ha_commit_inplace_alter_table(altered_table,
                                                      ha_alter_info, commit))
     {
       part_inplace_ctx->handler_ctx_array[index]= ha_alter_info->handler_ctx;
@@ -7078,7 +7071,7 @@ bool ha_partition::commit_inplace_alter_
       {
         index++;
         ha_alter_info->handler_ctx= part_inplace_ctx->handler_ctx_array[index];
-        m_file[index]->ha_commit_inplace_alter_table(altered_table, create_info,
+        m_file[index]->ha_commit_inplace_alter_table(altered_table,
                                                      ha_alter_info, false);
         part_inplace_ctx->handler_ctx_array[index]= ha_alter_info->handler_ctx;
         goto err;
@@ -7100,7 +7093,11 @@ err:
                                       Alter_inplace_info::ADD_UNIQUE_INDEX |
                                       Alter_inplace_info::ADD_PK_INDEX))
   {
-    Alter_inplace_info drop_info(ha_alter_info->ignore, NULL, 0);
+    Alter_inplace_info drop_info(ha_alter_info->create_info,
+                                 ha_alter_info->alter_info,
+                                 NULL, 0,
+                                 ha_alter_info->ignore);
+
     if (ha_alter_info->handler_flags & Alter_inplace_info::ADD_INDEX)
       drop_info.handler_flags|= Alter_inplace_info::DROP_INDEX;
     if (ha_alter_info->handler_flags & Alter_inplace_info::ADD_UNIQUE_INDEX)
@@ -7127,12 +7124,9 @@ err:
     for (uint i= 0; i < index; i++)
     {
       bool error= m_file[i]->ha_prepare_inplace_alter_table(altered_table,
-                                                            create_info,
                                                             &drop_info);
-      error|= m_file[i]->ha_inplace_alter_table(altered_table, create_info,
-                                                &drop_info);
+      error|= m_file[i]->ha_inplace_alter_table(altered_table, &drop_info);
       error|= m_file[i]->ha_commit_inplace_alter_table(altered_table,
-                                                       create_info,
                                                        &drop_info, true);
       if (error)
         sql_print_error("Failed with error handling of adding index:\n"
@@ -7146,7 +7140,7 @@ err:
     for (uint i= index+1; i < m_tot_parts; i++)
     {
       ha_alter_info->handler_ctx= part_inplace_ctx->handler_ctx_array[i];
-      if (m_file[i]->ha_commit_inplace_alter_table(altered_table, create_info,
+      if (m_file[i]->ha_commit_inplace_alter_table(altered_table,
                                                    ha_alter_info, false))
       {
         /* How could this happen? */

=== modified file 'sql/ha_partition.h'
--- a/sql/ha_partition.h	2012-01-26 10:02:26 +0000
+++ b/sql/ha_partition.h	2012-02-01 07:31:47 +0000
@@ -1057,17 +1057,12 @@ public:
   */
     virtual enum_alter_inplace_result
       check_if_supported_inplace_alter(TABLE *altered_table,
-                                       HA_CREATE_INFO *create_info,
-                                       const Alter_info *alter_info,
                                        Alter_inplace_info *ha_alter_info);
     virtual bool prepare_inplace_alter_table(TABLE *altered_table,
-                                             HA_CREATE_INFO *create_info,
                                              Alter_inplace_info *ha_alter_info);
     virtual bool inplace_alter_table(TABLE *altered_table,
-                                     HA_CREATE_INFO *create_info,
                                      Alter_inplace_info *ha_alter_info);
     virtual bool commit_inplace_alter_table(TABLE *altered_table,
-                                            HA_CREATE_INFO *create_info,
                                             Alter_inplace_info *ha_alter_info,
                                             bool commit);
     virtual void notify_table_changed();

=== modified file 'sql/handler.cc'
--- a/sql/handler.cc	2012-01-31 12:45:28 +0000
+++ b/sql/handler.cc	2012-02-01 07:31:47 +0000
@@ -3669,7 +3669,6 @@ handler::ha_discard_or_import_tablespace
 
 
 bool handler::ha_commit_inplace_alter_table(TABLE *altered_table,
-                                            HA_CREATE_INFO *create_info,
                                             Alter_inplace_info *ha_alter_info,
                                             bool commit)
 {
@@ -3685,8 +3684,7 @@ bool handler::ha_commit_inplace_alter_ta
                                                    MDL_EXCLUSIVE) ||
                !commit);
 
-   return commit_inplace_alter_table(altered_table, create_info,
-                                     ha_alter_info, commit);
+   return commit_inplace_alter_table(altered_table, ha_alter_info, commit);
 }
 
 
@@ -3768,12 +3766,12 @@ static void check_alter_index_flags(ulon
 
 enum_alter_inplace_result
 handler::check_if_supported_inplace_alter(TABLE *altered_table,
-                                          HA_CREATE_INFO *create_info,
-                                          const Alter_info *alter_info,
                                           Alter_inplace_info *ha_alter_info)
 {
   DBUG_ENTER("check_if_supported_alter");
 
+  HA_CREATE_INFO *create_info= ha_alter_info->create_info;
+
   ulong handler_alter_flags= table->file->alter_table_flags(0);
   DBUG_PRINT("info", ("handler_alter_flags: %lu", handler_alter_flags));
 
@@ -3947,7 +3945,6 @@ handler::check_if_supported_inplace_alte
 
 
 bool handler::ha_prepare_inplace_alter_table(TABLE *altered_table,
-                                             HA_CREATE_INFO *create_info,
                                              Alter_inplace_info *ha_alter_info)
 {
   DBUG_ASSERT(table_share->tmp_table != NO_TMP_TABLE ||
@@ -3956,8 +3953,7 @@ bool handler::ha_prepare_inplace_alter_t
 
   prepare_for_alter();
 
-  return prepare_inplace_alter_table(altered_table, create_info,
-                                     ha_alter_info);
+  return prepare_inplace_alter_table(altered_table, ha_alter_info);
 }
 
 
@@ -3967,7 +3963,6 @@ bool handler::ha_prepare_inplace_alter_t
 */
 
 bool handler::inplace_alter_table(TABLE *altered_table,
-                                  HA_CREATE_INFO *create_info,
                                   Alter_inplace_info *ha_alter_info)
 {
   DBUG_ENTER("inplace_alter_table");
@@ -4065,7 +4060,6 @@ bool handler::inplace_alter_table(TABLE
 */
 
 bool handler::commit_inplace_alter_table(TABLE *altered_table,
-                                         HA_CREATE_INFO *create_info,
                                          Alter_inplace_info *ha_alter_info,
                                          bool commit)
 {

=== modified file 'sql/handler.h'
--- a/sql/handler.h	2012-01-31 14:35:18 +0000
+++ b/sql/handler.h	2012-02-01 07:31:47 +0000
@@ -1028,6 +1028,30 @@ public:
   static const HA_ALTER_FLAGS ALTER_RENAME               = 1L << 17;
 
   /**
+    Create options (like MAX_ROWS) for the new version of table.
+
+    @note The referenced instance of HA_CREATE_INFO object was already
+          used to create new .FRM file for table being altered. So it
+          has been processed by mysql_prepare_create_table() already.
+          For example, this means that it has HA_OPTION_PACK_RECORD
+          flag in HA_CREATE_INFO::table_options member correctly set.
+  */
+  HA_CREATE_INFO *create_info;
+
+  /**
+    Alter options, fields and keys for the new version of table.
+
+    @note The referenced instance of Alter_info object was already
+          used to create new .FRM file for table being altered. So it
+          has been processed by mysql_prepare_create_table() already.
+          In particular, this means that in Create_field objects for
+          fields which were present in some form in the old version
+          of table, Create_field::field member points to corresponding
+          Field instance for old version of table.
+  */
+  Alter_info *alter_info;
+
+  /**
     Array of KEYs for new version of table - including KEYs to be added.
 
     @note Currently this array is produced as result of
@@ -1081,8 +1105,13 @@ public:
   /** true for ALTER IGNORE TABLE ... */
   const bool ignore;
 
-  Alter_inplace_info(bool ignore_arg, KEY *key_info_arg, uint key_count_arg)
-    :key_info_buffer(key_info_arg),
+  Alter_inplace_info(HA_CREATE_INFO *create_info_arg,
+                     Alter_info *alter_info_arg,
+                     KEY *key_info_arg, uint key_count_arg,
+                     bool ignore_arg)
+    : create_info(create_info_arg),
+    alter_info(alter_info_arg),
+    key_info_buffer(key_info_arg),
     key_count(key_count_arg),
     index_drop_count(0),
     index_drop_buffer(NULL),
@@ -2521,10 +2550,9 @@ public:
     Check if a storage engine supports a particular alter table in-place
 
     @param    altered_table     TABLE object for new version of table.
-    @param    create_info       Information from the parsing phase about new
-                                table properties.
-    @param    alter_info        Data related to detected changes.
-    @param    ha_alter_info     Structure holding data used during in-place alter.
+    @param    ha_alter_info     Structure describing changes to be done
+                                by ALTER TABLE and holding data used
+                                during in-place alter.
 
     @retval   HA_ALTER_ERROR                  Unexpected error.
     @retval   HA_ALTER_INPLACE_NOT_SUPPORTED  Not supported, must use copy.
@@ -2541,15 +2569,9 @@ public:
     to determine if the storage engine supports in-place ALTER or not.
 
     @note Called without holding thr_lock.c lock.
-
-    TODO/FIXME: Clarify if HA_CREATE_INFO and Alter_info get here after
-                being processed by mysql_prepare_create_table() or straigth
-                from the parser.
  */
  virtual enum_alter_inplace_result
  check_if_supported_inplace_alter(TABLE *altered_table,
-                                  HA_CREATE_INFO *create_info,
-                                  const Alter_info *alter_info,
                                   Alter_inplace_info *ha_alter_info);
 
 
@@ -2558,7 +2580,6 @@ public:
     @see prepare_inplace_alter_table()
  */
  bool ha_prepare_inplace_alter_table(TABLE *altered_table,
-                                     HA_CREATE_INFO *create_info,
                                      Alter_inplace_info *ha_alter_info);
 
 
@@ -2567,10 +2588,9 @@ public:
     @see inplace_alter_table()
  */
  bool ha_inplace_alter_table(TABLE *altered_table,
-                             HA_CREATE_INFO *create_info,
                              Alter_inplace_info *ha_alter_info)
  {
-   return inplace_alter_table(altered_table, create_info, ha_alter_info);
+   return inplace_alter_table(altered_table, ha_alter_info);
  }
 
 
@@ -2580,7 +2600,6 @@ public:
     @see commit_inplace_alter_table()
  */
  bool ha_commit_inplace_alter_table(TABLE *altered_table,
-                                    HA_CREATE_INFO *create_info,
                                     Alter_inplace_info *ha_alter_info,
                                     bool commit);
 
@@ -2616,15 +2635,14 @@ protected:
     for a given partition.
 
     @param    altered_table     TABLE object for new version of table.
-    @param    create_info       Information from the parsing phase about new
-                                table properties.
-    @param    ha_alter_info     Structure holding data used during in-place alter.
+    @param    ha_alter_info     Structure describing changes to be done
+                                by ALTER TABLE and holding data used
+                                during in-place alter.
 
     @retval   true              Error
     @retval   false             Success
  */
  virtual bool prepare_inplace_alter_table(TABLE *altered_table,
-                                          HA_CREATE_INFO *create_info,
                                           Alter_inplace_info *ha_alter_info)
  { return false; }
 
@@ -2641,16 +2659,14 @@ protected:
     will be called with commit= false.
 
     @param    altered_table     TABLE object for new version of table.
-    @param    create_info       Information from the parsing phase about new
-                                table properties.
-    @param    ha_alter_info     Structure holding data used during in-place alter.
-    @param    alter_flags       Bitmask that shows what will be changed.
+    @param    ha_alter_info     Structure describing changes to be done
+                                by ALTER TABLE and holding data used
+                                during in-place alter.
 
     @retval   true              Error
     @retval   false             Success
  */
  virtual bool inplace_alter_table(TABLE *altered_table,
-                                  HA_CREATE_INFO *create_info,
                                   Alter_inplace_info *ha_alter_info);
 
 
@@ -2674,17 +2690,15 @@ protected:
     @see prepare_inplace_alter_table().
 
     @param    altered_table     TABLE object for new version of table.
-    @param    create_info       Information from the parsing phase about new
-                                table properties.
-    @param    ha_alter_info     Structure holding data used during in-place alter.
-    @param    alter_flags       Bitmask that shows what will be changed.
+    @param    ha_alter_info     Structure describing changes to be done
+                                by ALTER TABLE and holding data used
+                                during in-place alter.
     @param    commit            True => Commit, False => Rollback.
 
     @retval   true              Error
     @retval   false             Success
  */
  virtual bool commit_inplace_alter_table(TABLE *altered_table,
-                                         HA_CREATE_INFO *create_info,
                                          Alter_inplace_info *ha_alter_info,
                                          bool commit);
 

=== modified file 'sql/sql_table.cc'
--- a/sql/sql_table.cc	2012-01-31 14:35:18 +0000
+++ b/sql/sql_table.cc	2012-02-01 07:31:47 +0000
@@ -5082,14 +5082,16 @@ static int compare_uint(const uint *s, c
    Compare original and new versions of a table and fill Alter_inplace_info
    describing differences between those versions.
 
-   @param       thd                Thread
-   @param       table              The original table.
-   @param       alter_info         Alter options, fields and keys for the new
-                                   table.
-   @param       create_info        Create options for the new table.
-   @param       varchar            Indicates that new definition has new
-                                   VARCHAR column.
-   @param[out]  ha_alter_info      Data structures needed for in-place alter
+   @param          thd                Thread
+   @param          table              The original table.
+   @param          varchar            Indicates that new definition has new
+                                      VARCHAR column.
+   @param[in/out]  ha_alter_info      Data structure which already contains
+                                      basic information about create options,
+                                      field and keys for the new version of
+                                      table and which should be completed with
+                                      more detailed information needed for
+                                      in-place ALTER.
 
    First argument 'table' contains information of the original
    table, which includes all corresponding parts that the new
@@ -5120,8 +5122,6 @@ static int compare_uint(const uint *s, c
 
 static bool fill_alter_inplace_info(THD *thd,
                                     TABLE *table,
-                                    Alter_info *alter_info,
-                                    HA_CREATE_INFO *create_info,
                                     bool varchar,
                                     Alter_inplace_info *ha_alter_info)
 {
@@ -5131,6 +5131,7 @@ static bool fill_alter_inplace_info(THD
   KEY_PART_INFO *key_part, *new_part;
   KEY_PART_INFO *end;
   uint candidate_key_count= 0;
+  Alter_info *alter_info= ha_alter_info->alter_info;
   DBUG_ENTER("fill_alter_inplace_info");
 
   /* Allocate result buffers. */
@@ -5782,11 +5783,9 @@ static bool is_inplace_alter_impossible(
   @param table_list         TABLE_LIST for the table to change.
   @param table              The original TABLE.
   @param altered_table      TABLE object for new version of the table.
-  @param create_info        Information from the parsing phase about new
-                            table properties.
-  @param alter_info         Data related to detected changes.
-  @param ha_alter_flags     Bitmask that shows what will be changed.
-  @param ha_alter_info      Storage place for data used during different phases.
+  @param ha_alter_info      Structure describing ALTER TABLE to be carried
+                            out and serving as a storage place for data
+                            used during different phases.
   @param inplace_supported  Enum describing the locking requirements.
   @param target_mdl_request Metadata request/lock on the target table name.
   @param alter_ctx          ALTER TABLE runtime context.
@@ -5811,8 +5810,6 @@ static bool mysql_inplace_alter_table(TH
                                       TABLE_LIST *table_list,
                                       TABLE *table,
                                       TABLE *altered_table,
-                                      HA_CREATE_INFO *create_info,
-                                      Alter_info *alter_info,
                                       Alter_inplace_info *ha_alter_info,
                                       enum_alter_inplace_result inplace_supported,
                                       MDL_request *target_mdl_request,
@@ -5821,6 +5818,8 @@ static bool mysql_inplace_alter_table(TH
   Open_table_context ot_ctx(thd, MYSQL_OPEN_REOPEN);
   handlerton *db_type= table->s->db_type();
   MDL_ticket *mdl_ticket= table->mdl_ticket;
+  HA_CREATE_INFO *create_info= ha_alter_info->create_info;
+  Alter_info *alter_info= ha_alter_info->alter_info;
 
   DBUG_ENTER("mysql_inplace_alter_table");
 
@@ -5860,7 +5859,6 @@ static bool mysql_inplace_alter_table(TH
   DEBUG_SYNC(thd, "alter_table_inplace_after_lock_upgrade");
 
   if (table->file->ha_prepare_inplace_alter_table(altered_table,
-                                                  create_info,
                                                   ha_alter_info))
   {
     goto rollback;
@@ -5885,7 +5883,6 @@ static bool mysql_inplace_alter_table(TH
   DEBUG_SYNC(thd, "alter_table_inplace_after_lock_downgrade");
 
   if (table->file->ha_inplace_alter_table(altered_table,
-                                          create_info,
                                           ha_alter_info))
   {
     goto rollback;
@@ -5897,7 +5894,6 @@ static bool mysql_inplace_alter_table(TH
 
   DBUG_EXECUTE_IF("alter_table_rollback_new_index", {
       table->file->ha_commit_inplace_alter_table(altered_table,
-                                                 create_info,
                                                  ha_alter_info,
                                                  false);
       my_error(ER_UNKNOWN_ERROR, MYF(0));
@@ -5907,7 +5903,6 @@ static bool mysql_inplace_alter_table(TH
   DEBUG_SYNC(thd, "alter_table_inplace_before_commit");
 
   if (table->file->ha_commit_inplace_alter_table(altered_table,
-                                                 create_info,
                                                  ha_alter_info,
                                                  true))
   {
@@ -5989,7 +5984,6 @@ static bool mysql_inplace_alter_table(TH
 
  rollback:
   table->file->ha_commit_inplace_alter_table(altered_table,
-                                             create_info,
                                              ha_alter_info,
                                              false);
  cleanup:
@@ -7049,13 +7043,13 @@ bool mysql_alter_table(THD *thd,char *ne
 
   if (alter_info->requested_algorithm != Alter_info::ALTER_TABLE_ALGORITHM_COPY)
   {
-    Alter_inplace_info ha_alter_info(ignore, key_info, key_count);
+    Alter_inplace_info ha_alter_info(create_info, alter_info,
+                                     key_info, key_count, ignore);
     TABLE *altered_table= NULL;
     bool use_inplace= true;
 
     /* Fill the Alter_inplace_info structure. */
-    if (fill_alter_inplace_info(thd, table, alter_info, create_info,
-                                varchar, &ha_alter_info))
+    if (fill_alter_inplace_info(thd, table, varchar, &ha_alter_info))
       goto err_new_table_cleanup;
 
     // We assume that the table is non-temporary.
@@ -7088,8 +7082,6 @@ bool mysql_alter_table(THD *thd,char *ne
     // Ask storage engine whether to use copy or in-place
     enum_alter_inplace_result inplace_supported=
       table->file->check_if_supported_inplace_alter(altered_table,
-                                                    create_info,
-                                                    alter_info,
                                                     &ha_alter_info);
 
     switch (inplace_supported) {
@@ -7148,7 +7140,6 @@ bool mysql_alter_table(THD *thd,char *ne
     {
       if (mysql_inplace_alter_table(thd, table_list, table,
                                     altered_table,
-                                    create_info, alter_info,
                                     &ha_alter_info,
                                     inplace_supported, &target_mdl_request,
                                     &alter_ctx))

No bundle (reason: useless for push emails).
Thread
bzr push into mysql-trunk-wl5534 branch (Dmitry.Lenev:3475 to 3476) WL#5534Dmitry Lenev1 Feb