List:Commits« Previous MessageNext Message »
From:Jon Olav Hauglid Date:March 21 2011 4:48pm
Subject:bzr commit into mysql-trunk branch (jon.hauglid:3304) WL#5534
View as plain text  
#At file:///export/home/x/mysql-trunk-alter-refactor/ based on revid:georgi.kodinov@stripped

 3304 Jon Olav Hauglid	2011-03-21
      WL#5534 Online ALTER in 5.6*
      
      Tentative pre-requisite patch, first round of refactoring.
      
      The following changes are made to mysql_alter_table():
      - Extracted new simple_rename_or_index_change() function
        for ALTER_RENAME / ALTER_KEYS_ONOFF without touching .FRM.
      - Extracted new can_alter_index_inplace() function
        for determining of add/drop index can be done in-place
        and if so, the level of locking required.
      - Extracted new alter_indexes() function
        for in-place add/drop of indexes.
      - Removed need_copy_table variable, use Alter_info::change_level
        instead.
      - A couple of code blocks moved closer to related code.

    modified:
      sql/sql_table.cc
=== modified file 'sql/sql_table.cc'
--- a/sql/sql_table.cc	2011-03-17 11:11:39 +0000
+++ b/sql/sql_table.cc	2011-03-21 16:48:06 +0000
@@ -5817,6 +5817,419 @@ err:
 }
 
 
