From: Jon Olav Hauglid Date: October 12 2011 9:35am Subject: bzr push into mysql-trunk-wl5534 branch (jon.hauglid:3402 to 3403) WL#5534 List-Archive: http://lists.mysql.com/commits/141398 Message-Id: <201110120936.p9C9a1eb016583@acsmt356.oracle.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 3403 Jon Olav Hauglid 2011-10-12 WL#5534 Online ALTER, Phase 1. Patch #31: Various refactorings: - Remove unused variables and parameters. - Delay variable definitions. - Reduce goto usage modified: sql/sql_partition.cc sql/sql_partition.h sql/sql_table.cc 3402 Jon Olav Hauglid 2011-10-05 WL#5534 Online ALTER, Phase 1. Patch #30: - Move debug sync point from default implementation of new online alter api to server code. This makes it independent on storage engines. modified: sql/handler.cc sql/sql_table.cc === modified file 'sql/sql_partition.cc' --- a/sql/sql_partition.cc 2011-09-09 16:24:30 +0000 +++ b/sql/sql_partition.cc 2011-10-12 09:35:16 +0000 @@ -4628,7 +4628,6 @@ bool compare_partition_options(HA_CREATE @param[in] table Table object @param[in,out] alter_info Alter information @param[in,out] create_info Create info for CREATE TABLE - @param[in] old_db_type Old engine type @param[out] partition_changed Boolean indicating whether partition changed @param[out] fast_alter_table Internal temporary table allowing fast partition change or NULL if not possible @@ -4649,7 +4648,6 @@ bool compare_partition_options(HA_CREATE uint prep_alter_part_table(THD *thd, TABLE *table, Alter_info *alter_info, HA_CREATE_INFO *create_info, - handlerton *old_db_type, bool *partition_changed, char *db, const char *table_name, === modified file 'sql/sql_partition.h' --- a/sql/sql_partition.h 2011-09-09 16:24:30 +0000 +++ b/sql/sql_partition.h 2011-10-12 09:35:16 +0000 @@ -254,7 +254,6 @@ uint set_part_state(Alter_info *alter_in enum partition_state part_state); uint prep_alter_part_table(THD *thd, TABLE *table, Alter_info *alter_info, HA_CREATE_INFO *create_info, - handlerton *old_db_type, bool *partition_changed, char *db, const char *table_name, === modified file 'sql/sql_table.cc' --- a/sql/sql_table.cc 2011-10-05 13:57:08 +0000 +++ b/sql/sql_table.cc 2011-10-12 09:35:16 +0000 @@ -6124,26 +6124,33 @@ static bool create_altered_table(THD *th the table. */ static bool mysql_inplace_alter_table(THD *thd, + TABLE_LIST *table_list, TABLE *table, - char *new_alias, - char *tmp_name, - char *path, HA_CREATE_INFO *create_info, - Alter_inplace_information *ha_alter_info, - HA_ALTER_FLAGS *ha_alter_flags, Alter_info *alter_info, + HA_ALTER_FLAGS *ha_alter_flags, + Alter_inplace_information *ha_alter_info, + enum_alter_inplace_result inplace_supported, MDL_request *target_mdl_request, - enum_alter_inplace_result inplace_supported) + char *new_alias, + char *tmp_name, + char *db, + char *new_db) { - char new_name[FN_REFLEN + 1]; - char new_db[FN_REFLEN + 1]; - Open_table_context ot_ctx(thd, MYSQL_OPEN_REOPEN); - TABLE tab; - TABLE_LIST tbl; - handlerton *db_type= table->s->db_type(); - DBUG_ENTER("mysql_inplace_alter_table"); + create_info->options&= ~HA_LEX_CREATE_TMP_TABLE; + create_info->frm_only= 1; + if (create_altered_table(thd, + db, + new_db, + tmp_name, + create_info, + alter_info)) + { + DBUG_RETURN(true); + } + if ((inplace_supported == HA_ALTER_INPLACE_EXCLUSIVE_LOCK || alter_info->requested_lock == Alter_info::ALTER_TABLE_LOCK_EXCLUSIVE) && wait_while_table_is_used(thd, table, HA_EXTRA_FORCE_REOPEN)) @@ -6169,6 +6176,10 @@ static bool mysql_inplace_alter_table(TH if (trans_commit_stmt(thd) || trans_commit_implicit(thd)) DBUG_RETURN(true); + Open_table_context ot_ctx(thd, MYSQL_OPEN_REOPEN); + handlerton *db_type= table->s->db_type(); + MDL_ticket *mdl_ticket= table->mdl_ticket; + /* Tell the handler to prepare for the online alter */ @@ -6180,18 +6191,27 @@ static bool mysql_inplace_alter_table(TH ha_alter_info, ha_alter_flags)) { - goto err; + goto rollback; } if (table->file->inplace_alter_table(create_info, ha_alter_info, ha_alter_flags)) { - goto err; + goto rollback; } + /* + The ALTER TABLE is always in its own transaction. + Commit must not be called while LOCK_open is locked. It could call + wait_if_global_read_lock(), which could create a deadlock if called + with LOCK_open. + */ + if (trans_commit_stmt(thd) || trans_commit_implicit(thd)) + goto rollback; + if (wait_while_table_is_used(thd, table, HA_EXTRA_PREPARE_FOR_RENAME)) - goto err; + goto rollback; DBUG_EXECUTE_IF("alter_table_rollback_new_index", { table->file->commit_inplace_alter_table(create_info, @@ -6207,7 +6227,7 @@ static bool mysql_inplace_alter_table(TH ha_alter_flags, true)) { - goto err; + goto rollback; } /* @@ -6215,10 +6235,7 @@ static bool mysql_inplace_alter_table(TH and will be renamed to the original table name. */ - strcpy(new_name, table->s->table_name.str); - strcpy(new_db, table->s->db.str); - - close_all_tables_for_name(thd, table->s, FALSE); + close_all_tables_for_name(thd, table->s, false); if (mysql_rename_table(db_type, new_db, @@ -6226,36 +6243,22 @@ static bool mysql_inplace_alter_table(TH new_db, new_alias, FN_FROM_IS_TMP | FRM_ONLY)) { - goto err; + DBUG_RETURN(true); } - /* - The ALTER TABLE is always in its own transaction. - Commit must not be called while LOCK_open is locked. It could call - wait_if_global_read_lock(), which could create a deadlock if called - with LOCK_open. - */ - if (trans_commit_stmt(thd) || trans_commit_implicit(thd)) - goto err; - - tbl.init_one_table(new_db, strlen(new_db), - new_name, strlen(new_name), - new_name, TL_READ_NO_INSERT); - tbl.open_type= OT_TEMPORARY_OR_BASE; - tbl.i_s_requested_object= OPEN_TABLE_ONLY; - tbl.open_strategy= TABLE_LIST::OPEN_NORMAL; - tbl.mdl_request.ticket= target_mdl_request->ticket; - - if (open_table(thd, &tbl, thd->mem_root, &ot_ctx)) - goto err; + table_list->table= table= NULL; + table_list->mdl_request.ticket= mdl_ticket; + if (open_table(thd, table_list, thd->mem_root, &ot_ctx)) + DBUG_RETURN(true); /* Tell the handler that the changed frm is on disk and table has been re-opened */ - tbl.table->file->ha_create_handler_files(path, NULL, - CHF_INDEX_FLAG, - create_info); + char path[FN_REFLEN + 1]; + build_table_filename(path, sizeof(path) - 1, db, table_list->table_name, "", 0); + table_list->table->file->ha_create_handler_files(path, NULL, CHF_INDEX_FLAG, + create_info); /* We are going to reopen table down on the road, so we have to restore @@ -6263,11 +6266,11 @@ static bool mysql_inplace_alter_table(TH object to make it suitable for reopening. */ close_thread_table(thd, &thd->open_tables); - tbl.table= NULL; + table_list->table= NULL; DBUG_RETURN(false); - err: + rollback: table->file->commit_inplace_alter_table(create_info, ha_alter_info, ha_alter_flags, @@ -6752,7 +6755,6 @@ err: @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 @@ -6760,8 +6762,6 @@ err: @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 @@ -6769,49 +6769,47 @@ err: */ static int simple_rename_or_index_change(THD *thd, TABLE_LIST *table_list, - TABLE *table, Alter_info *alter_info, + 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) + char *new_alias, + char *db, char *table_name, + char *alias) { + TABLE *table= table_list->table; + MDL_ticket *mdl_ticket= table->mdl_ticket; int error= 0; - switch (alter_info->keys_onoff) { - case Alter_info::LEAVE_AS_IS: - break; - case Alter_info::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 Alter_info::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, Sql_condition::WARN_LEVEL_NOTE, - ER_ILLEGAL_HA, ER(ER_ILLEGAL_HA), - table->alias); - error= 0; - } - else if (error > 0) + DBUG_ENTER("simple_rename_or_index_change"); + + if (alter_info->keys_onoff != Alter_info::LEAVE_AS_IS) { - table->file->print_error(error, MYF(0)); - error= -1; + if (wait_while_table_is_used(thd, table, HA_EXTRA_FORCE_REOPEN)) + DBUG_RETURN(-1); + if (alter_info->keys_onoff == Alter_info::ENABLE) + { + DBUG_EXECUTE_IF("sleep_alter_enable_indexes", my_sleep(6000000);); + error= table->file->ha_enable_indexes(HA_KEY_SWITCH_NONUNIQ_SAVE); + } + else if (alter_info->keys_onoff == Alter_info::DISABLE) + error=table->file->ha_disable_indexes(HA_KEY_SWITCH_NONUNIQ_SAVE); + + if (error == HA_ERR_WRONG_COMMAND) + { + push_warning_printf(thd, Sql_condition::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_STAGE_INFO(thd, stage_rename); + handlerton *old_db_type= table->s->db_type(); /* Then do a 'simple' rename of the table. First we need to close all instances of 'source' table. @@ -6821,8 +6819,8 @@ static int simple_rename_or_index_change 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); + 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)) @@ -6859,7 +6857,7 @@ static int simple_rename_or_index_change else mdl_ticket->downgrade_exclusive_lock(MDL_SHARED_NO_READ_WRITE); } - return error; + DBUG_RETURN(error); } @@ -6911,19 +6909,6 @@ bool mysql_alter_table(THD *thd,char *ne Alter_info *alter_info, uint order_num, ORDER *order, bool ignore) { - TABLE *table, *new_table= 0; - MDL_ticket *mdl_ticket; - MDL_request target_mdl_request; - int error= 0; - char tmp_name[80],old_name[32],new_name_buff[FN_REFLEN + 1]; - char new_alias_buff[FN_REFLEN], *table_name, *db, *new_alias, *alias; - char path[FN_REFLEN + 1]; - char reg_path[FN_REFLEN+1]; - ha_rows copied=0,deleted=0; - handlerton *old_db_type, *new_db_type, *save_old_db_type; -#ifdef WITH_PARTITION_STORAGE_ENGINE - bool partition_changed= FALSE; -#endif DBUG_ENTER("mysql_alter_table"); /* @@ -6934,11 +6919,9 @@ bool mysql_alter_table(THD *thd,char *ne */ if (table_list && table_list->db && table_list->table_name) { - int table_kind= 0; - - table_kind= check_if_log_table(table_list->db_length, table_list->db, - table_list->table_name_length, - table_list->table_name, 0); + int table_kind= check_if_log_table(table_list->db_length, table_list->db, + table_list->table_name_length, + table_list->table_name, false); if (table_kind) { @@ -6946,7 +6929,7 @@ bool mysql_alter_table(THD *thd,char *ne if (logger.is_log_table_enabled(table_kind)) { my_error(ER_BAD_LOG_STATEMENT, MYF(0), "ALTER"); - DBUG_RETURN(TRUE); + DBUG_RETURN(true); } /* Disable alter of log tables to unsupported engine */ @@ -6955,44 +6938,27 @@ bool mysql_alter_table(THD *thd,char *ne !(create_info->db_type->flags & HTON_SUPPORT_LOG_TABLES))) { my_error(ER_UNSUPORTED_LOG_ENGINE, MYF(0)); - DBUG_RETURN(TRUE); + DBUG_RETURN(true); } #ifdef WITH_PARTITION_STORAGE_ENGINE if (alter_info->flags & ALTER_PARTITION) { my_error(ER_WRONG_USAGE, MYF(0), "PARTITION", "log table"); - DBUG_RETURN(TRUE); + DBUG_RETURN(true); } #endif } } - /* - 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_STAGE_INFO(thd, stage_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; - - thd->add_to_binlog_accessed_dbs(db); - if (new_db != db) - thd->add_to_binlog_accessed_dbs(new_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); - /* DISCARD/IMPORT TABLESPACE is always alone in an ALTER TABLE */ if (alter_info->tablespace_op != Alter_info::NO_TABLESPACE_OP) /* Conditionally writes to binlog. */ DBUG_RETURN(mysql_discard_or_import_tablespace(thd,table_list, alter_info)); + THD_STAGE_INFO(thd, stage_init); + /* Code below can handle only base tables so ensure that we won't open a view. Note that RENAME TABLE the only ALTER clause which is supported for views @@ -7003,19 +6969,17 @@ bool mysql_alter_table(THD *thd,char *ne Alter_table_prelocking_strategy alter_prelocking_strategy(alter_info); DEBUG_SYNC(thd, "alter_table_before_open_tables"); - error= open_and_lock_tables(thd, table_list, FALSE, 0, - &alter_prelocking_strategy); + int error= open_and_lock_tables(thd, table_list, false, 0, + &alter_prelocking_strategy); DEBUG_SYNC(thd, "alter_opened_table"); if (error) - { - DBUG_RETURN(TRUE); - } + DBUG_RETURN(true); - table= table_list->table; + TABLE *table= table_list->table; table->use_all_columns(); - mdl_ticket= table->mdl_ticket; + MDL_ticket *mdl_ticket= table->mdl_ticket; /* Prohibit changing of the UNION list of a non-temporary MERGE table @@ -7029,13 +6993,33 @@ bool mysql_alter_table(THD *thd,char *ne (table->s->tmp_table == NO_TMP_TABLE)) { my_error(ER_LOCK_OR_ACTIVE_TRANSACTION, MYF(0)); - DBUG_RETURN(TRUE); + 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; + + thd->add_to_binlog_accessed_dbs(db); + if (new_db != db) + thd->add_to_binlog_accessed_dbs(new_db); + + char *new_alias; + MDL_request target_mdl_request; + /* Check that we are not trying to rename to an existing table */ if (new_name) { DBUG_PRINT("info", ("new_db.new_name: '%s'.'%s'", new_db, new_name)); + char new_name_buff[FN_REFLEN + 1]; + char new_alias_buff[FN_REFLEN]; strmov(new_name_buff,new_name); strmov(new_alias= new_alias_buff, new_name); if (lower_case_table_names) @@ -7063,7 +7047,7 @@ bool mysql_alter_table(THD *thd,char *ne if (find_temporary_table(thd,new_db,new_name_buff)) { my_error(ER_TABLE_EXISTS_ERROR, MYF(0), new_name_buff); - DBUG_RETURN(TRUE); + DBUG_RETURN(true); } } else @@ -7079,12 +7063,12 @@ bool mysql_alter_table(THD *thd,char *ne MDL_INTENTION_EXCLUSIVE)); if (thd->mdl_context.try_acquire_lock(&target_mdl_request)) - DBUG_RETURN(TRUE); + 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); + DBUG_RETURN(true); } DEBUG_SYNC(thd, "locked_table_name"); /* @@ -7097,7 +7081,7 @@ bool mysql_alter_table(THD *thd,char *ne { /* Table will be closed in do_command() */ my_error(ER_TABLE_EXISTS_ERROR, MYF(0), new_alias); - goto err; + DBUG_RETURN(true); } } } @@ -7108,7 +7092,6 @@ bool mysql_alter_table(THD *thd,char *ne new_name= table_name; } - old_db_type= table->s->db_type(); if (!create_info->db_type) { #ifdef WITH_PARTITION_STORAGE_ENGINE @@ -7126,19 +7109,18 @@ bool mysql_alter_table(THD *thd,char *ne } else #endif - create_info->db_type= old_db_type; + create_info->db_type= table->s->db_type(); } if (check_engine(thd, new_name, create_info)) - goto err; - new_db_type= create_info->db_type; + DBUG_RETURN(true); - if ((new_db_type != old_db_type || + if ((create_info->db_type != table->s->db_type() || alter_info->flags & ALTER_PARTITION) && !table->file->can_switch_engines()) { my_error(ER_ROW_IS_REFERENCED, MYF(0)); - goto err; + DBUG_RETURN(true); } /* @@ -7158,44 +7140,41 @@ bool mysql_alter_table(THD *thd,char *ne } DBUG_PRINT("info", ("old type: %s new type: %s", - ha_resolve_storage_engine_name(old_db_type), - ha_resolve_storage_engine_name(new_db_type))); - if (ha_check_storage_engine_flag(old_db_type, HTON_ALTER_NOT_SUPPORTED) || - ha_check_storage_engine_flag(new_db_type, HTON_ALTER_NOT_SUPPORTED)) + ha_resolve_storage_engine_name(table->s->db_type()), + ha_resolve_storage_engine_name(create_info->db_type))); + if (ha_check_storage_engine_flag(table->s->db_type(), HTON_ALTER_NOT_SUPPORTED) || + ha_check_storage_engine_flag(create_info->db_type, HTON_ALTER_NOT_SUPPORTED)) { DBUG_PRINT("info", ("doesn't support alter")); - my_error(ER_ILLEGAL_HA, MYF(0), table_name); - goto err; + my_error(ER_ILLEGAL_HA, MYF(0), table_list->table_name); + DBUG_RETURN(true); } THD_STAGE_INFO(thd, stage_setup); if (!(alter_info->flags & ~(ALTER_RENAME | ALTER_KEYS_ONOFF)) && !table->s->tmp_table) // no need to touch frm { - 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)); + DBUG_RETURN(simple_rename_or_index_change(thd, table_list, alter_info, + new_db, new_name, new_alias, + db, table_name, alias)); } /* We have to do full alter table. */ if (mysql_prepare_alter_table(thd, table, create_info, alter_info)) - goto err; + DBUG_RETURN(true); set_table_default_charset(thd, create_info, db); - 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); - #ifdef WITH_PARTITION_STORAGE_ENGINE + bool partition_changed= false; { TABLE *table_for_fast_alter_partition= NULL; bool is_fast_alter_partitioning= false; - if (prep_alter_part_table(thd, table, alter_info, create_info, old_db_type, + 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, &table_for_fast_alter_partition, @@ -7204,22 +7183,9 @@ bool mysql_alter_table(THD *thd,char *ne /* If prep_alter_part_table created an intermediate table, destroy it. */ if (table_for_fast_alter_partition) close_temporary(table_for_fast_alter_partition, true, false); - goto err; + DBUG_RETURN(true); } - /* - If the old table had partitions and we are doing ALTER TABLE ... - engine= , the new table must preserve the original - partitioning. That means that the new engine is still the - partitioning engine, not the engine specified in the parser. - This is discovered in prep_alter_part_table, which in such case - updates create_info->db_type. - Now we need to update the stack copy of create_info->db_type, - as otherwise we won't be able to correctly move the files of the - temporary table to the result table files. - */ - new_db_type= create_info->db_type; - if (is_fast_alter_partitioning) { DBUG_RETURN(fast_alter_partition_table(thd, table, alter_info, @@ -7245,11 +7211,23 @@ bool mysql_alter_table(THD *thd,char *ne Alter_info::ALTER_TABLE_ALGORITHM_INPLACE) { my_error(ER_NOT_SUPPORTED_YET, MYF(0), thd->query()); - goto err; + DBUG_RETURN(true); } alter_info->requested_algorithm= Alter_info::ALTER_TABLE_ALGORITHM_COPY; } + handlerton *old_db_type= table->s->db_type(); + handlerton *new_db_type= create_info->db_type; + 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); + if (alter_info->requested_algorithm != Alter_info::ALTER_TABLE_ALGORITHM_COPY) { Alter_inplace_information ha_alter_info; @@ -7262,7 +7240,7 @@ bool mysql_alter_table(THD *thd,char *ne &ha_alter_info, &table_changes)) { - goto err; + DBUG_RETURN(true); } /* @@ -7299,7 +7277,7 @@ bool mysql_alter_table(THD *thd,char *ne Alter_info::ALTER_TABLE_LOCK_SHARED) { my_error(ER_NOT_SUPPORTED_YET, MYF(0), thd->query()); - goto err; + DBUG_RETURN(true); } break; case HA_ALTER_INPLACE_SHARED_LOCK: @@ -7308,7 +7286,7 @@ bool mysql_alter_table(THD *thd,char *ne Alter_info::ALTER_TABLE_LOCK_NONE) { my_error(ER_NOT_SUPPORTED_YET, MYF(0), thd->query()); - goto err; + DBUG_RETURN(true); } break; case HA_ALTER_INPLACE_NO_LOCK_AFTER_PREPARE: @@ -7320,44 +7298,25 @@ bool mysql_alter_table(THD *thd,char *ne Alter_info::ALTER_TABLE_ALGORITHM_INPLACE) { my_error(ER_NOT_SUPPORTED_YET, MYF(0), thd->query()); - goto err; + DBUG_RETURN(true); } // Otherwise use COPY use_inplace= false; break; case HA_ALTER_ERROR: default: - goto err; + DBUG_RETURN(true); } if (use_inplace) { - create_info->options&= ~HA_LEX_CREATE_TMP_TABLE; - create_info->frm_only= 1; - - if (create_altered_table(thd, - db, - new_db, - tmp_name, - create_info, - alter_info)) - { - goto err; - } - - if (mysql_inplace_alter_table(thd, - table, - new_alias, - tmp_name, - path, - create_info, - &ha_alter_info, - &ha_alter_flags, - alter_info, - &target_mdl_request, - inplace_supported)) + if (mysql_inplace_alter_table(thd, table_list, table, + create_info, alter_info, + &ha_alter_flags, &ha_alter_info, + inplace_supported, &target_mdl_request, + new_alias, tmp_name, db, new_db)) { - goto err; + DBUG_RETURN(true); } goto end_inplace; @@ -7370,17 +7329,17 @@ bool mysql_alter_table(THD *thd,char *ne // If EXCLUSIVE lock is requested, upgrade already. if (alter_info->requested_lock == Alter_info::ALTER_TABLE_LOCK_EXCLUSIVE && wait_while_table_is_used(thd, table, HA_EXTRA_FORCE_REOPEN)) - goto err; + DBUG_RETURN(true); // COPY algorithm doesn't work with concurrent writes. if (alter_info->requested_lock == Alter_info::ALTER_TABLE_LOCK_NONE) { my_error(ER_NOT_SUPPORTED_YET, MYF(0), thd->query()); - goto err; + DBUG_RETURN(true); } if (create_altered_table(thd, db, new_db, tmp_name, create_info, alter_info)) - goto err; + DBUG_RETURN(true); /* Open the table since we need to copy the data. */ if (table->s->tmp_table != NO_TMP_TABLE) @@ -7400,7 +7359,7 @@ bool mysql_alter_table(THD *thd,char *ne 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); + new_table= open_table_uncached(thd, path, new_db, tmp_name, true); } if (!new_table) goto err_new_table_cleanup; @@ -7459,7 +7418,7 @@ bool mysql_alter_table(THD *thd,char *ne thd->locked_tables_mode != LTM_PRELOCKED_UNDER_LOCK_TABLES) { mysql_unlock_tables(thd, thd->lock); - thd->lock=0; + thd->lock= NULL; } else { @@ -7471,15 +7430,19 @@ bool mysql_alter_table(THD *thd,char *ne } } /* Remove link to old table and rename the new one */ - close_temporary_table(thd, table, 1, 1); + 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)) goto err_new_table_cleanup; /* We don't replicate alter table statement on temporary tables */ if (!thd->is_current_stmt_binlog_format_row() && - write_bin_log(thd, TRUE, thd->query(), thd->query_length())) - DBUG_RETURN(TRUE); - goto end_temporary; + write_bin_log(thd, true, thd->query(), thd->query_length())) + DBUG_RETURN(true); + my_snprintf(tmp_name, sizeof(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); + DBUG_RETURN(false); } /* @@ -7487,8 +7450,8 @@ bool mysql_alter_table(THD *thd,char *ne not delete it! Even altough MERGE tables do not have their children attached here it is safe to call close_temporary_table(). */ - close_temporary_table(thd, new_table, 1, 0); - new_table= 0; + close_temporary_table(thd, new_table, true, false); + new_table= NULL; DEBUG_SYNC(thd, "alter_table_before_rename_result_table"); @@ -7510,10 +7473,6 @@ bool mysql_alter_table(THD *thd,char *ne */ THD_STAGE_INFO(thd, stage_rename_result_table); - my_snprintf(old_name, sizeof(old_name), "%s2-%lx-%lx", tmp_file_prefix, - current_pid, thd->thread_id); - if (lower_case_table_names) - my_casedn_str(files_charset_info, old_name); if (wait_while_table_is_used(thd, table, HA_EXTRA_PREPARE_FOR_RENAME)) { @@ -7524,10 +7483,7 @@ bool mysql_alter_table(THD *thd,char *ne close_all_tables_for_name(thd, table->s, new_name != table_name || new_db != db); - - error=0; - table_list->table= table= 0; /* Safety */ - save_old_db_type= old_db_type; + table_list->table= table= NULL; /* Safety */ /* This leads to the storage engine (SE) not being notified for renames in @@ -7542,6 +7498,12 @@ 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. */ + char old_name[32]; + my_snprintf(old_name, sizeof(old_name), "%s2-%lx-%lx", tmp_file_prefix, + current_pid, thd->thread_id); + if (lower_case_table_names) + my_casedn_str(files_charset_info, old_name); + if (mysql_rename_table(old_db_type, db, table_name, db, old_name, FN_TO_IS_TMP)) { @@ -7583,8 +7545,8 @@ end_inplace: DBUG_ASSERT(!(mysql_bin_log.is_open() && thd->is_current_stmt_binlog_format_row() && (create_info->options & HA_LEX_CREATE_TMP_TABLE))); - if (write_bin_log(thd, TRUE, thd->query(), thd->query_length())) - DBUG_RETURN(TRUE); + if (write_bin_log(thd, true, thd->query(), thd->query_length())) + DBUG_RETURN(true); if (ha_check_storage_engine_flag(old_db_type, HTON_FLUSH_AFTER_RENAME)) { @@ -7596,7 +7558,7 @@ end_inplace: 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); + t_table= open_table_uncached(thd, path, new_db, new_name, false); if (t_table) { intern_close_table(t_table); @@ -7607,8 +7569,8 @@ end_inplace: new_db,table_name); ha_flush_logs(old_db_type); } - table_list->table=0; // For query cache - query_cache_invalidate3(thd, table_list, 0); + table_list->table= NULL; // For query cache + query_cache_invalidate3(thd, table_list, false); if (thd->locked_tables_mode == LTM_LOCK_TABLES || thd->locked_tables_mode == LTM_PRELOCKED_UNDER_LOCK_TABLES) @@ -7619,24 +7581,22 @@ end_inplace: mdl_ticket->downgrade_exclusive_lock(MDL_SHARED_NO_READ_WRITE); } -end_temporary: my_snprintf(tmp_name, sizeof(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); - DBUG_RETURN(FALSE); + DBUG_RETURN(false); err_new_table_cleanup: if (new_table) { /* close_temporary_table() frees the new_table pointer. */ - close_temporary_table(thd, new_table, 1, 1); + close_temporary_table(thd, new_table, true, true); } else (void) quick_rm_table(new_db_type, new_db, tmp_name, create_info->frm_only ? FN_IS_TMP | FRM_ONLY : FN_IS_TMP); -err: /* No default value was provided for a DATE/DATETIME field, the current sql_mode doesn't allow the '0000-00-00' value and @@ -7664,14 +7624,14 @@ err: DBUG_ASSERT(0); } bool save_abort_on_warning= thd->abort_on_warning; - thd->abort_on_warning= TRUE; + thd->abort_on_warning= true; make_truncated_value_warning(thd, Sql_condition::WARN_LEVEL_WARN, f_val, strlength(f_val), t_type, alter_info->datetime_field->field_name); thd->abort_on_warning= save_abort_on_warning; } - DBUG_RETURN(TRUE); + DBUG_RETURN(true); err_with_mdl: /* @@ -7682,7 +7642,7 @@ err_with_mdl: */ thd->locked_tables_list.unlink_all_closed_tables(thd, NULL, 0); thd->mdl_context.release_all_locks_for_name(mdl_ticket); - DBUG_RETURN(TRUE); + DBUG_RETURN(true); } /* mysql_alter_table */ No bundle (reason: useless for push emails).