From: Date: December 29 2008 6:51pm
Subject: bzr commit into mysql-5.1-bugteam branch (mattias.jonsson:2742)
 Bug#30102
List-Archive: http://lists.mysql.com/commits/62429
X-Bug: 30102
Message-Id: <20081229175126.F25111A4A9FA@witty.localhost>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7BIT

#At file:///Users/mattiasj/clones/bzrroot/b30102-51-bugteam/ based on revid:sergey.glukhov@stripped

 2742 Mattias Jonsson	2008-12-29
      Bug#30102: Rename table does corrupt tables with partition files on failure
      
      Problem was that a failing rename just left the partitions at the state
      it was at the failure.
      
      Solution was to try to revert the started rename if a failure occured.
added:
  mysql-test/r/partition_not_embedded.result
  mysql-test/t/partition_not_embedded.test
modified:
  sql/ha_partition.cc
  sql/handler.cc

per-file messages:
  mysql-test/r/partition_not_embedded.result
    Bug#30102: Rename table does corrupt tables with partition files on failure
    
    New result file
  mysql-test/t/partition_not_embedded.test
    Bug#30102: Rename table does corrupt tables with partition files on failure
    
    New test file
    (list_files does not report the files in embedded)
  sql/ha_partition.cc
    Bug#30102: Rename table does corrupt tables with partition files on failure
    
    Better error handling for rename partitions (reverting the started rename
    operation)
    
    Different order of files for delete.
  sql/handler.cc
    Bug#30102: Rename table does corrupt tables with partition files on failure
    
    Tries to remove as many table files as possible
    if the first delete succeeds.
=== added file 'mysql-test/r/partition_not_embedded.result'
--- a/mysql-test/r/partition_not_embedded.result	1970-01-01 00:00:00 +0000
+++ b/mysql-test/r/partition_not_embedded.result	2008-12-29 17:51:15 +0000
@@ -0,0 +1,107 @@
+DROP TABLE IF EXISTS t1, t2;
+# Bug#30102 test
+CREATE TABLE t1 (a INT)
+PARTITION BY RANGE (a)
+(PARTITION p0 VALUES LESS THAN (6),
+PARTITION `p1....................` VALUES LESS THAN (9),
+PARTITION p2 VALUES LESS THAN MAXVALUE);
+# List of files in database `test`, all original t1-files here
+t1#P#p0.MYD
+t1#P#p0.MYI
+t1#P#p1@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@stripped
+t1#P#p1@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@stripped
+t1#P#p2.MYD
+t1#P#p2.MYI
+t1.frm
+t1.par
+INSERT INTO t1 VALUES (1), (2), (3), (4), (5), (6), (7), (8), (9), (10);
+# Renaming to a file name where the first partition is 250 chars
+# and the second partition is 350 chars
+RENAME TABLE t1 TO `t2_new..............................................end`;
+Got one of the listed errors
+# List of files in database `test`, should not be any t2-files here
+# List of files in database `test`, should be all t1-files here
+t1#P#p0.MYD
+t1#P#p0.MYI
+t1#P#p1@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@stripped
+t1#P#p1@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@stripped
+t1#P#p2.MYD
+t1#P#p2.MYI
+t1.frm
+t1.par
+SELECT * FROM t1;
+a
+1
+10
+2
+3
+4
+5
+6
+7
+8
+9
+# Renaming to a file name where the first partition is 155 chars
+# and the second partition is 255 chars
+RENAME TABLE t1 TO `t2_............................end`;
+# List of files in database `test`, should not be any t1-files here
+# List of files in database `test`, should be all t2-files here
+t2_@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002eend#P#p0.MYD
+t2_@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002eend#P#p0.MYI
+t2_@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002eend#P#p1@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@stripped
+t2_@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002eend#P#p1@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@stripped
+t2_@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002eend#P#p2.MYD
+t2_@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002eend#P#p2.MYI
+t2_@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@stripped
+t2_@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@stripped
+SELECT * FROM `t2_............................end`;
+a
+1
+10
+2
+3
+4
+5
+6
+7
+8
+9
+RENAME TABLE `t2_............................end` to t1;
+# List of files in database `test`, should be all t1-files here
+t1#P#p0.MYD
+t1#P#p0.MYI
+t1#P#p1@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@stripped
+t1#P#p1@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@stripped
+t1#P#p2.MYD
+t1#P#p2.MYI
+t1.frm
+t1.par
+# Renaming to a file name where the first partition is 156 chars
+# and the second partition is 256 chars
+RENAME TABLE t1 TO `t2_............................_end`;
+Got one of the listed errors
+# List of files in database `test`, should not be any t2-files here
+# List of files in database `test`, should be all t1-files here
+t1#P#p0.MYD
+t1#P#p0.MYI
+t1#P#p1@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@stripped
+t1#P#p1@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@stripped
+t1#P#p2.MYD
+t1#P#p2.MYI
+t1.frm
+t1.par
+SELECT * FROM t1;
+a
+1
+10
+2
+3
+4
+5
+6
+7
+8
+9
+DROP TABLE t1;
+# Should not be any files left here
+# End of bug#30102 test.