+/**
+  Rename table and/or turn indexes on/off without touching .FRM
+
+  @param thd          Thread handler
+  @param table_list   TABLE_LIST for the table to change
+  @param table        TABLE for the table to change
+  @param alter_info   List of keys to be changed
+  @param new_db       New datbase name
+  @param new_name     New table name
+  @param new_alias    New table alias
+  @param db           Old database name
+  @param table_name   Old table name
+  @param alias        Old alias
+  @param old_db_type  Table type for handler
+  @param mdl_ticket   Ticket representing the MDL lock on the table
+
+  @return Operation status
+    @retval 0               Success
+    @retval != 0            Failure
+*/
+
+static int simple_rename_or_index_change(THD *thd, TABLE_LIST *table_list,
+                                         TABLE *table, Alter_info *alter_info,
+                                         char *new_db, char *new_name,
+                                         char *new_alias, char *db,
+                                         char *table_name, char *alias,
+                                         handlerton *old_db_type,
+                                         MDL_ticket *mdl_ticket)
+{
+  int error= 0;
+  switch (alter_info->keys_onoff) {
+  case LEAVE_AS_IS:
+    break;
+  case ENABLE:
+    if (wait_while_table_is_used(thd, table, HA_EXTRA_FORCE_REOPEN))
+      return -1;
+    DBUG_EXECUTE_IF("sleep_alter_enable_indexes", my_sleep(6000000););
+    error= table->file->ha_enable_indexes(HA_KEY_SWITCH_NONUNIQ_SAVE);
+    break;
+  case DISABLE:
+    if (wait_while_table_is_used(thd, table, HA_EXTRA_FORCE_REOPEN))
+      return -1;
+    error=table->file->ha_disable_indexes(HA_KEY_SWITCH_NONUNIQ_SAVE);
+    break;
+  default:
+    DBUG_ASSERT(FALSE);
+    error= 0;
+    break;
+  }
+  if (error == HA_ERR_WRONG_COMMAND)
+  {
+    push_warning_printf(thd, MYSQL_ERROR::WARN_LEVEL_NOTE,
+                        ER_ILLEGAL_HA, ER(ER_ILLEGAL_HA),
+                        table->alias);
+    error= 0;
+  }
+  else if (error > 0)
+  {
+    table->file->print_error(error, MYF(0));
+    error= -1;
+  }
+
+  if (!error && (new_name != table_name || new_db != db))
+  {
+    thd_proc_info(thd, "rename");
+    /*
+      Then do a 'simple' rename of the table. First we need to close all
+      instances of 'source' table.
+      Note that if wait_while_table_is_used() returns error here (i.e. if
+      this thread was killed) then it must be that previous step of
+      simple rename did nothing and therefore we can safely return
+      without additional clean-up.
+    */
+    if (wait_while_table_is_used(thd, table, HA_EXTRA_FORCE_REOPEN))
+      return -1;
+    close_all_tables_for_name(thd, table->s, TRUE);
+
+    *fn_ext(new_name)=0;
+    if (mysql_rename_table(old_db_type,db,table_name,new_db,new_alias, 0))
+      error= -1;
+    else if (Table_triggers_list::change_table_name(thd, db,
+                                                    alias, table_name,
+                                                    new_db, new_alias))
+    {
+      (void) mysql_rename_table(old_db_type, new_db, new_alias, db,
+                                table_name, 0);
+      error= -1;
+    }
+  }
+
+  if (!error)
+  {
+    error= write_bin_log(thd, TRUE, thd->query(), thd->query_length());
+    if (!error)
+      my_ok(thd);
+  }
+  table_list->table= NULL;                    // For query cache
+  query_cache_invalidate3(thd, table_list, 0);
+
+  if ((thd->locked_tables_mode == LTM_LOCK_TABLES ||
+       thd->locked_tables_mode == LTM_PRELOCKED_UNDER_LOCK_TABLES))
+  {
+    /*
+      Under LOCK TABLES we should adjust meta-data locks before finishing
+      statement. Otherwise we can rely on them being released
+      along with the implicit commit.
+    */
+    if (new_name != table_name || new_db != db)
+      thd->mdl_context.release_all_locks_for_name(mdl_ticket);
+    else
+      mdl_ticket->downgrade_exclusive_lock(MDL_SHARED_NO_READ_WRITE);
+  }
+  return error;
+}
+
+
+/**
+  Check if indexes can be dropped/added in-place.
+
+  @param table[in]                  Table to be modified
+  @param alter_info[in]             Alter options, fields and keys for
+                                    the new table
+  @param index_drop_count[in]       The number of indexes to drop
+  @param index_drop_buffer[in]      An array of offsets into table->key_info
+  @param index_add_count[in]        The number of indexes to add
+  @param index_add_buffer[in]       An array of offsets into key_info_buffer
+  @param key_info_buffer[in]        An array of KEY structs for new indexes
+  @param need_lock_for_indexes[out] true if exclusive lock is needed
+
+  @return Value indicating if in-place operation is supported
+*/
+
+static bool can_alter_index_inplace(TABLE *table, Alter_info *alter_info,
+                                    uint index_drop_count,
+                                    uint *index_drop_buffer,
+                                    uint index_add_count,
+                                    uint *index_add_buffer,
+                                    uint candidate_key_count,
+                                    KEY *key_info_buffer,
+                                    bool *need_lock_for_indexes)
+{
+  int   pk_changed= 0;
+  ulong alter_flags= 0;
+  ulong needed_inplace_with_read_flags= 0;
+  ulong needed_inplace_flags= 0;
+  KEY   *key;
+  uint  *idx_p;
+  uint  *idx_end_p;
+  bool no_pk;
+  bool can_alter_index_inplace= false;
+
+  alter_flags= table->file->alter_table_flags(alter_info->flags);
+  DBUG_PRINT("info", ("alter_flags: %lu", alter_flags));
+  /* Check dropped indexes. */
+  for (idx_p= index_drop_buffer, idx_end_p= idx_p + index_drop_count;
+       idx_p < idx_end_p;
+       idx_p++)
+  {
+    key= table->key_info + *idx_p;
+    DBUG_PRINT("info", ("index dropped: '%s'", key->name));
+    if (key->flags & HA_NOSAME)
+    {
+      /*
+         Unique key. Check for "PRIMARY".
+         or if dropping last unique key
+      */
+      if ((uint) (key - table->key_info) == table->s->primary_key)
+      {
+        DBUG_PRINT("info", ("Dropping primary key"));
+        /* Primary key. */
+        needed_inplace_with_read_flags|= HA_INPLACE_DROP_PK_INDEX_NO_WRITE;
+        needed_inplace_flags|= HA_INPLACE_DROP_PK_INDEX_NO_READ_WRITE;
+        pk_changed++;
+        candidate_key_count--;
+      }
+      else
+      {
+        KEY_PART_INFO *part_end= key->key_part + key->key_parts;
+        bool is_candidate_key= true;
+
+        /* Non-primary unique key. */
+        needed_inplace_with_read_flags|=
+          HA_INPLACE_DROP_UNIQUE_INDEX_NO_WRITE;
+        needed_inplace_flags|= HA_INPLACE_DROP_UNIQUE_INDEX_NO_READ_WRITE;
+
+        /*
+          Check if all fields in key are declared
+          NOT NULL and adjust candidate_key_count
+        */
+        for (KEY_PART_INFO *key_part= key->key_part;
+             key_part < part_end;
+             key_part++)
+          is_candidate_key=
+            (is_candidate_key &&
+             (! table->field[key_part->fieldnr-1]->maybe_null()));
+        if (is_candidate_key)
+          candidate_key_count--;
+      }
+    }
+    else
+    {
+      /* Non-unique key. */
+      needed_inplace_with_read_flags|= HA_INPLACE_DROP_INDEX_NO_WRITE;
+      needed_inplace_flags|= HA_INPLACE_DROP_INDEX_NO_READ_WRITE;
+    }
+  }
+  no_pk= ((table->s->primary_key == MAX_KEY) ||
+          (needed_inplace_with_read_flags &
+           HA_INPLACE_DROP_PK_INDEX_NO_WRITE));
+  /* Check added indexes. */
+  for (idx_p= index_add_buffer, idx_end_p= idx_p + index_add_count;
+       idx_p < idx_end_p;
+       idx_p++)
+  {
+    key= key_info_buffer + *idx_p;
+    DBUG_PRINT("info", ("index added: '%s'", key->name));
+    if (key->flags & HA_NOSAME)
+    {
+      /* Unique key */
+
+      KEY_PART_INFO *part_end= key->key_part + key->key_parts;
+      bool is_candidate_key= true;
+
+      /*
+        Check if all fields in key are declared
+        NOT NULL
+      */
+      for (KEY_PART_INFO *key_part= key->key_part;
+           key_part < part_end;
+           key_part++)
+        is_candidate_key=
+          (is_candidate_key &&
+           (! table->field[key_part->fieldnr]->maybe_null()));
+
+      /*
+        Check for "PRIMARY"
+        or if adding first unique key
+        defined on non-nullable fields
+      */
+
+      if ((!my_strcasecmp(system_charset_info,
+                          key->name, primary_key_name)) ||
+          (no_pk && candidate_key_count == 0 && is_candidate_key))
+      {
+        DBUG_PRINT("info", ("Adding primary key"));
+        /* Primary key. */
+        needed_inplace_with_read_flags|= HA_INPLACE_ADD_PK_INDEX_NO_WRITE;
+        needed_inplace_flags|= HA_INPLACE_ADD_PK_INDEX_NO_READ_WRITE;
+        pk_changed++;
+        no_pk= false;
+      }
+      else
+      {
+        /* Non-primary unique key. */
+        needed_inplace_with_read_flags|= HA_INPLACE_ADD_UNIQUE_INDEX_NO_WRITE;
+        needed_inplace_flags|= HA_INPLACE_ADD_UNIQUE_INDEX_NO_READ_WRITE;
+      }
+    }
+    else
+    {
+      /* Non-unique key. */
+      needed_inplace_with_read_flags|= HA_INPLACE_ADD_INDEX_NO_WRITE;
+      needed_inplace_flags|= HA_INPLACE_ADD_INDEX_NO_READ_WRITE;
+    }
+  }
+
+  if ((candidate_key_count > 0) &&
+      (needed_inplace_with_read_flags & HA_INPLACE_DROP_PK_INDEX_NO_WRITE))
+  {
+    /*
+      Dropped primary key when there is some other unique 
+      not null key that should be converted to primary key
+    */
+    needed_inplace_with_read_flags|= HA_INPLACE_ADD_PK_INDEX_NO_WRITE;
+    needed_inplace_flags|= HA_INPLACE_ADD_PK_INDEX_NO_READ_WRITE;
+    pk_changed= 2;
+  }
+
+  DBUG_PRINT("info",
+             ("needed_inplace_with_read_flags: 0x%lx, needed_inplace_flags: 0x%lx",
+              needed_inplace_with_read_flags, needed_inplace_flags));
+  /*
+    In-place add/drop index is possible only if
+    the primary key is not added and dropped in the same statement.
+    Otherwise we have to recreate the table.
+    need_copy_table is no-zero at this place.
+  */
+  if ( pk_changed < 2 )
+  {
+    if ((alter_flags & needed_inplace_with_read_flags) ==
+        needed_inplace_with_read_flags)
+    {
+      /* All required in-place flags to allow concurrent reads are present. */
+      can_alter_index_inplace= true;
+      *need_lock_for_indexes= false;
+    }
+    else if ((alter_flags & needed_inplace_flags) == needed_inplace_flags)
+    {
+      /* All required in-place flags are present. */
+      can_alter_index_inplace= true;
+    }
+  }
+  DBUG_PRINT("info", ("can_alter_index_inplace: %d  need_lock: %d",
+                      can_alter_index_inplace, *need_lock_for_indexes));
+  return can_alter_index_inplace;
+}
+
+
+/**
+  Execute in-place add and drop of indexes.
+
+  @param thd               Thread handle
+  @param table             Table to be modified
+  @param key_info_buffer   An array of KEY structs for new indexes
+  @param index_drop_count  The number of indexes to drop
+  @param index_drop_buffer An array of offsets into table->key_info
+  @param index_add_count   The number of indexes to add
+  @param index_add_buffer  An array of offsets into key_info_buffer
+
+  @return Operation status
+    @retval 0               Success
+    @retval != 0            Failure
+*/
+
+static int alter_indexes(THD *thd, TABLE *table, KEY *key_info_buffer,
+                         uint index_drop_count, uint *index_drop_buffer,
+                         uint index_add_count, uint *index_add_buffer)
+{
+  uint          *key_numbers;
+  uint          *keyno_p;
+  KEY           *key_info;
+  KEY           *key;
+  uint          *idx_p;
+  uint          *idx_end_p;
+  KEY_PART_INFO *key_part;
+  KEY_PART_INFO *part_end;
+  int           error;
+  DBUG_PRINT("info", ("No new_table, checking add/drop index"));
+
+  table->file->ha_prepare_for_alter();
+  if (index_add_count)
+  {
+    /* The add_index() method takes an array of KEY structs. */
+    key_info= (KEY*) thd->alloc(sizeof(KEY) * index_add_count);
+    key= key_info;
+    for (idx_p= index_add_buffer, idx_end_p= idx_p + index_add_count;
+         idx_p < idx_end_p;
+         idx_p++, key++)
+    {
+      /* Copy the KEY struct. */
+      *key= key_info_buffer[*idx_p];
+      /* Fix the key parts. */
+      part_end= key->key_part + key->key_parts;
+      for (key_part= key->key_part; key_part < part_end; key_part++)
+        key_part->field= table->field[key_part->fieldnr];
+    }
+    /* Add the indexes. */
+    if ((error= table->file->add_index(table, key_info, index_add_count)))
+    {
+      /*
+        Exchange the key_info for the error message. If we exchange
+        key number by key name in the message later, we need correct info.
+      */
+      KEY *save_key_info= table->key_info;
+      table->key_info= key_info;
+      table->file->print_error(error, MYF(0));
+      table->key_info= save_key_info;
+      return error;
+    }
+  }
+  /*end of if (index_add_count)*/
+
+  if (index_drop_count)
+  {
+    /* The prepare_drop_index() method takes an array of key numbers. */
+    key_numbers= (uint*) thd->alloc(sizeof(uint) * index_drop_count);
+    keyno_p= key_numbers;
+    /* Get the number of each key. */
+    for (idx_p= index_drop_buffer, idx_end_p= idx_p + index_drop_count;
+         idx_p < idx_end_p;
+         idx_p++, keyno_p++)
+      *keyno_p= *idx_p;
+    /*
+      Tell the handler to prepare for drop indexes.
+      This re-numbers the indexes to get rid of gaps.
+    */
+    if ((error= table->file->prepare_drop_index(table, key_numbers,
+                                                index_drop_count)))
+    {
+      table->file->print_error(error, MYF(0));
+      return error;
+    }
+
+    /* Tell the handler to finally drop the indexes. */
+    if ((error= table->file->final_drop_index(table)))
+    {
+      table->file->print_error(error, MYF(0));
+      return error;
+    }
+  }
+  /*end of if (index_drop_count)*/
+
+  /*
+    The final .frm file is already created as a temporary file
+    and will be renamed to the original table name later.
+  */
+
+  /* Need to commit before a table is unlocked (NDB requirement). */
+  DBUG_PRINT("info", ("Committing before unlocking table"));
+  return trans_commit_stmt(thd) || trans_commit_implicit(thd);
+}
+
+
 /*
   Alter table
 
@@ -5876,7 +6289,6 @@ bool mysql_alter_table(THD *thd,char *ne
   char reg_path[FN_REFLEN+1];
   ha_rows copied,deleted;
   handlerton *old_db_type, *new_db_type, *save_old_db_type;
-  enum_alter_table_change_level need_copy_table= ALTER_TABLE_METADATA_ONLY;
 #ifdef WITH_PARTITION_STORAGE_ENGINE
   TABLE *table_for_fast_alter_partition= NULL;
   bool partition_changed= FALSE;
@@ -5887,8 +6299,6 @@ bool mysql_alter_table(THD *thd,char *ne
   uint *index_drop_buffer= NULL;
   uint index_add_count= 0;
   uint *index_add_buffer= NULL;
-  uint candidate_key_count= 0;
-  bool no_pk;
   DBUG_ENTER("mysql_alter_table");
 
   /*
@@ -5933,20 +6343,6 @@ bool mysql_alter_table(THD *thd,char *ne
     }
   }
 
-  /*
-    Assign variables table_name, new_name, db, new_db, path, reg_path
-    to simplify further comparisions: we want to see if it's a RENAME
-    later just by comparing the pointers, avoiding the need for strcmp.
-  */
-  thd_proc_info(thd, "init");
-  table_name=table_list->table_name;
-  alias= (lower_case_table_names == 2) ? table_list->alias : table_name;
-  db=table_list->db;
-  if (!new_db || !my_strcasecmp(table_alias_charset, new_db, db))
-    new_db= db;
-  build_table_filename(reg_path, sizeof(reg_path) - 1, db, table_name, reg_ext, 0);
-  build_table_filename(path, sizeof(path) - 1, db, table_name, "", 0);
-
   mysql_ha_rm_tables(thd, table_list);
 
   /* DISCARD/IMPORT TABLESPACE is always alone in an ALTER TABLE */
