List:Commits« Previous MessageNext Message »
From:Dmitry Lenev Date:January 30 2012 8:40am
Subject:bzr push into mysql-trunk-wl5534 branch (Dmitry.Lenev:3472 to 3473) WL#5534
View as plain text  
 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 <algorithm>
 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).
Thread
bzr push into mysql-trunk-wl5534 branch (Dmitry.Lenev:3472 to 3473) WL#5534Dmitry Lenev30 Jan