=== added file 'mysql-test/t/partition_not_embedded.test'
--- a/mysql-test/t/partition_not_embedded.test	1970-01-01 00:00:00 +0000
+++ b/mysql-test/t/partition_not_embedded.test	2008-12-29 17:51:15 +0000
@@ -0,0 +1,69 @@
+-- source include/have_partition.inc
+-- source include/not_embedded.inc
+--disable_warnings
+DROP TABLE IF EXISTS t1, t2;
+--enable_warnings
+let $MYSQLD_DATADIR= `SELECT @@datadir`;
+
+#
+# Bug#30102: rename table does corrupt tables with partition files on failure
+#
+--echo # Bug#30102 test
+CREATE TABLE t1 (a INT)
+PARTITION BY RANGE (a)
+(PARTITION p0 VALUES LESS THAN (6),
+ PARTITION `p1....................` VALUES LESS THAN (9),
+ PARTITION p2 VALUES LESS THAN MAXVALUE);
+# partition p1 is 't1#P#p1' + @002e * 20 = 107 characters + file ending
+# total path lenght of './test/t1#P#p1@002e@002e<...>@002e.MY[ID]' is 118 chars
+--echo # List of files in database `test`, all original t1-files here
+--list_files $MYSQLD_DATADIR/test t1*
+INSERT INTO t1 VALUES (1), (2), (3), (4), (5), (6), (7), (8), (9), (10);
+--echo # Renaming to a file name where the first partition is 250 chars
+--echo # and the second partition is 350 chars
+# 7,7 avoids the error message, which is not deterministic.
+--error 7,7
+RENAME TABLE t1 TO `t2_new..............................................end`;
+#                         1234567890123456789012345678901234567890123456
+--echo # List of files in database `test`, should not be any t2-files here
+--list_files $MYSQLD_DATADIR/test t2*
+--echo # List of files in database `test`, should be all t1-files here
+--list_files $MYSQLD_DATADIR/test t1*
+--sorted_result
+SELECT * FROM t1;
+--echo # Renaming to a file name where the first partition is 155 chars
+--echo # and the second partition is 255 chars
+RENAME TABLE t1 TO `t2_............................end`;
+#                      1234567890123456789012345678
+# t2_ + end
+#     .MY[ID] or .frm
+#         #P#p[012]
+#             28 * @002e
+# 6 + 4 + 5 + 28 * 5 = 155
+--echo # List of files in database `test`, should not be any t1-files here
+--list_files $MYSQLD_DATADIR/test t1*
+--echo # List of files in database `test`, should be all t2-files here
+--list_files $MYSQLD_DATADIR/test t2*
+--sorted_result
+SELECT * FROM `t2_............................end`;
+RENAME TABLE `t2_............................end` to t1;
+--echo # List of files in database `test`, should be all t1-files here
+--list_files $MYSQLD_DATADIR/test t1*
+--echo # Renaming to a file name where the first partition is 156 chars
+--echo # and the second partition is 256 chars
+# 7,7 avoids the error message, which is not deterministic.
+--error 7,7
+RENAME TABLE t1 TO `t2_............................_end`;
+#                      1234567890123456789012345678
+# 7 + 4 + 5 + 28 * 5 = 16 + 140 = 156
+--echo # List of files in database `test`, should not be any t2-files here
+--list_files $MYSQLD_DATADIR/test t2*
+--echo # List of files in database `test`, should be all t1-files here
+--list_files $MYSQLD_DATADIR/test t1*
+--sorted_result
+SELECT * FROM t1;
+DROP TABLE t1;
+--echo # Should not be any files left here
+--list_files $MYSQLD_DATADIR/test t1*
+--list_files $MYSQLD_DATADIR/test t2*
+--echo # End of bug#30102 test.