@@ -5994,6 +6390,20 @@ bool mysql_alter_table(THD *thd,char *ne
     DBUG_RETURN(TRUE);
   }
 
+  /*
+    Assign variables table_name, new_name, db, new_db, path, reg_path
+    to simplify further comparisions: we want to see if it's a RENAME
+    later just by comparing the pointers, avoiding the need for strcmp.
+  */
+  thd_proc_info(thd, "init");
+  table_name=table_list->table_name;
+  alias= (lower_case_table_names == 2) ? table_list->alias : table_name;
+  db=table_list->db;
+  if (!new_db || !my_strcasecmp(table_alias_charset, new_db, db))
+    new_db= db;
+  build_table_filename(reg_path, sizeof(reg_path) - 1, db, table_name, reg_ext, 0);
+  build_table_filename(path, sizeof(path) - 1, db, table_name, "", 0);
+
   /* Check that we are not trying to rename to an existing table */
   if (new_name)
   {
@@ -6134,112 +6544,10 @@ bool mysql_alter_table(THD *thd,char *ne
   if (!(alter_info->flags & ~(ALTER_RENAME | ALTER_KEYS_ONOFF)) &&
       !table->s->tmp_table) // no need to touch frm
   {
-    switch (alter_info->keys_onoff) {
-    case LEAVE_AS_IS:
-      break;
-    case ENABLE:
-      if (wait_while_table_is_used(thd, table, HA_EXTRA_FORCE_REOPEN))
-        goto err;
-      DBUG_EXECUTE_IF("sleep_alter_enable_indexes", my_sleep(6000000););
-      error= table->file->ha_enable_indexes(HA_KEY_SWITCH_NONUNIQ_SAVE);
-      break;
-    case DISABLE:
-      if (wait_while_table_is_used(thd, table, HA_EXTRA_FORCE_REOPEN))
-        goto err;
-      error=table->file->ha_disable_indexes(HA_KEY_SWITCH_NONUNIQ_SAVE);
-      break;
-    default:
-      DBUG_ASSERT(FALSE);
-      error= 0;
-      break;
-    }
-    if (error == HA_ERR_WRONG_COMMAND)
-    {
-      error= 0;
-      push_warning_printf(thd, MYSQL_ERROR::WARN_LEVEL_NOTE,
-                          ER_ILLEGAL_HA, ER(ER_ILLEGAL_HA),
-                          table->alias);
-    }
-
-    if (!error && (new_name != table_name || new_db != db))
-    {
-      thd_proc_info(thd, "rename");
-      /*
-        Then do a 'simple' rename of the table. First we need to close all
-        instances of 'source' table.
-        Note that if wait_while_table_is_used() returns error here (i.e. if
-        this thread was killed) then it must be that previous step of
-        simple rename did nothing and therefore we can safely return
-        without additional clean-up.
-      */
-      if (wait_while_table_is_used(thd, table, HA_EXTRA_FORCE_REOPEN))
-        goto err;
-      close_all_tables_for_name(thd, table->s, TRUE);
-      /*
-        Then, we want check once again that target table does not exist.
-        Actually the order of these two steps does not matter since
-        earlier we took exclusive metadata lock on the target table, so
-        we do them in this particular order only to be consistent with 5.0,
-        in which we don't take this lock and where this order really matters.
-        TODO: Investigate if we need this access() check at all.
-      */
-      if (!access(new_name_buff,F_OK))
-      {
-        my_error(ER_TABLE_EXISTS_ERROR, MYF(0), new_name);
-        error= -1;
-      }
-      else
-      {
-        *fn_ext(new_name)=0;
-        if (mysql_rename_table(old_db_type,db,table_name,new_db,new_alias, 0))
-          error= -1;
-        else if (Table_triggers_list::change_table_name(thd, db,
-                                                        alias, table_name,
-                                                        new_db, new_alias))
-        {
-          (void) mysql_rename_table(old_db_type, new_db, new_alias, db,
-                                    table_name, 0);
-          error= -1;
-        }
-      }
-    }
-
-    if (error == HA_ERR_WRONG_COMMAND)
-    {
-      error= 0;
-      push_warning_printf(thd, MYSQL_ERROR::WARN_LEVEL_NOTE,
-                          ER_ILLEGAL_HA, ER(ER_ILLEGAL_HA),
-                          table->alias);
-    }
-
-    if (!error)
-    {
-      error= write_bin_log(thd, TRUE, thd->query(), thd->query_length());
-      if (!error)
-        my_ok(thd);
-    }
-    else if (error > 0)
-    {
-      table->file->print_error(error, MYF(0));
-      error= -1;
-    }
-    table_list->table= NULL;                    // For query cache
-    query_cache_invalidate3(thd, table_list, 0);
-
-    if ((thd->locked_tables_mode == LTM_LOCK_TABLES ||
-         thd->locked_tables_mode == LTM_PRELOCKED_UNDER_LOCK_TABLES))
-    {
-      /*
-        Under LOCK TABLES we should adjust meta-data locks before finishing
-        statement. Otherwise we can rely on them being released
-        along with the implicit commit.
-      */
-      if (new_name != table_name || new_db != db)
-        thd->mdl_context.release_all_locks_for_name(mdl_ticket);
-      else
-        mdl_ticket->downgrade_exclusive_lock(MDL_SHARED_NO_READ_WRITE);
-    }
-    DBUG_RETURN(error);
+    DBUG_RETURN(simple_rename_or_index_change(thd, table_list, table,
+                                              alter_info, new_db, new_name,
+                                              new_alias, db, table_name, alias,
+                                              old_db_type, mdl_ticket));
   }
 
   /* We have to do full alter table. */
@@ -6264,27 +6572,23 @@ bool mysql_alter_table(THD *thd,char *ne
   */
   new_db_type= create_info->db_type;
 
-  if (is_index_maintenance_unique (table, alter_info))
-    need_copy_table= ALTER_TABLE_DATA_CHANGED;
+  if (is_index_maintenance_unique (table, alter_info)
+      || thd->variables.old_alter_table
+#ifdef WITH_PARTITION_STORAGE_ENGINE
+      || partition_changed
+#endif
+      || (table->s->db_type() != create_info->db_type))
+    alter_info->change_level= ALTER_TABLE_DATA_CHANGED;
 
   if (mysql_prepare_alter_table(thd, table, create_info, alter_info))
     goto err;
 
-  if (need_copy_table == ALTER_TABLE_METADATA_ONLY)
-    need_copy_table= alter_info->change_level;
-
   set_table_default_charset(thd, create_info, db);
 
-  if (thd->variables.old_alter_table
-      || (table->s->db_type() != create_info->db_type)
-#ifdef WITH_PARTITION_STORAGE_ENGINE
-      || partition_changed
-#endif
-     )
-    need_copy_table= ALTER_TABLE_DATA_CHANGED;
-  else
+  if (alter_info->change_level == ALTER_TABLE_METADATA_ONLY)
   {
     Alter_table_change_level need_copy_table_res;
+    uint candidate_key_count= 0;
     /* Check how much the tables differ. */
     if (mysql_compare_tables(table, alter_info,
                              create_info, order_num,
@@ -6294,196 +6598,27 @@ bool mysql_alter_table(THD *thd,char *ne
                              &index_add_buffer, &index_add_count,
                              &candidate_key_count, FALSE))
       goto err;
-   
-    DBUG_EXECUTE_IF("alter_table_only_metadata_change", {
-      if (need_copy_table_res != ALTER_TABLE_METADATA_ONLY)
-        goto err; });
-    DBUG_EXECUTE_IF("alter_table_only_index_change", {
-      if (need_copy_table_res != ALTER_TABLE_INDEX_CHANGED)
-        goto err; });
-   
-    if (need_copy_table == ALTER_TABLE_METADATA_ONLY)
-      need_copy_table= need_copy_table_res;
-  }
-
-  /*
-    If there are index changes only, try to do them in-place. "Index
-    changes only" means also that the handler for the table does not
-    change. The table is open and locked. The handler can be accessed.
-  */
-  if (need_copy_table == ALTER_TABLE_INDEX_CHANGED)
-  {
-    int   pk_changed= 0;
-    ulong alter_flags= 0;
-    ulong needed_inplace_with_read_flags= 0;
-    ulong needed_inplace_flags= 0;
-    KEY   *key;
-    uint  *idx_p;
-    uint  *idx_end_p;
-
-    alter_flags= table->file->alter_table_flags(alter_info->flags);
-    DBUG_PRINT("info", ("alter_flags: %lu", alter_flags));
-    /* Check dropped indexes. */
-    for (idx_p= index_drop_buffer, idx_end_p= idx_p + index_drop_count;
-         idx_p < idx_end_p;
-         idx_p++)
-    {
-      key= table->key_info + *idx_p;
-      DBUG_PRINT("info", ("index dropped: '%s'", key->name));
-      if (key->flags & HA_NOSAME)
-      {
-        /* 
-           Unique key. Check for "PRIMARY". 
-           or if dropping last unique key
-        */
-        if ((uint) (key - table->key_info) == table->s->primary_key)
-        {
-          DBUG_PRINT("info", ("Dropping primary key"));
-          /* Primary key. */
-          needed_inplace_with_read_flags|= HA_INPLACE_DROP_PK_INDEX_NO_WRITE;
-          needed_inplace_flags|= HA_INPLACE_DROP_PK_INDEX_NO_READ_WRITE;
-          pk_changed++;
-          candidate_key_count--;
-        }
-        else
-        {
-          KEY_PART_INFO *part_end= key->key_part + key->key_parts;
-          bool is_candidate_key= true;
 
-          /* Non-primary unique key. */
-          needed_inplace_with_read_flags|=
-            HA_INPLACE_DROP_UNIQUE_INDEX_NO_WRITE;
-          needed_inplace_flags|= HA_INPLACE_DROP_UNIQUE_INDEX_NO_READ_WRITE;
-
-          /*
-            Check if all fields in key are declared
-            NOT NULL and adjust candidate_key_count
-          */
-          for (KEY_PART_INFO *key_part= key->key_part;
-               key_part < part_end;
-               key_part++)
-            is_candidate_key=
-              (is_candidate_key && 
-               (! table->field[key_part->fieldnr-1]->maybe_null()));
-          if (is_candidate_key)
-            candidate_key_count--;
-        }
-      }
-      else
-      {
-        /* Non-unique key. */
-        needed_inplace_with_read_flags|= HA_INPLACE_DROP_INDEX_NO_WRITE;
-        needed_inplace_flags|= HA_INPLACE_DROP_INDEX_NO_READ_WRITE;
-      }
-    }
-    no_pk= ((table->s->primary_key == MAX_KEY) ||
-            (needed_inplace_with_read_flags &
-             HA_INPLACE_DROP_PK_INDEX_NO_WRITE));
-    /* Check added indexes. */
-    for (idx_p= index_add_buffer, idx_end_p= idx_p + index_add_count;
-         idx_p < idx_end_p;
-         idx_p++)
-    {
-      key= key_info_buffer + *idx_p;
-      DBUG_PRINT("info", ("index added: '%s'", key->name));
-      if (key->flags & HA_NOSAME)
-      {
-        /* Unique key */
-
-        KEY_PART_INFO *part_end= key->key_part + key->key_parts;    
-        bool is_candidate_key= true;
-
-        /*
-          Check if all fields in key are declared
-          NOT NULL
-         */
-        for (KEY_PART_INFO *key_part= key->key_part;
-             key_part < part_end;
-             key_part++)
-          is_candidate_key=
-            (is_candidate_key && 
-             (! table->field[key_part->fieldnr]->maybe_null()));
+    alter_info->change_level= need_copy_table_res;
 
-        /*
-           Check for "PRIMARY"
-           or if adding first unique key
-           defined on non-nullable fields
-        */
-
-        if ((!my_strcasecmp(system_charset_info,
-                            key->name, primary_key_name)) ||
-            (no_pk && candidate_key_count == 0 && is_candidate_key))
-        {
-          DBUG_PRINT("info", ("Adding primary key"));
-          /* Primary key. */
-          needed_inplace_with_read_flags|= HA_INPLACE_ADD_PK_INDEX_NO_WRITE;
-          needed_inplace_flags|= HA_INPLACE_ADD_PK_INDEX_NO_READ_WRITE;
-          pk_changed++;
-          no_pk= false;
-        }
-        else
-        {
-          /* Non-primary unique key. */
-          needed_inplace_with_read_flags|= HA_INPLACE_ADD_UNIQUE_INDEX_NO_WRITE;
-          needed_inplace_flags|= HA_INPLACE_ADD_UNIQUE_INDEX_NO_READ_WRITE;
-        }
-      }
-      else
-      {
-        /* Non-unique key. */
-        needed_inplace_with_read_flags|= HA_INPLACE_ADD_INDEX_NO_WRITE;
-        needed_inplace_flags|= HA_INPLACE_ADD_INDEX_NO_READ_WRITE;
-      }
-    }
-
-    if ((candidate_key_count > 0) &&
-        (needed_inplace_with_read_flags & HA_INPLACE_DROP_PK_INDEX_NO_WRITE))
-    {
-      /*
-        Dropped primary key when there is some other unique 
-        not null key that should be converted to primary key
-      */
-      needed_inplace_with_read_flags|= HA_INPLACE_ADD_PK_INDEX_NO_WRITE;
-      needed_inplace_flags|= HA_INPLACE_ADD_PK_INDEX_NO_READ_WRITE;
-      pk_changed= 2;
-    }
+    DBUG_EXECUTE_IF("alter_table_only_index_change", {
+      if (alter_info->change_level != ALTER_TABLE_INDEX_CHANGED)
+        goto err; });
 
-    DBUG_PRINT("info",
-          ("needed_inplace_with_read_flags: 0x%lx, needed_inplace_flags: 0x%lx",
-           needed_inplace_with_read_flags, needed_inplace_flags));
     /*
-      In-place add/drop index is possible only if
-      the primary key is not added and dropped in the same statement.
-      Otherwise we have to recreate the table.
-      need_copy_table is no-zero at this place.
+      If there are index changes only, try to do them in-place. "Index
+      changes only" means also that the handler for the table does not
+      change. The table is open and locked. The handler can be accessed.
     */
-    if ( pk_changed < 2 )
-    {
-      if ((alter_flags & needed_inplace_with_read_flags) ==
-          needed_inplace_with_read_flags)
-      {
-        /* All required in-place flags to allow concurrent reads are present. */
-        need_copy_table= ALTER_TABLE_METADATA_ONLY;
-        need_lock_for_indexes= FALSE;
-      }
-      else if ((alter_flags & needed_inplace_flags) == needed_inplace_flags)
-      {
-        /* All required in-place flags are present. */
-        need_copy_table= ALTER_TABLE_METADATA_ONLY;
-      }
-    }
-    DBUG_PRINT("info", ("need_copy_table: %u  need_lock: %d",
-                        need_copy_table, need_lock_for_indexes));
+    if (alter_info->change_level == ALTER_TABLE_INDEX_CHANGED &&
+        can_alter_index_inplace(table, alter_info,
+                                index_drop_count, index_drop_buffer,
+                                index_add_count, index_add_buffer,
+                                candidate_key_count, key_info_buffer,
+                                &need_lock_for_indexes))
+      alter_info->change_level= ALTER_TABLE_METADATA_ONLY;
   }
 
-  /*
-    better have a negative test here, instead of positive, like
-    alter_info->flags & ALTER_ADD_COLUMN|ALTER_ADD_INDEX|...
-    so that ALTER TABLE won't break when somebody will add new flag
-  */
-  if (need_copy_table == ALTER_TABLE_METADATA_ONLY)
-    create_info->frm_only= 1;
-
 #ifdef WITH_PARTITION_STORAGE_ENGINE
   if (table_for_fast_alter_partition)
   {
@@ -6546,6 +6681,14 @@ bool mysql_alter_table(THD *thd,char *ne
   else
     create_info->data_file_name=create_info->index_file_name=0;
 
+  /*
+    better have a negative test here, instead of positive, like
+    alter_info->flags & ALTER_ADD_COLUMN|ALTER_ADD_INDEX|...
+    so that ALTER TABLE won't break when somebody will add new flag
+  */
+  if (alter_info->change_level == ALTER_TABLE_METADATA_ONLY)
+    create_info->frm_only= 1;
+
   DEBUG_SYNC(thd, "alter_table_before_create_table_no_lock");
   DBUG_EXECUTE_IF("sleep_before_create_table_no_lock",
                   my_sleep(100000););
@@ -6564,8 +6707,9 @@ bool mysql_alter_table(THD *thd,char *ne
     goto err;
 
   /* Open the table if we need to copy the data. */
-  DBUG_PRINT("info", ("need_copy_table: %u", need_copy_table));
-  if (need_copy_table != ALTER_TABLE_METADATA_ONLY)
+  DBUG_PRINT("info", ("Alter_info::change_level: %u",
+                      alter_info->change_level));
+  if (alter_info->change_level != ALTER_TABLE_METADATA_ONLY)
   {
     if (table->s->tmp_table)
     {
@@ -6604,7 +6748,8 @@ bool mysql_alter_table(THD *thd,char *ne
     We do not copy data for MERGE tables. Only the children have data.
     MERGE tables have HA_NO_COPY_ON_ALTER set.
   */
-  if (new_table && !(new_table->file->ha_table_flags() & HA_NO_COPY_ON_ALTER))
+  if (alter_info->change_level != ALTER_TABLE_METADATA_ONLY &&
+      !(new_table->file->ha_table_flags() & HA_NO_COPY_ON_ALTER))
   {
     /* We don't want update TIMESTAMP fields during ALTER TABLE. */
     new_table->timestamp_field_type= TIMESTAMP_NO_AUTO_SET;
@@ -6614,11 +6759,12 @@ bool mysql_alter_table(THD *thd,char *ne
         my_error(ER_LOCK_WAIT_TIMEOUT, MYF(0));
         goto err_new_table_cleanup;
       });
-    error= copy_data_between_tables(table, new_table,
-                                    alter_info->create_list, ignore,
-                                    order_num, order, &copied, &deleted,
-                                    alter_info->keys_onoff,
-                                    alter_info->error_if_not_empty);
+    if (copy_data_between_tables(table, new_table,
+                                 alter_info->create_list, ignore,
+                                 order_num, order, &copied, &deleted,
+                                 alter_info->keys_onoff,
+                                 alter_info->error_if_not_empty))
+      goto err_new_table_cleanup;
   }
   else
   {
@@ -6636,102 +6782,19 @@ bool mysql_alter_table(THD *thd,char *ne
     DEBUG_SYNC(thd, "alter_table_manage_keys");
     alter_table_manage_keys(table, table->file->indexes_are_disabled(),
                             alter_info->keys_onoff);
-    error= trans_commit_stmt(thd);
-    if (trans_commit_implicit(thd))
-      error= 1;
+    if (trans_commit_stmt(thd) || trans_commit_implicit(thd))
+      goto err_new_table_cleanup;
   }
   thd->count_cuted_fields= CHECK_FIELD_IGNORE;
 
   /* If we did not need to copy, we might still need to add/drop indexes. */
-  if (! new_table)
+  if (alter_info->change_level == ALTER_TABLE_METADATA_ONLY)
   {
-    uint          *key_numbers;
-    uint          *keyno_p;
-    KEY           *key_info;
-    KEY           *key;
-    uint          *idx_p;
-    uint          *idx_end_p;
-    KEY_PART_INFO *key_part;
-    KEY_PART_INFO *part_end;
-    DBUG_PRINT("info", ("No new_table, checking add/drop index"));
-
-    table->file->ha_prepare_for_alter();
-    if (index_add_count)
-    {
-      /* The add_index() method takes an array of KEY structs. */
-      key_info= (KEY*) thd->alloc(sizeof(KEY) * index_add_count);
-      key= key_info;
-      for (idx_p= index_add_buffer, idx_end_p= idx_p + index_add_count;
-           idx_p < idx_end_p;
-           idx_p++, key++)
-      {
-        /* Copy the KEY struct. */
-        *key= key_info_buffer[*idx_p];
-        /* Fix the key parts. */
-        part_end= key->key_part + key->key_parts;
-        for (key_part= key->key_part; key_part < part_end; key_part++)
-          key_part->field= table->field[key_part->fieldnr];
-      }
-      /* Add the indexes. */
-      if ((error= table->file->add_index(table, key_info, index_add_count)))
-      {
-        /*
-          Exchange the key_info for the error message. If we exchange
-          key number by key name in the message later, we need correct info.
-        */
-        KEY *save_key_info= table->key_info;
-        table->key_info= key_info;
-        table->file->print_error(error, MYF(0));
-        table->key_info= save_key_info;
-        goto err_new_table_cleanup;
-      }
-    }
-    /*end of if (index_add_count)*/
-
-    if (index_drop_count)
-    {
-      /* The prepare_drop_index() method takes an array of key numbers. */
-      key_numbers= (uint*) thd->alloc(sizeof(uint) * index_drop_count);
-      keyno_p= key_numbers;
-      /* Get the number of each key. */
-      for (idx_p= index_drop_buffer, idx_end_p= idx_p + index_drop_count;
-           idx_p < idx_end_p;
-           idx_p++, keyno_p++)
-        *keyno_p= *idx_p;
-      /*
-        Tell the handler to prepare for drop indexes.
-        This re-numbers the indexes to get rid of gaps.
-      */
-      if ((error= table->file->prepare_drop_index(table, key_numbers,
-                                                  index_drop_count)))
-      {
-        table->file->print_error(error, MYF(0));
-        goto err_new_table_cleanup;
-      }
-
-      /* Tell the handler to finally drop the indexes. */
-      if ((error= table->file->final_drop_index(table)))
-      {
-        table->file->print_error(error, MYF(0));
-        goto err_new_table_cleanup;
-      }
-    }
-    /*end of if (index_drop_count)*/
-
-    /*
-      The final .frm file is already created as a temporary file
-      and will be renamed to the original table name later.
-    */
-
-    /* Need to commit before a table is unlocked (NDB requirement). */
-    DBUG_PRINT("info", ("Committing before unlocking table"));
-    if (trans_commit_stmt(thd) || trans_commit_implicit(thd))
+    if (alter_indexes(thd, table, key_info_buffer,
+                      index_drop_count, index_drop_buffer,
+                      index_add_count, index_add_buffer))
       goto err_new_table_cleanup;
   }
-  /*end of if (! new_table) for add/drop index*/
-
-  if (error)
-    goto err_new_table_cleanup;
 
   if (table->s->tmp_table != NO_TMP_TABLE)
   {
@@ -6824,7 +6887,7 @@ bool mysql_alter_table(THD *thd,char *ne
     table is renamed and the SE is also changed, then an intermediate table
     is created and the additional call will not take place.
   */
-  if (need_copy_table == ALTER_TABLE_METADATA_ONLY)
+  if (alter_info->change_level == ALTER_TABLE_METADATA_ONLY)
   {
     DBUG_ASSERT(new_db_type == old_db_type);
     /* This type cannot happen in regular ALTER. */
@@ -6833,36 +6896,31 @@ bool mysql_alter_table(THD *thd,char *ne
   if (mysql_rename_table(old_db_type, db, table_name, db, old_name,
                          FN_TO_IS_TMP))
   {
-    error=1;
     (void) quick_rm_table(new_db_type, new_db, tmp_name, FN_IS_TMP);
+    /* This shouldn't happen. But let us play it safe. */
+    goto err_with_mdl;
   }
   else if (mysql_rename_table(new_db_type, new_db, tmp_name, new_db,
                               new_alias, FN_FROM_IS_TMP) ||
            ((new_name != table_name || new_db != db) && // we also do rename
-            (need_copy_table != ALTER_TABLE_METADATA_ONLY ||
+            (alter_info->change_level != ALTER_TABLE_METADATA_ONLY ||
              mysql_rename_table(save_old_db_type, db, table_name, new_db,
                                 new_alias, NO_FRM_RENAME)) &&
             Table_triggers_list::change_table_name(thd, db, alias, table_name,
                                                    new_db, new_alias)))
   {
     /* Try to get everything back. */
-    error=1;
     (void) quick_rm_table(new_db_type,new_db,new_alias, 0);
     (void) quick_rm_table(new_db_type, new_db, tmp_name, FN_IS_TMP);
     (void) mysql_rename_table(old_db_type, db, old_name, db, alias,
                             FN_FROM_IS_TMP);
-  }
-
-  if (! error)
-    (void) quick_rm_table(old_db_type, db, old_name, FN_IS_TMP);
-
-  if (error)
-  {
     /* This shouldn't happen. But let us play it safe. */
     goto err_with_mdl;
   }
 
-  if (need_copy_table == ALTER_TABLE_METADATA_ONLY)
+  (void) quick_rm_table(old_db_type, db, old_name, FN_IS_TMP);
+
+  if (alter_info->change_level == ALTER_TABLE_METADATA_ONLY)
   {
     /*
       Now we have to inform handler that new .FRM file is in place.


Attachment: [text/bzr-bundle] bzr/jon.hauglid@oracle.com-20110321164806-1vpc0sgpu568fntd.bundle
Thread
bzr commit into mysql-trunk branch (jon.hauglid:3304) WL#5534Jon Olav Hauglid21 Mar