From: Jon Olav Hauglid Date: December 20 2011 1:11pm Subject: bzr push into mysql-trunk-wl5534 branch (jon.hauglid:3455 to 3456) WL#5534 List-Archive: http://lists.mysql.com/commits/142211 Message-Id: <201112201311.pBKDBpCx017041@acsmt356.oracle.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 3456 Jon Olav Hauglid 2011-12-20 WL#5534 Online ALTER, Phase 1. Patch #69: Review changes: - Introduce Alter_table_ctx class, representing ALTER TABLE runtime context. - Move datetime_field / error_of_not_empty from Alter_info to Alter_table_ctx. - Move db/table_name/alias/new_db/new_name/new_alias to Alter_table_ctx. modified: sql/sql_alter.cc sql/sql_alter.h sql/sql_partition_admin.cc sql/sql_table.cc sql/sql_table.h 3455 Jon Olav Hauglid 2011-12-19 WL#5534 Online ALTER, Phase 1. Patch #67: Review changes: - Make IMPORT/DISCARD tablespace a separate Sql_cmd. - Remove related code from Alter_info and mysql_alter_table(). - Remove unneeded Alter_info parameter in Alter_table_prelocking_strategy. modified: sql/sql_alter.cc sql/sql_alter.h sql/sql_base.h sql/sql_partition_admin.cc sql/sql_table.cc sql/sql_table.h sql/sql_yacc.yy === modified file 'sql/sql_alter.cc' --- a/sql/sql_alter.cc 2011-12-19 14:38:42 +0000 +++ b/sql/sql_alter.cc 2011-12-20 12:49:28 +0000 @@ -29,8 +29,6 @@ Alter_info::Alter_info(const Alter_info keys_onoff(rhs.keys_onoff), partition_names(rhs.partition_names, mem_root), num_parts(rhs.num_parts), - datetime_field(rhs.datetime_field), - error_if_not_empty(rhs.error_if_not_empty), requested_algorithm(rhs.requested_algorithm), requested_lock(rhs.requested_lock) { @@ -83,6 +81,86 @@ bool Alter_info::set_requested_lock(cons } +Alter_table_ctx::Alter_table_ctx() + : datetime_field(NULL), error_if_not_empty(false), + tables_opened(0), + db(NULL), table_name(NULL), alias(NULL), + new_db(NULL), new_name(NULL), new_alias(NULL) +{ +} + + +Alter_table_ctx::Alter_table_ctx(THD *thd, TABLE_LIST *table_list, + uint tables_opened_arg, + char *new_db_arg, char *new_name_arg) + : datetime_field(NULL), error_if_not_empty(false), + tables_opened(tables_opened_arg), + new_db(new_db_arg), new_name(new_name_arg) +{ + /* + Assign members db, table_name, new_db and new_name + to simplify further comparisions: we want to see if it's a RENAME + later just by comparing the pointers, avoiding the need for strcmp. + */ + db= table_list->db; + table_name= table_list->table_name; + alias= (lower_case_table_names == 2) ? table_list->alias : table_name; + + if (!new_db || !my_strcasecmp(table_alias_charset, new_db, db)) + new_db= db; + + if (new_name) + { + DBUG_PRINT("info", ("new_db.new_name: '%s'.'%s'", new_db, new_name)); + + if (lower_case_table_names == 1) // Convert new_name/new_alias to lower case + { + my_casedn_str(files_charset_info, new_name); + new_alias= new_name; + } + else if (lower_case_table_names == 2) // Convert new_name to lower case + { + strmov(new_alias= new_alias_buff, new_name); + my_casedn_str(files_charset_info, new_name); + } + else + new_alias= new_name; // LCTN=0 => case sensitive + case preserving + + if (!is_database_changed() && + !my_strcasecmp(table_alias_charset, new_name, table_name)) + { + /* + Source and destination table names are equal: + make is_table_renamed() more efficient. + */ + new_alias= table_name; + new_name= table_name; + } + } + else + { + new_alias= alias; + new_name= table_name; + } + + my_snprintf(tmp_name, sizeof(tmp_name), "%s-%lx_%lx", tmp_file_prefix, + current_pid, thd->thread_id); + /* Safety fix for InnoDB */ + if (lower_case_table_names) + my_casedn_str(files_charset_info, tmp_name); + + build_table_filename(path, sizeof(path) - 1, db, table_name, "", 0); + + build_table_filename(new_path, sizeof(new_path) - 1, new_db, new_name, "", 0); + + build_table_filename(new_filename, sizeof(new_filename) - 1, + new_db, new_name, reg_ext, 0); + + build_table_filename(tmp_path, sizeof(tmp_path) - 1, new_db, tmp_name, "", + FN_IS_TMP); +} + + bool Sql_cmd_alter_table::execute(THD *thd) { LEX *lex= thd->lex; === modified file 'sql/sql_alter.h' --- a/sql/sql_alter.h 2011-12-19 14:38:42 +0000 +++ b/sql/sql_alter.h 2011-12-20 12:49:28 +0000 @@ -162,17 +162,26 @@ public: ALTER_TABLE_LOCK_EXCLUSIVE }; + + // Columns and keys to be dropped. List drop_list; + // Columns for ALTER_COLUMN_CHANGE_DEFAULT. List alter_list; + // List of keys, used by both CREATE and ALTER TABLE. List key_list; + // List of columns, used by both CREATE and ALTER TABLE. List create_list; + // Type of ALTER TABLE operation. uint flags; + // Enable or disable keys. enum_enable_or_disable keys_onoff; + // List of partitions. List partition_names; + // Number of partitions. uint num_parts; - Create_field *datetime_field; - bool error_if_not_empty; + // Type of ALTER TABLE algorithm. enum_alter_table_algorithm requested_algorithm; + // Type of ALTER TABLE lock. enum_alter_table_lock requested_lock; @@ -180,8 +189,6 @@ public: flags(0), keys_onoff(LEAVE_AS_IS), num_parts(0), - datetime_field(NULL), - error_if_not_empty(FALSE), requested_algorithm(ALTER_TABLE_ALGORITHM_DEFAULT), requested_lock(ALTER_TABLE_LOCK_DEFAULT) {} @@ -196,8 +203,6 @@ public: keys_onoff= LEAVE_AS_IS; num_parts= 0; partition_names.empty(); - datetime_field= 0; - error_if_not_empty= FALSE; requested_algorithm= ALTER_TABLE_ALGORITHM_DEFAULT; requested_lock= ALTER_TABLE_LOCK_DEFAULT; } @@ -251,6 +256,75 @@ private: }; +/** Runtime context for ALTER TABLE. */ +class Alter_table_ctx +{ +public: + Alter_table_ctx(); + + Alter_table_ctx(THD *thd, TABLE_LIST *table_list, uint tables_opened_arg, + char *new_db_arg, char *new_name_arg); + + /** + @return true if the table is moved to another database, false otherwise. + */ + bool is_database_changed() const + { return (new_db != db); }; + + /** + @return true if the table is renamed, false otherwise. + */ + bool is_table_renamed() const + { return (is_database_changed() || new_name != table_name); }; + + /** + @return filename (including .frm) for the new table. + */ + const char *get_new_filename() const + { return new_filename; } + + /** + @return path to the original table. + */ + const char *get_path() const + { return path; } + + /** + @return path to the new table. + */ + const char *get_new_path() const + { return new_path; } + + /** + @return path to the temporary table created during ALTER TABLE. + */ + const char *get_tmp_path() const + { return tmp_path; } + +public: + Create_field *datetime_field; + bool error_if_not_empty; + uint tables_opened; + char *db; + char *table_name; + char *alias; + char *new_db; + char *new_name; + char *new_alias; + char tmp_name[80]; + +private: + char new_filename[FN_REFLEN + 1]; + char new_alias_buff[FN_REFLEN + 1]; + char path[FN_REFLEN + 1]; + char new_path[FN_REFLEN + 1]; + char tmp_path[FN_REFLEN + 1]; + + Alter_table_ctx &operator=(const Alter_table_ctx &rhs); // not implemented + Alter_table_ctx(const Alter_table_ctx &rhs); // not implemented +}; + + /** Sql_cmd_common_alter_table represents the common properties of the ALTER TABLE statements. === modified file 'sql/sql_partition_admin.cc' --- a/sql/sql_partition_admin.cc 2011-12-19 14:38:42 +0000 +++ b/sql/sql_partition_admin.cc 2011-12-20 12:49:28 +0000 @@ -175,6 +175,7 @@ static bool compare_table_with_partition { HA_CREATE_INFO table_create_info, part_create_info; Alter_info part_alter_info; + Alter_table_ctx part_alter_ctx; // Not used DBUG_ENTER("compare_table_with_partition"); bool metadata_equal= false; @@ -188,7 +189,7 @@ static bool compare_table_with_partition part_table->use_all_columns(); table->use_all_columns(); if (mysql_prepare_alter_table(thd, part_table, &part_create_info, - &part_alter_info)) + &part_alter_info, &part_alter_ctx)) { my_error(ER_TABLES_DIFFERENT_METADATA, MYF(0)); DBUG_RETURN(TRUE); === modified file 'sql/sql_table.cc' --- a/sql/sql_table.cc 2011-12-19 14:38:42 +0000 +++ b/sql/sql_table.cc 2011-12-20 12:49:28 +0000 @@ -4738,6 +4738,7 @@ bool mysql_create_like_table(THD* thd, T { HA_CREATE_INFO local_create_info; Alter_info local_alter_info; + Alter_table_ctx local_alter_ctx; // Not used bool res= TRUE; bool is_trans= FALSE; uint not_used; @@ -4766,7 +4767,7 @@ bool mysql_create_like_table(THD* thd, T local_create_info.db_type= src_table->table->s->db_type(); local_create_info.row_type= src_table->table->s->row_type; if (mysql_prepare_alter_table(thd, src_table->table, &local_create_info, - &local_alter_info)) + &local_alter_info, &local_alter_ctx)) goto err; #ifdef WITH_PARTITION_STORAGE_ENGINE /* Partition info is not handled by mysql_prepare_alter_table() call. */ @@ -5755,12 +5756,7 @@ static bool is_inplace_alter_impossible( @param ha_alter_info 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 new_db Target database name. - @param new_name Target table name. - @param new_alias Target table name alias. - @param alias Old table name alias - @param tmp_name Temporary table name. - @param tables_opened Tables opened by open_tables(). + @param alter_ctx ALTER TABLE runtime context. @retval true Error @retval false Success @@ -5787,18 +5783,11 @@ static bool mysql_inplace_alter_table(TH Alter_inplace_info *ha_alter_info, enum_alter_inplace_result inplace_supported, MDL_request *target_mdl_request, - const char *new_db, - const char *new_name, - const char *new_alias, - const char *alias, - const char *tmp_name, - uint tables_opened) + Alter_table_ctx *alter_ctx) { Open_table_context ot_ctx(thd, MYSQL_OPEN_REOPEN); handlerton *db_type= table->s->db_type(); MDL_ticket *mdl_ticket= table->mdl_ticket; - const char *db= table_list->db; - const char *table_name= table_list->table_name; DBUG_ENTER("mysql_inplace_alter_table"); @@ -5832,7 +5821,7 @@ static bool mysql_inplace_alter_table(TH } // It's now safe to take the table level lock. - if (lock_tables(thd, table_list, tables_opened, 0)) + if (lock_tables(thd, table_list, alter_ctx->tables_opened, 0)) goto cleanup; DEBUG_SYNC(thd, "alter_table_inplace_after_lock_upgrade"); @@ -5895,19 +5884,20 @@ static bool mysql_inplace_alter_table(TH goto rollback; } - close_all_tables_for_name(thd, table->s, - (new_name != table_name || new_db != db)); + close_all_tables_for_name(thd, table->s, alter_ctx->is_table_renamed()); close_temporary_table(thd, altered_table, true, false); /* Replace the old .FRM with the new .FRM, but keep the old name for now. Rename to the new name (if needed) will be handled separately below. */ - if (mysql_rename_table(db_type, new_db, tmp_name, db, table_name, + if (mysql_rename_table(db_type, alter_ctx->new_db, alter_ctx->tmp_name, + alter_ctx->db, alter_ctx->table_name, FN_FROM_IS_TMP | FRM_ONLY)) { // Since changes were done in-place, we can't revert them. - (void) quick_rm_table(thd, db_type, new_db, tmp_name, + (void) quick_rm_table(thd, db_type, + alter_ctx->new_db, alter_ctx->tmp_name, FN_IS_TMP | NO_HA_TABLE); DBUG_RETURN(true); } @@ -5921,9 +5911,8 @@ static bool mysql_inplace_alter_table(TH Tell the handler that the changed frm is on disk and table has been re-opened */ - char path[FN_REFLEN + 1]; - build_table_filename(path, sizeof(path) - 1, db, table_name, "", 0); - table_list->table->file->ha_create_handler_files(path, NULL, CHF_INDEX_FLAG, + table_list->table->file->ha_create_handler_files(alter_ctx->get_path(), + NULL, CHF_INDEX_FLAG, create_info); /* @@ -5935,12 +5924,14 @@ static bool mysql_inplace_alter_table(TH table_list->table= NULL; // Rename altered table if requested. - if (new_name != table_name || new_db != db) + if (alter_ctx->is_table_renamed()) { // Remove TABLE and TABLE_SHARE for old name from TDC. - tdc_remove_table(thd, TDC_RT_REMOVE_ALL, db, table_name, false); + tdc_remove_table(thd, TDC_RT_REMOVE_ALL, + alter_ctx->db, alter_ctx->table_name, false); - if (mysql_rename_table(db_type, db, table_name, new_db, new_alias, 0)) + if (mysql_rename_table(db_type, alter_ctx->db, alter_ctx->table_name, + alter_ctx->new_db, alter_ctx->new_alias, 0)) { /* If the rename fails we will still have a working table @@ -5948,14 +5939,20 @@ static bool mysql_inplace_alter_table(TH */ DBUG_RETURN(true); } - if (Table_triggers_list::change_table_name(thd, db, alias, table_name, - new_db, new_alias)) + if (Table_triggers_list::change_table_name(thd, + alter_ctx->db, + alter_ctx->alias, + alter_ctx->table_name, + alter_ctx->new_db, + alter_ctx->new_alias)) { /* If the rename of trigger files fails, try to rename the table back so we at least have matching table and trigger files. */ - (void) mysql_rename_table(db_type, new_db, new_alias, db, alias, 0); + (void) mysql_rename_table(db_type, + alter_ctx->new_db, alter_ctx->new_alias, + alter_ctx->db, alter_ctx->alias, 0); DBUG_RETURN(true); } } @@ -5970,8 +5967,8 @@ static bool mysql_inplace_alter_table(TH cleanup: close_temporary_table(thd, altered_table, true, false); // Delete temporary .frm/.par - (void) quick_rm_table(thd, create_info->db_type, new_db, tmp_name, - FN_IS_TMP | NO_HA_TABLE); + (void) quick_rm_table(thd, create_info->db_type, alter_ctx->new_db, + alter_ctx->tmp_name, FN_IS_TMP | NO_HA_TABLE); DBUG_RETURN(true); } @@ -6033,6 +6030,7 @@ blob_length_by_type(enum_field_types typ But since ALTER might end-up doing CREATE, this distinction is gone and we just carry around two structures. + @param[in,out] alter_ctx Runtime context for ALTER TABLE. @return Fills various create_info members based on information retrieved @@ -6048,7 +6046,8 @@ blob_length_by_type(enum_field_types typ bool mysql_prepare_alter_table(THD *thd, TABLE *table, HA_CREATE_INFO *create_info, - Alter_info *alter_info) + Alter_info *alter_info, + Alter_table_ctx *alter_ctx) { /* New column definitions are added here */ List new_create_list; @@ -6206,12 +6205,12 @@ mysql_prepare_alter_table(THD *thd, TABL def->sql_type == MYSQL_TYPE_NEWDATE || def->sql_type == MYSQL_TYPE_DATETIME || def->sql_type == MYSQL_TYPE_DATETIME2) && - !alter_info->datetime_field && + !alter_ctx->datetime_field && !(~def->flags & (NO_DEFAULT_VALUE_FLAG | NOT_NULL_FLAG)) && thd->variables.sql_mode & MODE_NO_ZERO_DATE) { - alter_info->datetime_field= def; - alter_info->error_if_not_empty= TRUE; + alter_ctx->datetime_field= def; + alter_ctx->error_if_not_empty= true; } if (!def->after) new_create_list.push_back(def); @@ -6454,13 +6453,7 @@ err: @param thd Thread handler @param table_list TABLE_LIST for the table to change @param keys_onoff ENABLE or DISABLE KEYS? - @param new_db New database 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 tables_opened Tables opened by open_tables() + @param alter_ctx ALTER TABLE runtime context. @return Operation status @retval 0 Success @@ -6470,13 +6463,7 @@ err: static int simple_rename_or_index_change(THD *thd, TABLE_LIST *table_list, Alter_info::enum_enable_or_disable keys_onoff, - const char *new_db, - const char *new_name, - const char *new_alias, - const char *db, - const char *table_name, - const char *alias, - uint tables_opened) + Alter_table_ctx *alter_ctx) { TABLE *table= table_list->table; MDL_ticket *mdl_ticket= table->mdl_ticket; @@ -6489,7 +6476,7 @@ simple_rename_or_index_change(THD *thd, DBUG_RETURN(-1); // It's now safe to take the table level lock. - if (lock_tables(thd, table_list, tables_opened, 0)) + if (lock_tables(thd, table_list, alter_ctx->tables_opened, 0)) DBUG_RETURN(-1); if (keys_onoff == Alter_info::ENABLE) @@ -6515,7 +6502,7 @@ simple_rename_or_index_change(THD *thd, } } - if (!error && (new_name != table_name || new_db != db)) + if (!error && alter_ctx->is_table_renamed()) { THD_STAGE_INFO(thd, stage_rename); handlerton *old_db_type= table->s->db_type(); @@ -6531,15 +6518,19 @@ simple_rename_or_index_change(THD *thd, DBUG_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)) + if (mysql_rename_table(old_db_type, alter_ctx->db, alter_ctx->table_name, + alter_ctx->new_db, alter_ctx->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); + else if (Table_triggers_list::change_table_name(thd, + alter_ctx->db, + alter_ctx->alias, + alter_ctx->table_name, + alter_ctx->new_db, + alter_ctx->new_alias)) + { + (void) mysql_rename_table(old_db_type, + alter_ctx->new_db, alter_ctx->new_alias, + alter_ctx->db, alter_ctx->table_name, 0); error= -1; } } @@ -6561,7 +6552,7 @@ simple_rename_or_index_change(THD *thd, statement. Otherwise we can rely on them being released along with the implicit commit. */ - if (new_name != table_name || new_db != db) + if (alter_ctx->is_table_renamed()) thd->mdl_context.release_all_locks_for_name(mdl_ticket); else mdl_ticket->downgrade_lock(MDL_SHARED_NO_READ_WRITE); @@ -6696,105 +6687,63 @@ 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. - */ - char *table_name= table_list->table_name; - char *alias= (lower_case_table_names == 2) ? table_list->alias : table_name; - char *db= table_list->db; - if (!new_db || !my_strcasecmp(table_alias_charset, new_db, db)) - new_db= db; + Alter_table_ctx alter_ctx(thd, table_list, tables_opened, new_db, new_name); /* Add old and new (if any) databases to the list of accessed databases for this statement. Needed for MTS. */ - thd->add_to_binlog_accessed_dbs(db); - if (new_db != db) - thd->add_to_binlog_accessed_dbs(new_db); - - char new_name_buff[FN_REFLEN + 1]; - char new_alias_buff[FN_REFLEN]; - char *new_alias; + thd->add_to_binlog_accessed_dbs(alter_ctx.db); + if (alter_ctx.is_database_changed()) + thd->add_to_binlog_accessed_dbs(alter_ctx.new_db); + MDL_request target_mdl_request; /* Check that we are not trying to rename to an existing table */ - if (new_name) + if (alter_ctx.is_table_renamed()) { - DBUG_PRINT("info", ("new_db.new_name: '%s'.'%s'", new_db, new_name)); - strmov(new_name_buff,new_name); - strmov(new_alias= new_alias_buff, new_name); - if (lower_case_table_names) + if (table->s->tmp_table != NO_TMP_TABLE) { - if (lower_case_table_names != 2) + if (find_temporary_table(thd, alter_ctx.new_db, alter_ctx.new_name)) { - my_casedn_str(files_charset_info, new_name_buff); - new_alias= new_name; // Create lower case table name + my_error(ER_TABLE_EXISTS_ERROR, MYF(0), alter_ctx.new_alias); + DBUG_RETURN(true); } - my_casedn_str(files_charset_info, new_name); } - if (new_db == db && - !my_strcasecmp(table_alias_charset, new_name_buff, table_name)) + else { + target_mdl_request.init(MDL_key::TABLE, + alter_ctx.new_db, alter_ctx.new_name, + MDL_EXCLUSIVE, MDL_TRANSACTION); /* - Source and destination table names are equal: make later check - easier. + Global intention exclusive lock must have been already acquired when + table to be altered was open, so there is no need to do it here. */ - new_alias= new_name= table_name; - } - else - { - if (table->s->tmp_table != NO_TMP_TABLE) + DBUG_ASSERT(thd->mdl_context.is_lock_owner(MDL_key::GLOBAL, + "", "", + MDL_INTENTION_EXCLUSIVE)); + + if (thd->mdl_context.try_acquire_lock(&target_mdl_request)) + DBUG_RETURN(true); + if (target_mdl_request.ticket == NULL) { - if (find_temporary_table(thd,new_db,new_name_buff)) - { - my_error(ER_TABLE_EXISTS_ERROR, MYF(0), new_name_buff); - DBUG_RETURN(true); - } + /* Table exists and is locked by some thread. */ + my_error(ER_TABLE_EXISTS_ERROR, MYF(0), alter_ctx.new_alias); + DBUG_RETURN(true); } - else + DEBUG_SYNC(thd, "locked_table_name"); + /* + Table maybe does not exist, but we got an exclusive lock + on the name, now we can safely try to find out for sure. + */ + if (!access(alter_ctx.get_new_filename(), F_OK)) { - target_mdl_request.init(MDL_key::TABLE, new_db, new_name, - MDL_EXCLUSIVE, MDL_TRANSACTION); - /* - Global intention exclusive lock must have been already acquired when - table to be altered was open, so there is no need to do it here. - */ - DBUG_ASSERT(thd->mdl_context.is_lock_owner(MDL_key::GLOBAL, - "", "", - MDL_INTENTION_EXCLUSIVE)); - - if (thd->mdl_context.try_acquire_lock(&target_mdl_request)) - DBUG_RETURN(true); - if (target_mdl_request.ticket == NULL) - { - /* Table exists and is locked by some thread. */ - my_error(ER_TABLE_EXISTS_ERROR, MYF(0), new_alias); - DBUG_RETURN(true); - } - DEBUG_SYNC(thd, "locked_table_name"); - /* - Table maybe does not exist, but we got an exclusive lock - on the name, now we can safely try to find out for sure. - */ - build_table_filename(new_name_buff, sizeof(new_name_buff) - 1, - new_db, new_name_buff, reg_ext, 0); - if (!access(new_name_buff, F_OK)) - { - /* Table will be closed in do_command() */ - my_error(ER_TABLE_EXISTS_ERROR, MYF(0), new_alias); - DBUG_RETURN(true); - } + /* Table will be closed in do_command() */ + my_error(ER_TABLE_EXISTS_ERROR, MYF(0), alter_ctx.new_alias); + DBUG_RETURN(true); } } } - else - { - new_alias= (lower_case_table_names == 2) ? alias : table_name; - new_name= table_name; - } if (!create_info->db_type) { @@ -6816,7 +6765,7 @@ bool mysql_alter_table(THD *thd,char *ne create_info->db_type= table->s->db_type(); } - if (check_engine(thd, new_name, create_info)) + if (check_engine(thd, alter_ctx.new_name, create_info)) DBUG_RETURN(true); if ((create_info->db_type != table->s->db_type() || @@ -6870,9 +6819,7 @@ bool mysql_alter_table(THD *thd,char *ne } DBUG_RETURN(simple_rename_or_index_change(thd, table_list, alter_info->keys_onoff, - new_db, new_name, new_alias, - db, table_name, alias, - tables_opened)); + &alter_ctx)); } /* We have to do full alter table. */ @@ -6881,12 +6828,9 @@ bool mysql_alter_table(THD *thd,char *ne bool partition_changed= false; TABLE *table_for_fast_alter_partition= NULL; { - char path[FN_REFLEN + 1]; - build_table_filename(path, sizeof(path) - 1, db, table_name, "", 0); - if (prep_alter_part_table(thd, table, alter_info, create_info, - &partition_changed, - db, table_name, path, + &partition_changed, alter_ctx.db, + alter_ctx.table_name, alter_ctx.get_path(), &table_for_fast_alter_partition)) { /* @@ -6902,7 +6846,8 @@ bool mysql_alter_table(THD *thd,char *ne } #endif - if (mysql_prepare_alter_table(thd, table, create_info, alter_info)) + if (mysql_prepare_alter_table(thd, table, create_info, alter_info, + &alter_ctx)) { #ifdef WITH_PARTITION_STORAGE_ENGINE if (table_for_fast_alter_partition) @@ -6911,7 +6856,7 @@ bool mysql_alter_table(THD *thd,char *ne DBUG_RETURN(true); } - set_table_default_charset(thd, create_info, db); + set_table_default_charset(thd, create_info, alter_ctx.db); #ifdef WITH_PARTITION_STORAGE_ENGINE if (table_for_fast_alter_partition) @@ -6933,7 +6878,7 @@ bool mysql_alter_table(THD *thd,char *ne // In-place execution of ALTER TABLE for partitioning. DBUG_RETURN(fast_alter_partition_table(thd, table, alter_info, create_info, table_list, - db, table_name, + alter_ctx.db, alter_ctx.table_name, table_for_fast_alter_partition)); } #endif @@ -6968,13 +6913,6 @@ bool mysql_alter_table(THD *thd,char *ne TABLE *new_table= NULL; ha_rows copied=0,deleted=0; - char tmp_name[80]; - my_snprintf(tmp_name, sizeof(tmp_name), "%s-%lx_%lx", tmp_file_prefix, - current_pid, thd->thread_id); - /* Safety fix for InnoDB */ - if (lower_case_table_names) - my_casedn_str(files_charset_info, tmp_name); - /* Handling of symlinked tables: If no rename: @@ -7001,12 +6939,12 @@ bool mysql_alter_table(THD *thd,char *ne */ char index_file[FN_REFLEN], data_file[FN_REFLEN]; - if (new_db == db) + if (!alter_ctx.is_database_changed()) { if (create_info->index_file_name) { /* Fix index_file_name to have 'tmp_name' as basename */ - strmov(index_file, tmp_name); + strmov(index_file, alter_ctx.tmp_name); create_info->index_file_name=fn_same(index_file, create_info->index_file_name, 1); @@ -7014,7 +6952,7 @@ bool mysql_alter_table(THD *thd,char *ne if (create_info->data_file_name) { /* Fix data_file_name to have 'tmp_name' as basename */ - strmov(data_file, tmp_name); + strmov(data_file, alter_ctx.tmp_name); create_info->data_file_name=fn_same(data_file, create_info->data_file_name, 1); @@ -7054,9 +6992,8 @@ bool mysql_alter_table(THD *thd,char *ne Alter_info save_alter_info(*alter_info, thd->mem_root); tmp_disable_binlog(thd); - error= mysql_create_table_no_lock(thd, new_db, tmp_name, - create_info, - alter_info, + error= mysql_create_table_no_lock(thd, alter_ctx.new_db, alter_ctx.tmp_name, + create_info, alter_info, true, 0, NULL, true); reenable_binlog(thd); @@ -7068,7 +7005,6 @@ bool mysql_alter_table(THD *thd,char *ne if (alter_info->requested_algorithm != Alter_info::ALTER_TABLE_ALGORITHM_COPY) { - char path[FN_REFLEN + 1]; Alter_inplace_info ha_alter_info(ignore); TABLE *altered_table= NULL; bool use_inplace= true; @@ -7082,10 +7018,9 @@ bool mysql_alter_table(THD *thd,char *ne // We assume that the table is non-temporary. DBUG_ASSERT(!table->s->tmp_table); - build_table_filename(path, sizeof(path) - 1, new_db, tmp_name, "", - FN_IS_TMP); - - if (!(altered_table= open_table_uncached(thd, path, new_db, tmp_name, + if (!(altered_table= open_table_uncached(thd, alter_ctx.get_tmp_path(), + alter_ctx.new_db, + alter_ctx.tmp_name, true, false))) goto err_new_table_cleanup; @@ -7158,8 +7093,7 @@ bool mysql_alter_table(THD *thd,char *ne create_info, alter_info, &ha_alter_info, inplace_supported, &target_mdl_request, - new_db, new_name, new_alias, alias, - tmp_name, tables_opened)) + &alter_ctx)) { DBUG_RETURN(true); } @@ -7201,7 +7135,7 @@ bool mysql_alter_table(THD *thd,char *ne } // It's now safe to take the table level lock. - if (lock_tables(thd, table_list, tables_opened, 0)) + if (lock_tables(thd, table_list, alter_ctx.tables_opened, 0)) goto err_new_table_cleanup; { @@ -7217,12 +7151,14 @@ bool mysql_alter_table(THD *thd,char *ne path_length= build_tmptable_filename(thd, path, sizeof(path) - 1); } else - path_length= build_table_filename(path, sizeof(path) - 1, new_db, - tmp_name, reg_ext, FN_IS_TMP); + path_length= build_table_filename(path, sizeof(path) - 1, + alter_ctx.new_db, alter_ctx.tmp_name, + reg_ext, FN_IS_TMP); path[path_length - reg_ext_length]= '\0'; // Remove .frm extension - if (ha_create_table(thd, path, new_db, tmp_name, create_info, false)) + if (ha_create_table(thd, path, alter_ctx.new_db, alter_ctx.tmp_name, + create_info, false)) goto err_new_table_cleanup; /* Mark that we have created table in storage engine. */ @@ -7230,7 +7166,8 @@ bool mysql_alter_table(THD *thd,char *ne if (create_info->options & HA_LEX_CREATE_TMP_TABLE) { - if (!open_table_uncached(thd, path, new_db, tmp_name, true, true)) + if (!open_table_uncached(thd, path, alter_ctx.new_db, alter_ctx.tmp_name, + true, true)) goto err_new_table_cleanup; } } @@ -7240,21 +7177,20 @@ bool mysql_alter_table(THD *thd,char *ne if (table->s->tmp_table != NO_TMP_TABLE) { TABLE_LIST tbl; - tbl.init_one_table(new_db, strlen(new_db), - tmp_name, strlen(tmp_name), - tmp_name, TL_READ_NO_INSERT); + tbl.init_one_table(alter_ctx.new_db, strlen(alter_ctx.new_db), + alter_ctx.tmp_name, strlen(alter_ctx.tmp_name), + alter_ctx.tmp_name, TL_READ_NO_INSERT); /* Table is in thd->temporary_tables */ (void) open_temporary_table(thd, &tbl); new_table= tbl.table; } else { - char path[FN_REFLEN + 1]; /* table is a normal table: Create temporary table in same directory */ - build_table_filename(path, sizeof(path) - 1, new_db, tmp_name, "", - FN_IS_TMP); /* Open our intermediate table. */ - new_table= open_table_uncached(thd, path, new_db, tmp_name, true, true); + new_table= open_table_uncached(thd, alter_ctx.get_tmp_path(), + alter_ctx.new_db, alter_ctx.tmp_name, + true, true); } if (!new_table) goto err_new_table_cleanup; @@ -7285,7 +7221,7 @@ bool mysql_alter_table(THD *thd,char *ne alter_info->create_list, ignore, order_num, order, &copied, &deleted, alter_info->keys_onoff, - alter_info->error_if_not_empty)) + alter_ctx.error_if_not_empty)) goto err_new_table_cleanup; } else @@ -7327,7 +7263,8 @@ bool mysql_alter_table(THD *thd,char *ne /* Remove link to old table and rename the new one */ close_temporary_table(thd, table, true, true); /* Should pass the 'new_name' as we store table name in the cache */ - if (rename_temporary_table(thd, new_table, new_db, new_name)) + if (rename_temporary_table(thd, new_table, + alter_ctx.new_db, alter_ctx.new_name)) goto err_new_table_cleanup; /* We don't replicate alter table statement on temporary tables */ if (!thd->is_current_stmt_binlog_format_row() && @@ -7368,8 +7305,7 @@ bool mysql_alter_table(THD *thd,char *ne if (wait_while_table_is_used(thd, table, HA_EXTRA_PREPARE_FOR_RENAME)) goto err_new_table_cleanup; - close_all_tables_for_name(thd, table->s, - new_name != table_name || new_db != db); + close_all_tables_for_name(thd, table->s, alter_ctx.is_table_renamed()); table_list->table= table= NULL; /* Safety */ /* @@ -7381,41 +7317,49 @@ bool mysql_alter_table(THD *thd,char *ne current_pid, thd->thread_id); if (lower_case_table_names) my_casedn_str(files_charset_info, backup_name); - if (mysql_rename_table(old_db_type, db, table_name, db, backup_name, - FN_TO_IS_TMP)) + if (mysql_rename_table(old_db_type, alter_ctx.db, alter_ctx.table_name, + alter_ctx.db, backup_name, FN_TO_IS_TMP)) { // Rename to temporary name failed, delete the new table, abort ALTER. - (void) quick_rm_table(thd, new_db_type, new_db, tmp_name, FN_IS_TMP); + (void) quick_rm_table(thd, new_db_type, alter_ctx.new_db, + alter_ctx.tmp_name, FN_IS_TMP); goto err_with_mdl; } // Rename the new table to the correct name. - if (mysql_rename_table(new_db_type, new_db, tmp_name, new_db, new_alias, + if (mysql_rename_table(new_db_type, alter_ctx.new_db, alter_ctx.tmp_name, + alter_ctx.new_db, alter_ctx.new_alias, FN_FROM_IS_TMP)) { // Rename failed, delete the temporary table. - (void) quick_rm_table(thd, new_db_type, new_db, tmp_name, FN_IS_TMP); + (void) quick_rm_table(thd, new_db_type, alter_ctx.new_db, + alter_ctx.tmp_name, FN_IS_TMP); // Restore the backup of the original table to the old name. - (void) mysql_rename_table(old_db_type, db, backup_name, db, alias, - FN_FROM_IS_TMP); + (void) mysql_rename_table(old_db_type, alter_ctx.db, backup_name, + alter_ctx.db, alter_ctx.alias, FN_FROM_IS_TMP); goto err_with_mdl; } // Check if we renamed the table and if so update trigger files. - if ((new_name != table_name || new_db != db) && - Table_triggers_list::change_table_name(thd, db, alias, table_name, - new_db, new_alias)) + if (alter_ctx.is_table_renamed() && + Table_triggers_list::change_table_name(thd, + alter_ctx.db, + alter_ctx.alias, + alter_ctx.table_name, + alter_ctx.new_db, + alter_ctx.new_alias)) { // Rename succeeded, delete the new table. - (void) quick_rm_table(thd, new_db_type, new_db, new_alias, 0); + (void) quick_rm_table(thd, new_db_type, + alter_ctx.new_db, alter_ctx.new_alias, 0); // Restore the backup of the original table to the old name. - (void) mysql_rename_table(old_db_type, db, backup_name, db, alias, - FN_FROM_IS_TMP); + (void) mysql_rename_table(old_db_type, alter_ctx.db, backup_name, + alter_ctx.db, alter_ctx.alias, FN_FROM_IS_TMP); goto err_with_mdl; } // ALTER TABLE succeeded, delete the backup of the old table. - if (quick_rm_table(thd, old_db_type, db, backup_name, FN_IS_TMP)) + if (quick_rm_table(thd, old_db_type, alter_ctx.db, backup_name, FN_IS_TMP)) { /* The fact that deletion of the backup failed is not critical @@ -7452,10 +7396,10 @@ end_inplace: have to open the new table. If not, we get a problem on server shutdown. But we do not need to attach MERGE children. */ - char path[FN_REFLEN]; TABLE *t_table; - build_table_filename(path, sizeof(path) - 1, new_db, new_name, "", 0); - t_table= open_table_uncached(thd, path, new_db, new_name, false, true); + t_table= open_table_uncached(thd, alter_ctx.get_new_path(), + alter_ctx.new_db, alter_ctx.new_name, + false, true); if (t_table) { intern_close_table(t_table); @@ -7463,7 +7407,7 @@ end_inplace: } else sql_print_warning("Could not open table %s.%s after rename\n", - new_db,table_name); + alter_ctx.new_db, alter_ctx.table_name); ha_flush_logs(old_db_type); } table_list->table= NULL; // For query cache @@ -7472,17 +7416,18 @@ end_inplace: if (thd->locked_tables_mode == LTM_LOCK_TABLES || thd->locked_tables_mode == LTM_PRELOCKED_UNDER_LOCK_TABLES) { - if ((new_name != table_name || new_db != db)) + if (alter_ctx.is_table_renamed()) thd->mdl_context.release_all_locks_for_name(mdl_ticket); else mdl_ticket->downgrade_lock(MDL_SHARED_NO_READ_WRITE); } end_temporary: - my_snprintf(tmp_name, sizeof(tmp_name), ER(ER_INSERT_INFO), + my_snprintf(alter_ctx.tmp_name, sizeof(alter_ctx.tmp_name), + ER(ER_INSERT_INFO), (ulong) (copied + deleted), (ulong) deleted, (ulong) thd->get_stmt_da()->current_statement_warn_count()); - my_ok(thd, copied + deleted, 0L, tmp_name); + my_ok(thd, copied + deleted, 0L, alter_ctx.tmp_name); DBUG_RETURN(false); err_new_table_cleanup: @@ -7492,7 +7437,8 @@ err_new_table_cleanup: close_temporary_table(thd, new_table, true, true); } else - (void) quick_rm_table(thd, new_db_type, new_db, tmp_name, + (void) quick_rm_table(thd, new_db_type, + alter_ctx.new_db, alter_ctx.tmp_name, (FN_IS_TMP | (no_ha_table ? NO_HA_TABLE : 0))); /* @@ -7501,12 +7447,12 @@ err_new_table_cleanup: the table to be altered isn't empty. Report error here. */ - if (alter_info->error_if_not_empty && + if (alter_ctx.error_if_not_empty && thd->get_stmt_da()->current_row_for_warning()) { uint f_length; enum enum_mysql_timestamp_type t_type= MYSQL_TIMESTAMP_DATE; - switch (alter_info->datetime_field->sql_type) + switch (alter_ctx.datetime_field->sql_type) { case MYSQL_TYPE_DATE: case MYSQL_TYPE_NEWDATE: @@ -7528,7 +7474,7 @@ err_new_table_cleanup: make_truncated_value_warning(thd, Sql_condition::WARN_LEVEL_WARN, ErrConvString(my_zero_datetime6, f_length), t_type, - alter_info->datetime_field->field_name); + alter_ctx.datetime_field->field_name); thd->abort_on_warning= save_abort_on_warning; } === modified file 'sql/sql_table.h' --- a/sql/sql_table.h 2011-12-19 14:38:42 +0000 +++ b/sql/sql_table.h 2011-12-20 12:49:28 +0000 @@ -21,6 +21,7 @@ #include "m_ctype.h" /* CHARSET_INFO */ class Alter_info; +class Alter_table_ctx; class Create_field; struct TABLE_LIST; class THD; @@ -162,7 +163,8 @@ int mysql_discard_or_import_tablespace(T bool discard); bool mysql_prepare_alter_table(THD *thd, TABLE *table, HA_CREATE_INFO *create_info, - Alter_info *alter_info); + Alter_info *alter_info, + Alter_table_ctx *alter_ctx); bool mysql_trans_prepare_alter_copy_data(THD *thd); bool mysql_trans_commit_alter_copy_data(THD *thd); bool mysql_alter_table(THD *thd, char *new_db, char *new_name, No bundle (reason: useless for push emails).