=== modified file 'sql/ha_partition.cc'
--- a/sql/ha_partition.cc	2008-12-16 11:44:18 +0000
+++ b/sql/ha_partition.cc	2008-12-29 17:51:15 +0000
@@ -423,12 +423,9 @@ bool ha_partition::initialize_partition(
 
 int ha_partition::delete_table(const char *name)
 {
-  int error;
   DBUG_ENTER("ha_partition::delete_table");
 
-  if ((error= del_ren_cre_table(name, NULL, NULL, NULL)))
-    DBUG_RETURN(error);
-  DBUG_RETURN(handler::delete_table(name));
+  DBUG_RETURN(del_ren_cre_table(name, NULL, NULL, NULL));
 }
 
 
@@ -456,12 +453,9 @@ int ha_partition::delete_table(const cha
 
 int ha_partition::rename_table(const char *from, const char *to)
 {
-  int error;
   DBUG_ENTER("ha_partition::rename_table");
 
-  if ((error= del_ren_cre_table(from, to, NULL, NULL)))
-    DBUG_RETURN(error);
-  DBUG_RETURN(handler::rename_table(from, to));
+  DBUG_RETURN(del_ren_cre_table(from, to, NULL, NULL));
 }
 
 
@@ -1807,6 +1801,15 @@ uint ha_partition::del_ren_cre_table(con
   DBUG_PRINT("enter", ("from: (%s) to: (%s)", from, to));
   name_buffer_ptr= m_name_buffer_ptr;
   file= m_file;
+  if (to == NULL && table_arg == NULL)
+  {
+    /*
+      Delete table, start by delete the .par file. If error, break, otherwise
+      delete as much as possible.
+    */
+    if ((error= handler::delete_table(from)))
+      DBUG_RETURN(error);
+  }
   /*
     Since ha_partition has HA_FILE_BASED, it must alter underlying table names
     if they do not have HA_FILE_BASED and lower_case_table_names == 2.
@@ -1828,6 +1831,8 @@ uint ha_partition::del_ren_cre_table(con
       create_partition_name(to_buff, to_path, name_buffer_ptr,
                             NORMAL_PART_NAME, FALSE);
       error= (*file)->ha_rename_table(from_buff, to_buff);
+      if (error)
+        goto rename_error;
     }
     else if (table_arg == NULL)			// delete branch
       error= (*file)->ha_delete_table(from_buff);
@@ -1843,6 +1848,15 @@ uint ha_partition::del_ren_cre_table(con
       save_error= error;
     i++;
   } while (*(++file));
+  if (to != NULL)
+  {
+    if ((error= handler::rename_table(from, to)))
+    {
+      /* Try to revert everything, ignore errors */
+      (void) handler::rename_table(to, from);
+      goto rename_error;
+    }
+  }
   DBUG_RETURN(save_error);
 create_error:
   name_buffer_ptr= m_name_buffer_ptr;
@@ -1850,7 +1864,21 @@ create_error:
   {
     create_partition_name(from_buff, from_path, name_buffer_ptr, NORMAL_PART_NAME,
                           FALSE);
-    VOID((*file)->ha_delete_table((const char*) from_buff));
+    (void) (*file)->ha_delete_table((const char*) from_buff);
+    name_buffer_ptr= strend(name_buffer_ptr) + 1;
+  }
+  DBUG_RETURN(error);
+rename_error:
+  name_buffer_ptr= m_name_buffer_ptr;
+  for (abort_file= file, file= m_file; file < abort_file; file++)
+  {
+    /* Revert the rename, back from 'to' to the original 'from' */
+    create_partition_name(from_buff, from_path, name_buffer_ptr,
+                          NORMAL_PART_NAME, FALSE);
+    create_partition_name(to_buff, to_path, name_buffer_ptr,
+                          NORMAL_PART_NAME, FALSE);
+    /* Ignore error here */
+    (void) (*file)->ha_rename_table(to_buff, from_buff);
     name_buffer_ptr= strend(name_buffer_ptr) + 1;
   }
   DBUG_RETURN(error);

=== modified file 'sql/handler.cc'
--- a/sql/handler.cc	2008-11-25 09:55:30 +0000
+++ b/sql/handler.cc	2008-12-29 17:51:15 +0000
@@ -2943,6 +2943,7 @@ uint handler::get_dup_key(int error)
 */
 int handler::delete_table(const char *name)
 {
+  int saved_error= 0;
   int error= 0;
   int enoent_or_zero= ENOENT;                   // Error if no file was deleted
   char buff[FN_REFLEN];
@@ -2952,21 +2953,31 @@ int handler::delete_table(const char *na
     fn_format(buff, name, "", *ext, MY_UNPACK_FILENAME|MY_APPEND_EXT);
     if (my_delete_with_symlink(buff, MYF(0)))
     {
-      if ((error= my_errno) != ENOENT)
-	break;
+      if (my_errno != ENOENT)
+      {
+        /*
+          If error on the first existing file, return the error.
+          Otherwise delete as much as possible.
+        */
+        if (enoent_or_zero)
+          return my_errno;
+	saved_error= my_errno;
+      }
     }
     else
       enoent_or_zero= 0;                        // No error for ENOENT
     error= enoent_or_zero;
   }
-  return error;
+  return saved_error ? saved_error : error;
 }
 
 
 int handler::rename_table(const char * from, const char * to)
 {
   int error= 0;
-  for (const char **ext= bas_ext(); *ext ; ext++)
+  const char **ext, **start_ext;
+  start_ext= bas_ext();
+  for (ext= start_ext; *ext ; ext++)
   {
     if (rename_file_ext(from, to, *ext))
     {
@@ -2975,6 +2986,12 @@ int handler::rename_table(const char * f
       error= 0;
     }
   }
+  if (error)
+  {
+    /* Try to revert the rename. Ignore errors. */
+    for (; ext >= start_ext; ext--)
+      rename_file_ext(to, from, *ext);
+  }
   return error;
 }