From: Dmitry Lenev Date: January 30 2012 8:40am Subject: bzr push into mysql-trunk-wl5534 branch (Dmitry.Lenev:3472 to 3473) WL#5534 List-Archive: http://lists.mysql.com/commits/142623 Message-Id: <20120130084032.16AC84206CF@jubjub> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 3473 Dmitry Lenev 2012-01-30 WL#5534 Online ALTER, Phase 1. Patch #80: Replaced hack which was used to obtain path to temporary table with a nicer approach. modified: sql/sql_alter.cc sql/sql_alter.h sql/sql_base.cc sql/sql_base.h sql/sql_insert.cc sql/sql_partition.cc sql/sql_partition.h sql/sql_table.cc sql/sql_table.h 3472 Dmitry Lenev 2012-01-27 WL#5534 Online ALTER, Phase 1. Patch #79: Improved some comments. modified: sql/mdl.cc sql/sql_table.cc === modified file 'sql/sql_alter.cc' --- a/sql/sql_alter.cc 2012-01-25 16:40:54 +0000 +++ b/sql/sql_alter.cc 2012-01-30 08:37:12 +0000 @@ -86,6 +86,9 @@ Alter_table_ctx::Alter_table_ctx() tables_opened(0), db(NULL), table_name(NULL), alias(NULL), new_db(NULL), new_name(NULL), new_alias(NULL) +#ifndef DBUG_OFF + , tmp_table(false) +#endif { } @@ -96,6 +99,9 @@ Alter_table_ctx::Alter_table_ctx(THD *th : datetime_field(NULL), error_if_not_empty(false), tables_opened(tables_opened_arg), new_db(new_db_arg), new_name(new_name_arg) +#ifndef DBUG_OFF + , tmp_table(false) +#endif { /* Assign members db, table_name, new_db and new_name @@ -149,15 +155,30 @@ Alter_table_ctx::Alter_table_ctx(THD *th if (lower_case_table_names) my_casedn_str(files_charset_info, tmp_name); - build_table_filename(path, sizeof(path) - 1, db, table_name, "", 0); + if (table_list->table->s->tmp_table == NO_TMP_TABLE) + { + 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_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(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); + build_table_filename(tmp_path, sizeof(tmp_path) - 1, new_db, tmp_name, "", + FN_IS_TMP); + } + else + { + /* + We are not filling path, new_path and new_filename members if + we are altering temporary table as these members are not used in + this case. This fact is enforced with assert. + */ + build_tmptable_filename(thd, tmp_path, sizeof(tmp_path)); +#ifndef DBUG_OFF + tmp_table= true; +#endif + } } === modified file 'sql/sql_alter.h' --- a/sql/sql_alter.h 2012-01-20 03:54:02 +0000 +++ b/sql/sql_alter.h 2012-01-30 08:37:12 +0000 @@ -281,19 +281,28 @@ public: @return filename (including .frm) for the new table. */ const char *get_new_filename() const - { return new_filename; } + { + DBUG_ASSERT(!tmp_table); + return new_filename; + } /** @return path to the original table. */ const char *get_path() const - { return path; } + { + DBUG_ASSERT(!tmp_table); + return path; + } /** @return path to the new table. */ const char *get_new_path() const - { return new_path; } + { + DBUG_ASSERT(!tmp_table); + return new_path; + } /** @return path to the temporary table created during ALTER TABLE. @@ -320,6 +329,11 @@ private: char new_path[FN_REFLEN + 1]; char tmp_path[FN_REFLEN + 1]; +#ifndef DBUG_OFF + /** Indicates that we are altering temporary table. Used only in asserts. */ + bool tmp_table; +#endif + Alter_table_ctx &operator=(const Alter_table_ctx &rhs); // not implemented Alter_table_ctx(const Alter_table_ctx &rhs); // not implemented }; === modified file 'sql/sql_base.cc' --- a/sql/sql_base.cc 2012-01-19 09:57:36 +0000 +++ b/sql/sql_base.cc 2012-01-30 08:37:12 +0000 @@ -6137,17 +6137,27 @@ TABLE *open_table_uncached(THD *thd, con } -bool rm_temporary_table(handlerton *base, char *path) +/** + Delete a temporary table. + + @param base Handlerton for table to be deleted. + @param path Path to the table to be deleted (i.e. path + to its .frm without an extension). + + @retval false - success. + @retval true - failure. +*/ + +bool rm_temporary_table(handlerton *base, const char *path) { bool error=0; handler *file; - char *ext; + char frm_path[FN_REFLEN + 1]; DBUG_ENTER("rm_temporary_table"); - strmov(ext= strend(path), reg_ext); - if (mysql_file_delete(key_file_frm, path, MYF(0))) + strxnmov(frm_path, sizeof(frm_path) - 1, path, reg_ext, NullS); + if (mysql_file_delete(key_file_frm, frm_path, MYF(0))) error=1; /* purecov: inspected */ - *ext= 0; // remove extension file= get_new_handler((TABLE_SHARE*) 0, current_thd->mem_root, base); if (file && file->ha_delete_table(path)) { === modified file 'sql/sql_base.h' --- a/sql/sql_base.h 2011-12-19 14:38:42 +0000 +++ b/sql/sql_base.h 2012-01-30 08:37:12 +0000 @@ -158,7 +158,7 @@ thr_lock_type read_lock_type_for_table(T TABLE_LIST *table_list); my_bool mysql_rm_tmp_tables(void); -bool rm_temporary_table(handlerton *base, char *path); +bool rm_temporary_table(handlerton *base, const char *path); void close_tables_for_reopen(THD *thd, TABLE_LIST **tables, const MDL_savepoint &start_of_statement_svp); TABLE_LIST *find_table_in_list(TABLE_LIST *table, === modified file 'sql/sql_insert.cc' --- a/sql/sql_insert.cc 2012-01-26 12:23:27 +0000 +++ b/sql/sql_insert.cc 2012-01-30 08:37:12 +0000 @@ -3792,8 +3792,8 @@ static TABLE *create_table_from_items(TH { if (!mysql_create_table_no_lock(thd, create_table->db, create_table->table_name, - create_info, alter_info, 0, - select_field_count, false, NULL)) + create_info, alter_info, + select_field_count, NULL)) { DEBUG_SYNC(thd,"create_table_select_before_open"); === modified file 'sql/sql_partition.cc' --- a/sql/sql_partition.cc 2011-12-21 08:49:47 +0000 +++ b/sql/sql_partition.cc 2012-01-30 08:37:12 +0000 @@ -67,6 +67,7 @@ // mysql_*_alter_copy_data #include "opt_range.h" // store_key_image_to_rec #include "sql_analyse.h" // append_escaped +#include "sql_alter.h" // Alter_table_ctx #include using std::max; @@ -4658,6 +4659,7 @@ 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] alter_ctx ALTER TABLE runtime context @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 @@ -4678,10 +4680,8 @@ bool compare_partition_options(HA_CREATE uint prep_alter_part_table(THD *thd, TABLE *table, Alter_info *alter_info, HA_CREATE_INFO *create_info, + Alter_table_ctx *alter_ctx, bool *partition_changed, - char *db, - const char *table_name, - const char *path, TABLE **fast_alter_table) { TABLE *new_table= NULL; @@ -4739,9 +4739,13 @@ uint prep_alter_part_table(THD *thd, TAB Open it as a copy of the original table, and modify its partition_info object to allow fast_alter_partition_table to perform the changes. */ - DBUG_ASSERT(thd->mdl_context.is_lock_owner(MDL_key::TABLE, db, table_name, + DBUG_ASSERT(thd->mdl_context.is_lock_owner(MDL_key::TABLE, + alter_ctx->db, + alter_ctx->table_name, MDL_INTENTION_EXCLUSIVE)); - new_table= open_table_uncached(thd, path, db, table_name, false, true); + new_table= open_table_uncached(thd, alter_ctx->get_path(), + alter_ctx->db, alter_ctx->table_name, + false, true); if (!new_table) DBUG_RETURN(TRUE); === modified file 'sql/sql_partition.h' --- a/sql/sql_partition.h 2011-12-21 08:49:47 +0000 +++ b/sql/sql_partition.h 2012-01-30 08:37:12 +0000 @@ -20,6 +20,7 @@ #include "table.h" /* TABLE_LIST */ class Alter_info; +class Alter_table_ctx; class Field; class String; class handler; @@ -254,10 +255,8 @@ bool 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, + Alter_table_ctx *alter_ctx, bool *partition_changed, - char *db, - const char *table_name, - const char *path, TABLE **fast_alter_table); char *generate_partition_syntax(partition_info *part_info, uint *buf_length, bool use_sql_alloc, === modified file 'sql/sql_table.cc' --- a/sql/sql_table.cc 2012-01-26 20:25:39 +0000 +++ b/sql/sql_table.cc 2012-01-30 08:37:12 +0000 @@ -580,22 +580,19 @@ uint build_table_filename(char *buff, si } -/* - Creates path to a file: mysql_tmpdir/#sql1234_12_1.ext - - SYNOPSIS - build_tmptable_filename() - thd The thread handle. - buff Where to write result in my_charset_filename. - bufflen buff size +/** + Create path to a temporary table mysql_tmpdir/#sql1234_12_1 + (i.e. to its .FRM file but without an extension). - NOTES + @param thd The thread handle. + @param buff Where to write result in my_charset_filename. + @param bufflen buff size + @note Uses current_pid, thread_id, and tmp_table counter to create a file name in mysql_tmpdir. - RETURN - path length + @return Path length. */ uint build_tmptable_filename(THD* thd, char *buff, size_t bufflen) @@ -603,9 +600,9 @@ uint build_tmptable_filename(THD* thd, c DBUG_ENTER("build_tmptable_filename"); char *p= strnmov(buff, mysql_tmpdir, bufflen); - my_snprintf(p, bufflen - (p - buff), "/%s%lx_%lx_%x%s", + my_snprintf(p, bufflen - (p - buff), "/%s%lx_%lx_%x", tmp_file_prefix, current_pid, - thd->thread_id, thd->tmp_table++, reg_ext); + thd->thread_id, thd->tmp_table++); if (lower_case_table_names) { @@ -4055,6 +4052,8 @@ void sp_prepare_create_field(THD *thd, C @param thd Thread object @param db Database @param table_name Table name + @param path Path to table (i.e. to its .FRM file without + the extension). @param create_info Create information (like MAX_ROWS) @param alter_info Description of fields and keys for new table @param internal_tmp_table Set to true if this is an internal temporary table @@ -4079,23 +4078,23 @@ void sp_prepare_create_field(THD *thd, C @retval true error */ -bool mysql_create_table_no_lock(THD *thd, - const char *db, const char *table_name, - HA_CREATE_INFO *create_info, - Alter_info *alter_info, - bool internal_tmp_table, - uint select_field_count, - bool no_ha_table, - bool *is_trans) +static +bool create_table_impl(THD *thd, + const char *db, const char *table_name, + const char *path, + HA_CREATE_INFO *create_info, + Alter_info *alter_info, + bool internal_tmp_table, + uint select_field_count, + bool no_ha_table, + bool *is_trans) { - char path[FN_REFLEN + 1]; - uint path_length; const char *alias; uint db_options, key_count; KEY *key_info_buffer; handler *file; bool error= TRUE; - DBUG_ENTER("mysql_create_table_no_lock"); + DBUG_ENTER("create_table_impl"); DBUG_PRINT("enter", ("db: '%s' table: '%s' tmp: %d", db, table_name, internal_tmp_table)); @@ -4291,17 +4290,8 @@ bool mysql_create_table_no_lock(THD *thd select_field_count)) goto err; - /* Check if table exists */ if (create_info->options & HA_LEX_CREATE_TMP_TABLE) - { - path_length= build_tmptable_filename(thd, path, sizeof(path)); create_info->table_options|=HA_CREATE_DELAY_KEY_WRITE; - } - else - { - path_length= build_table_filename(path, sizeof(path) - 1, db, alias, reg_ext, - internal_tmp_table ? FN_IS_TMP : 0); - } /* Check if table already exists */ if ((create_info->options & HA_LEX_CREATE_TMP_TABLE) && @@ -4321,7 +4311,10 @@ bool mysql_create_table_no_lock(THD *thd if (!internal_tmp_table && !(create_info->options & HA_LEX_CREATE_TMP_TABLE)) { - if (!access(path,F_OK)) + char frm_name[FN_REFLEN+1]; + strxnmov(frm_name, sizeof(frm_name) - 1, path, reg_ext, NullS); + + if (!access(frm_name, F_OK)) { if (create_info->options & HA_LEX_CREATE_IF_NOT_EXISTS) goto warn; @@ -4445,8 +4438,6 @@ bool mysql_create_table_no_lock(THD *thd } create_info->table_options=db_options; - path[path_length - reg_ext_length]= '\0'; // Remove .frm extension - /* Create .FRM (and .PAR file for partitioned table). If "no_ha_table" is false also create table in storage engine. @@ -4504,8 +4495,8 @@ bool mysql_create_table_no_lock(THD *thd if (result) { - char frm_name[FN_REFLEN]; - strxmov(frm_name, path, reg_ext, NullS); + char frm_name[FN_REFLEN + 1]; + strxnmov(frm_name, sizeof(frm_name) - 1, path, reg_ext, NullS); (void) mysql_file_delete(key_file_frm, frm_name, MYF(0)); (void) file->ha_create_handler_files(path, NULL, CHF_DELETE_FLAG, create_info); @@ -4530,6 +4521,29 @@ warn: /** + Simple wrapper around create_table_impl() to be used + in various version of CREATE TABLE statement. +*/ +bool mysql_create_table_no_lock(THD *thd, + const char *db, const char *table_name, + HA_CREATE_INFO *create_info, + Alter_info *alter_info, + uint select_field_count, + bool *is_trans) +{ + char path[FN_REFLEN + 1]; + + if (create_info->options & HA_LEX_CREATE_TMP_TABLE) + build_tmptable_filename(thd, path, sizeof(path)); + else + build_table_filename(path, sizeof(path) - 1, db, table_name, "", 0); + + return create_table_impl(thd, db, table_name, path, create_info, alter_info, + false, select_field_count, false, is_trans); +} + + +/** Implementation of SQLCOM_CREATE_TABLE. Take the metadata locks (including a shared lock on the affected @@ -4561,7 +4575,7 @@ bool mysql_create_table(THD *thd, TABLE_ result= mysql_create_table_no_lock(thd, create_table->db, create_table->table_name, create_info, - alter_info, false, 0, false, &is_trans); + alter_info, 0, &is_trans); /* Don't write statement if: - Table creation has failed @@ -4824,7 +4838,7 @@ bool mysql_create_like_table(THD* thd, T if ((res= mysql_create_table_no_lock(thd, table->db, table->table_name, &local_create_info, &local_alter_info, - false, 0, false, &is_trans))) + 0, &is_trans))) goto err; /* @@ -5551,9 +5565,9 @@ bool mysql_compare_tables(TABLE *table, /* mysql_prepare_alter_table() clears HA_OPTION_PACK_RECORD bit when preparing description of existing table. In ALTER TABLE it is later - updated to correct value by mysql_create_table_no_lock() call. + updated to correct value by create_table_impl() call. So to get correct value of this bit in this function we have to - mimic behavior of mysql_create_table_no_lock(). + mimic behavior of create_table_impl(). */ if (create_info->row_type == ROW_TYPE_DYNAMIC || (tmp_new_field->flags & BLOB_FLAG) || @@ -6847,8 +6861,7 @@ bool mysql_alter_table(THD *thd,char *ne TABLE *table_for_fast_alter_partition= NULL; { if (prep_alter_part_table(thd, table, alter_info, create_info, - &partition_changed, alter_ctx.db, - alter_ctx.table_name, alter_ctx.get_path(), + &alter_ctx, &partition_changed, &table_for_fast_alter_partition)) { /* @@ -7024,7 +7037,7 @@ bool mysql_alter_table(THD *thd,char *ne TODO/FIXME: This can be avoided in future if we change the code to pass structures built during .FRM creation (such as a KEY array) - from mysql_create_table_no_lock() to fill_alter_inplace_info() or, + from create_table_impl() to fill_alter_inplace_info() or, alternatively, will change the in-place SE API to accept structures which are constructed during opening new .FRM version instead (unbelievable, but the former and the latter have significant @@ -7034,9 +7047,10 @@ 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, alter_ctx.new_db, alter_ctx.tmp_name, - create_info, alter_info, - true, 0, true, NULL); + error= create_table_impl(thd, alter_ctx.new_db, alter_ctx.tmp_name, + alter_ctx.get_tmp_path(), + create_info, alter_info, + true, 0, true, NULL); reenable_binlog(thd); if (error) @@ -7196,25 +7210,8 @@ bool mysql_alter_table(THD *thd,char *ne goto err_new_table_cleanup; { - char path[FN_REFLEN + 1]; - uint path_length; - if (create_info->options & HA_LEX_CREATE_TMP_TABLE) - { - /* - TODO/FIXME: The below is ugly hack. To be fixed by - generating name of temporary table once. - */ - thd->tmp_table--; - path_length= build_tmptable_filename(thd, path, sizeof(path) - 1); - } - else - 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, alter_ctx.new_db, alter_ctx.tmp_name, + if (ha_create_table(thd, alter_ctx.get_tmp_path(), + alter_ctx.new_db, alter_ctx.tmp_name, create_info, false)) goto err_new_table_cleanup; @@ -7223,7 +7220,8 @@ bool mysql_alter_table(THD *thd,char *ne if (create_info->options & HA_LEX_CREATE_TMP_TABLE) { - if (!open_table_uncached(thd, path, alter_ctx.new_db, alter_ctx.tmp_name, + if (!open_table_uncached(thd, alter_ctx.get_tmp_path(), + alter_ctx.new_db, alter_ctx.tmp_name, true, true)) goto err_new_table_cleanup; } === modified file 'sql/sql_table.h' --- a/sql/sql_table.h 2012-01-25 16:40:54 +0000 +++ b/sql/sql_table.h 2012-01-30 08:37:12 +0000 @@ -148,6 +148,7 @@ uint build_table_filename(char *buff, si const char *table, const char *ext, uint flags); uint build_table_shadow_filename(char *buff, size_t bufflen, ALTER_PARTITION_PARAM_TYPE *lpt); +uint build_tmptable_filename(THD* thd, char *buff, size_t bufflen); bool mysql_create_table(THD *thd, TABLE_LIST *create_table, HA_CREATE_INFO *create_info, Alter_info *alter_info); @@ -155,8 +156,7 @@ bool mysql_create_table_no_lock(THD *thd const char *table_name, HA_CREATE_INFO *create_info, Alter_info *alter_info, - bool tmp_table, uint select_field_count, - bool no_ha_table, + uint select_field_count, bool *is_trans); int mysql_discard_or_import_tablespace(THD *thd, TABLE_LIST *table_list, No bundle (reason: useless for push emails).