From: Dmitry Lenev Date: February 1 2012 7:32am Subject: bzr push into mysql-trunk-wl5534 branch (Dmitry.Lenev:3475 to 3476) WL#5534 List-Archive: http://lists.mysql.com/commits/142685 Message-Id: <20120201073215.7B6E04205F7@jubjub> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 3476 Dmitry Lenev 2012-02-01 WL#5534 Online ALTER, Phase 1. Patch #82: Made HA_CREATE_INFO and Alter_info part of Alter_inplace_info. Documented their state. modified: sql/ha_partition.cc sql/ha_partition.h sql/handler.cc sql/handler.h sql/sql_table.cc 3475 Dmitry Lenev 2012-01-31 WL#5534 Online ALTER, Phase 1. Patch #81: - Documented how contents of Alter_inplace_info::key_info_buffer are produced. - Got rid of redundant calls to mysql_prepare_create_table(). modified: mysql-test/r/archive.result mysql-test/r/ctype_utf8mb4.result mysql-test/suite/innodb/r/innodb_index_large_prefix.result mysql-test/suite/innodb/r/innodb_mysql.result mysql-test/suite/innodb/r/innodb_prefix_index_liftedlimit.result sql/ha_partition.cc sql/handler.h sql/sql_table.cc === modified file 'sql/ha_partition.cc' --- a/sql/ha_partition.cc 2012-01-31 14:35:18 +0000 +++ b/sql/ha_partition.cc 2012-02-01 07:31:47 +0000 @@ -6941,8 +6941,6 @@ public: enum_alter_inplace_result ha_partition::check_if_supported_inplace_alter(TABLE *altered_table, - HA_CREATE_INFO *create_info, - const Alter_info *alter_info, Alter_inplace_info *ha_alter_info) { uint index= 0; @@ -6969,8 +6967,6 @@ ha_partition::check_if_supported_inplace { enum_alter_inplace_result p_result= m_file[index]->check_if_supported_inplace_alter(altered_table, - create_info, - alter_info, ha_alter_info); part_inplace_ctx->handler_ctx_array[index]= ha_alter_info->handler_ctx; @@ -6986,7 +6982,6 @@ ha_partition::check_if_supported_inplace bool ha_partition::prepare_inplace_alter_table(TABLE *altered_table, - HA_CREATE_INFO *create_info, Alter_inplace_info *ha_alter_info) { uint index= 0; @@ -7001,7 +6996,7 @@ bool ha_partition::prepare_inplace_alter for (index= 0; index < m_tot_parts && !error; index++) { ha_alter_info->handler_ctx= part_inplace_ctx->handler_ctx_array[index]; - if (m_file[index]->ha_prepare_inplace_alter_table(altered_table, create_info, + if (m_file[index]->ha_prepare_inplace_alter_table(altered_table, ha_alter_info)) error= true; part_inplace_ctx->handler_ctx_array[index]= ha_alter_info->handler_ctx; @@ -7013,7 +7008,6 @@ bool ha_partition::prepare_inplace_alter bool ha_partition::inplace_alter_table(TABLE *altered_table, - HA_CREATE_INFO *create_info, Alter_inplace_info *ha_alter_info) { uint index= 0; @@ -7028,7 +7022,7 @@ bool ha_partition::inplace_alter_table(T for (index= 0; index < m_tot_parts && !error; index++) { ha_alter_info->handler_ctx= part_inplace_ctx->handler_ctx_array[index]; - if (m_file[index]->ha_inplace_alter_table(altered_table, create_info, + if (m_file[index]->ha_inplace_alter_table(altered_table, ha_alter_info)) error= true; part_inplace_ctx->handler_ctx_array[index]= ha_alter_info->handler_ctx; @@ -7047,7 +7041,6 @@ bool ha_partition::inplace_alter_table(T (As X-lock will be held here if new indexes are to be committed) */ bool ha_partition::commit_inplace_alter_table(TABLE *altered_table, - HA_CREATE_INFO *create_info, Alter_inplace_info *ha_alter_info, bool commit) { @@ -7065,7 +7058,7 @@ bool ha_partition::commit_inplace_alter_ for (index= 0; index < m_tot_parts; index++) { ha_alter_info->handler_ctx= part_inplace_ctx->handler_ctx_array[index]; - if (m_file[index]->ha_commit_inplace_alter_table(altered_table, create_info, + if (m_file[index]->ha_commit_inplace_alter_table(altered_table, ha_alter_info, commit)) { part_inplace_ctx->handler_ctx_array[index]= ha_alter_info->handler_ctx; @@ -7078,7 +7071,7 @@ bool ha_partition::commit_inplace_alter_ { index++; ha_alter_info->handler_ctx= part_inplace_ctx->handler_ctx_array[index]; - m_file[index]->ha_commit_inplace_alter_table(altered_table, create_info, + m_file[index]->ha_commit_inplace_alter_table(altered_table, ha_alter_info, false); part_inplace_ctx->handler_ctx_array[index]= ha_alter_info->handler_ctx; goto err; @@ -7100,7 +7093,11 @@ err: Alter_inplace_info::ADD_UNIQUE_INDEX | Alter_inplace_info::ADD_PK_INDEX)) { - Alter_inplace_info drop_info(ha_alter_info->ignore, NULL, 0); + Alter_inplace_info drop_info(ha_alter_info->create_info, + ha_alter_info->alter_info, + NULL, 0, + ha_alter_info->ignore); + if (ha_alter_info->handler_flags & Alter_inplace_info::ADD_INDEX) drop_info.handler_flags|= Alter_inplace_info::DROP_INDEX; if (ha_alter_info->handler_flags & Alter_inplace_info::ADD_UNIQUE_INDEX) @@ -7127,12 +7124,9 @@ err: for (uint i= 0; i < index; i++) { bool error= m_file[i]->ha_prepare_inplace_alter_table(altered_table, - create_info, &drop_info); - error|= m_file[i]->ha_inplace_alter_table(altered_table, create_info, - &drop_info); + error|= m_file[i]->ha_inplace_alter_table(altered_table, &drop_info); error|= m_file[i]->ha_commit_inplace_alter_table(altered_table, - create_info, &drop_info, true); if (error) sql_print_error("Failed with error handling of adding index:\n" @@ -7146,7 +7140,7 @@ err: for (uint i= index+1; i < m_tot_parts; i++) { ha_alter_info->handler_ctx= part_inplace_ctx->handler_ctx_array[i]; - if (m_file[i]->ha_commit_inplace_alter_table(altered_table, create_info, + if (m_file[i]->ha_commit_inplace_alter_table(altered_table, ha_alter_info, false)) { /* How could this happen? */ === modified file 'sql/ha_partition.h' --- a/sql/ha_partition.h 2012-01-26 10:02:26 +0000 +++ b/sql/ha_partition.h 2012-02-01 07:31:47 +0000 @@ -1057,17 +1057,12 @@ public: */ virtual enum_alter_inplace_result check_if_supported_inplace_alter(TABLE *altered_table, - HA_CREATE_INFO *create_info, - const Alter_info *alter_info, Alter_inplace_info *ha_alter_info); virtual bool prepare_inplace_alter_table(TABLE *altered_table, - HA_CREATE_INFO *create_info, Alter_inplace_info *ha_alter_info); virtual bool inplace_alter_table(TABLE *altered_table, - HA_CREATE_INFO *create_info, Alter_inplace_info *ha_alter_info); virtual bool commit_inplace_alter_table(TABLE *altered_table, - HA_CREATE_INFO *create_info, Alter_inplace_info *ha_alter_info, bool commit); virtual void notify_table_changed(); === modified file 'sql/handler.cc' --- a/sql/handler.cc 2012-01-31 12:45:28 +0000 +++ b/sql/handler.cc 2012-02-01 07:31:47 +0000 @@ -3669,7 +3669,6 @@ handler::ha_discard_or_import_tablespace bool handler::ha_commit_inplace_alter_table(TABLE *altered_table, - HA_CREATE_INFO *create_info, Alter_inplace_info *ha_alter_info, bool commit) { @@ -3685,8 +3684,7 @@ bool handler::ha_commit_inplace_alter_ta MDL_EXCLUSIVE) || !commit); - return commit_inplace_alter_table(altered_table, create_info, - ha_alter_info, commit); + return commit_inplace_alter_table(altered_table, ha_alter_info, commit); } @@ -3768,12 +3766,12 @@ static void check_alter_index_flags(ulon enum_alter_inplace_result handler::check_if_supported_inplace_alter(TABLE *altered_table, - HA_CREATE_INFO *create_info, - const Alter_info *alter_info, Alter_inplace_info *ha_alter_info) { DBUG_ENTER("check_if_supported_alter"); + HA_CREATE_INFO *create_info= ha_alter_info->create_info; + ulong handler_alter_flags= table->file->alter_table_flags(0); DBUG_PRINT("info", ("handler_alter_flags: %lu", handler_alter_flags)); @@ -3947,7 +3945,6 @@ handler::check_if_supported_inplace_alte bool handler::ha_prepare_inplace_alter_table(TABLE *altered_table, - HA_CREATE_INFO *create_info, Alter_inplace_info *ha_alter_info) { DBUG_ASSERT(table_share->tmp_table != NO_TMP_TABLE || @@ -3956,8 +3953,7 @@ bool handler::ha_prepare_inplace_alter_t prepare_for_alter(); - return prepare_inplace_alter_table(altered_table, create_info, - ha_alter_info); + return prepare_inplace_alter_table(altered_table, ha_alter_info); } @@ -3967,7 +3963,6 @@ bool handler::ha_prepare_inplace_alter_t */ bool handler::inplace_alter_table(TABLE *altered_table, - HA_CREATE_INFO *create_info, Alter_inplace_info *ha_alter_info) { DBUG_ENTER("inplace_alter_table"); @@ -4065,7 +4060,6 @@ bool handler::inplace_alter_table(TABLE */ bool handler::commit_inplace_alter_table(TABLE *altered_table, - HA_CREATE_INFO *create_info, Alter_inplace_info *ha_alter_info, bool commit) { === modified file 'sql/handler.h' --- a/sql/handler.h 2012-01-31 14:35:18 +0000 +++ b/sql/handler.h 2012-02-01 07:31:47 +0000 @@ -1028,6 +1028,30 @@ public: static const HA_ALTER_FLAGS ALTER_RENAME = 1L << 17; /** + Create options (like MAX_ROWS) for the new version of table. + + @note The referenced instance of HA_CREATE_INFO object was already + used to create new .FRM file for table being altered. So it + has been processed by mysql_prepare_create_table() already. + For example, this means that it has HA_OPTION_PACK_RECORD + flag in HA_CREATE_INFO::table_options member correctly set. + */ + HA_CREATE_INFO *create_info; + + /** + Alter options, fields and keys for the new version of table. + + @note The referenced instance of Alter_info object was already + used to create new .FRM file for table being altered. So it + has been processed by mysql_prepare_create_table() already. + In particular, this means that in Create_field objects for + fields which were present in some form in the old version + of table, Create_field::field member points to corresponding + Field instance for old version of table. + */ + Alter_info *alter_info; + + /** Array of KEYs for new version of table - including KEYs to be added. @note Currently this array is produced as result of @@ -1081,8 +1105,13 @@ public: /** true for ALTER IGNORE TABLE ... */ const bool ignore; - Alter_inplace_info(bool ignore_arg, KEY *key_info_arg, uint key_count_arg) - :key_info_buffer(key_info_arg), + Alter_inplace_info(HA_CREATE_INFO *create_info_arg, + Alter_info *alter_info_arg, + KEY *key_info_arg, uint key_count_arg, + bool ignore_arg) + : create_info(create_info_arg), + alter_info(alter_info_arg), + key_info_buffer(key_info_arg), key_count(key_count_arg), index_drop_count(0), index_drop_buffer(NULL), @@ -2521,10 +2550,9 @@ public: Check if a storage engine supports a particular alter table in-place @param altered_table TABLE object for new version of table. - @param create_info Information from the parsing phase about new - table properties. - @param alter_info Data related to detected changes. - @param ha_alter_info Structure holding data used during in-place alter. + @param ha_alter_info Structure describing changes to be done + by ALTER TABLE and holding data used + during in-place alter. @retval HA_ALTER_ERROR Unexpected error. @retval HA_ALTER_INPLACE_NOT_SUPPORTED Not supported, must use copy. @@ -2541,15 +2569,9 @@ public: to determine if the storage engine supports in-place ALTER or not. @note Called without holding thr_lock.c lock. - - TODO/FIXME: Clarify if HA_CREATE_INFO and Alter_info get here after - being processed by mysql_prepare_create_table() or straigth - from the parser. */ virtual enum_alter_inplace_result check_if_supported_inplace_alter(TABLE *altered_table, - HA_CREATE_INFO *create_info, - const Alter_info *alter_info, Alter_inplace_info *ha_alter_info); @@ -2558,7 +2580,6 @@ public: @see prepare_inplace_alter_table() */ bool ha_prepare_inplace_alter_table(TABLE *altered_table, - HA_CREATE_INFO *create_info, Alter_inplace_info *ha_alter_info); @@ -2567,10 +2588,9 @@ public: @see inplace_alter_table() */ bool ha_inplace_alter_table(TABLE *altered_table, - HA_CREATE_INFO *create_info, Alter_inplace_info *ha_alter_info) { - return inplace_alter_table(altered_table, create_info, ha_alter_info); + return inplace_alter_table(altered_table, ha_alter_info); } @@ -2580,7 +2600,6 @@ public: @see commit_inplace_alter_table() */ bool ha_commit_inplace_alter_table(TABLE *altered_table, - HA_CREATE_INFO *create_info, Alter_inplace_info *ha_alter_info, bool commit); @@ -2616,15 +2635,14 @@ protected: for a given partition. @param altered_table TABLE object for new version of table. - @param create_info Information from the parsing phase about new - table properties. - @param ha_alter_info Structure holding data used during in-place alter. + @param ha_alter_info Structure describing changes to be done + by ALTER TABLE and holding data used + during in-place alter. @retval true Error @retval false Success */ virtual bool prepare_inplace_alter_table(TABLE *altered_table, - HA_CREATE_INFO *create_info, Alter_inplace_info *ha_alter_info) { return false; } @@ -2641,16 +2659,14 @@ protected: will be called with commit= false. @param altered_table TABLE object for new version of table. - @param create_info Information from the parsing phase about new - table properties. - @param ha_alter_info Structure holding data used during in-place alter. - @param alter_flags Bitmask that shows what will be changed. + @param ha_alter_info Structure describing changes to be done + by ALTER TABLE and holding data used + during in-place alter. @retval true Error @retval false Success */ virtual bool inplace_alter_table(TABLE *altered_table, - HA_CREATE_INFO *create_info, Alter_inplace_info *ha_alter_info); @@ -2674,17 +2690,15 @@ protected: @see prepare_inplace_alter_table(). @param altered_table TABLE object for new version of table. - @param create_info Information from the parsing phase about new - table properties. - @param ha_alter_info Structure holding data used during in-place alter. - @param alter_flags Bitmask that shows what will be changed. + @param ha_alter_info Structure describing changes to be done + by ALTER TABLE and holding data used + during in-place alter. @param commit True => Commit, False => Rollback. @retval true Error @retval false Success */ virtual bool commit_inplace_alter_table(TABLE *altered_table, - HA_CREATE_INFO *create_info, Alter_inplace_info *ha_alter_info, bool commit); === modified file 'sql/sql_table.cc' --- a/sql/sql_table.cc 2012-01-31 14:35:18 +0000 +++ b/sql/sql_table.cc 2012-02-01 07:31:47 +0000 @@ -5082,14 +5082,16 @@ static int compare_uint(const uint *s, c Compare original and new versions of a table and fill Alter_inplace_info describing differences between those versions. - @param thd Thread - @param table The original table. - @param alter_info Alter options, fields and keys for the new - table. - @param create_info Create options for the new table. - @param varchar Indicates that new definition has new - VARCHAR column. - @param[out] ha_alter_info Data structures needed for in-place alter + @param thd Thread + @param table The original table. + @param varchar Indicates that new definition has new + VARCHAR column. + @param[in/out] ha_alter_info Data structure which already contains + basic information about create options, + field and keys for the new version of + table and which should be completed with + more detailed information needed for + in-place ALTER. First argument 'table' contains information of the original table, which includes all corresponding parts that the new @@ -5120,8 +5122,6 @@ static int compare_uint(const uint *s, c static bool fill_alter_inplace_info(THD *thd, TABLE *table, - Alter_info *alter_info, - HA_CREATE_INFO *create_info, bool varchar, Alter_inplace_info *ha_alter_info) { @@ -5131,6 +5131,7 @@ static bool fill_alter_inplace_info(THD KEY_PART_INFO *key_part, *new_part; KEY_PART_INFO *end; uint candidate_key_count= 0; + Alter_info *alter_info= ha_alter_info->alter_info; DBUG_ENTER("fill_alter_inplace_info"); /* Allocate result buffers. */ @@ -5782,11 +5783,9 @@ static bool is_inplace_alter_impossible( @param table_list TABLE_LIST for the table to change. @param table The original TABLE. @param altered_table TABLE object for new version of the table. - @param create_info Information from the parsing phase about new - table properties. - @param alter_info Data related to detected changes. - @param ha_alter_flags Bitmask that shows what will be changed. - @param ha_alter_info Storage place for data used during different phases. + @param ha_alter_info Structure describing ALTER TABLE to be carried + out and serving as a storage place for data + used during different phases. @param inplace_supported Enum describing the locking requirements. @param target_mdl_request Metadata request/lock on the target table name. @param alter_ctx ALTER TABLE runtime context. @@ -5811,8 +5810,6 @@ static bool mysql_inplace_alter_table(TH TABLE_LIST *table_list, TABLE *table, TABLE *altered_table, - HA_CREATE_INFO *create_info, - Alter_info *alter_info, Alter_inplace_info *ha_alter_info, enum_alter_inplace_result inplace_supported, MDL_request *target_mdl_request, @@ -5821,6 +5818,8 @@ static bool mysql_inplace_alter_table(TH Open_table_context ot_ctx(thd, MYSQL_OPEN_REOPEN); handlerton *db_type= table->s->db_type(); MDL_ticket *mdl_ticket= table->mdl_ticket; + HA_CREATE_INFO *create_info= ha_alter_info->create_info; + Alter_info *alter_info= ha_alter_info->alter_info; DBUG_ENTER("mysql_inplace_alter_table"); @@ -5860,7 +5859,6 @@ static bool mysql_inplace_alter_table(TH DEBUG_SYNC(thd, "alter_table_inplace_after_lock_upgrade"); if (table->file->ha_prepare_inplace_alter_table(altered_table, - create_info, ha_alter_info)) { goto rollback; @@ -5885,7 +5883,6 @@ static bool mysql_inplace_alter_table(TH DEBUG_SYNC(thd, "alter_table_inplace_after_lock_downgrade"); if (table->file->ha_inplace_alter_table(altered_table, - create_info, ha_alter_info)) { goto rollback; @@ -5897,7 +5894,6 @@ static bool mysql_inplace_alter_table(TH DBUG_EXECUTE_IF("alter_table_rollback_new_index", { table->file->ha_commit_inplace_alter_table(altered_table, - create_info, ha_alter_info, false); my_error(ER_UNKNOWN_ERROR, MYF(0)); @@ -5907,7 +5903,6 @@ static bool mysql_inplace_alter_table(TH DEBUG_SYNC(thd, "alter_table_inplace_before_commit"); if (table->file->ha_commit_inplace_alter_table(altered_table, - create_info, ha_alter_info, true)) { @@ -5989,7 +5984,6 @@ static bool mysql_inplace_alter_table(TH rollback: table->file->ha_commit_inplace_alter_table(altered_table, - create_info, ha_alter_info, false); cleanup: @@ -7049,13 +7043,13 @@ bool mysql_alter_table(THD *thd,char *ne if (alter_info->requested_algorithm != Alter_info::ALTER_TABLE_ALGORITHM_COPY) { - Alter_inplace_info ha_alter_info(ignore, key_info, key_count); + Alter_inplace_info ha_alter_info(create_info, alter_info, + key_info, key_count, ignore); TABLE *altered_table= NULL; bool use_inplace= true; /* Fill the Alter_inplace_info structure. */ - if (fill_alter_inplace_info(thd, table, alter_info, create_info, - varchar, &ha_alter_info)) + if (fill_alter_inplace_info(thd, table, varchar, &ha_alter_info)) goto err_new_table_cleanup; // We assume that the table is non-temporary. @@ -7088,8 +7082,6 @@ bool mysql_alter_table(THD *thd,char *ne // Ask storage engine whether to use copy or in-place enum_alter_inplace_result inplace_supported= table->file->check_if_supported_inplace_alter(altered_table, - create_info, - alter_info, &ha_alter_info); switch (inplace_supported) { @@ -7148,7 +7140,6 @@ bool mysql_alter_table(THD *thd,char *ne { if (mysql_inplace_alter_table(thd, table_list, table, altered_table, - create_info, alter_info, &ha_alter_info, inplace_supported, &target_mdl_request, &alter_ctx)) No bundle (reason: useless for push emails).