MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:ahristov Date:November 29 2006 10:46am
Subject:bk commit into 5.1 tree (andrey:1.2386) BUG#22369
View as plain text  
Below is the list of changes that have just been committed into a local
5.1 repository of andrey. When andrey does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet@stripped, 2006-11-29 11:46:28+01:00, andrey@stripped +5 -0
  Update mysql_fix_privilege_tables.sql to handle upgrade from 5.0 while retaining
  old upgrade behaviour.
  Add test for upgrade from 5.0.30
  ---
  Fix for bug#22369: Alter table rename combined
  with other alterations causes lost tables
  
  SE - Storage Engine
  
  Using RENAME clause combined with other clauses of ALTER TABLE leaded to
  data loss (the data was there but not accessible). This could happen if the
  changes do not change the table much. Adding and droppping fields and
  indices was safe. Renaming a column with MODIFY or CHANGE was unsafe operation,
  if the actual column didn't change (changing from int to int, which is a noop)
    
  Depending on the storage the behavior is different:
  1)MyISAM/MEMORY - the ALTER TABLE statement completes
    without any error but next SELECT against the new table fails.
  2)InnoDB (and every other transactional table) - The ALTER TABLE statement
    fails. There are the the following files in the db dir -
    `new_table_name.frm` and a temporary table's frm. If the SE is file
    based, then the data and index files will be present but with the old
    names. What happens is that for InnoDB the table is not renamed in the
    internal DDIC.
  
  Fixed by adding additional call to SE's rename method, which should not include
  FRM file rename, because it firstly done during file names juggling.

  mysql-test/r/alter_table.result@stripped, 2006-11-29 11:46:22+01:00, andrey@stripped +42 -0
    update result file

  mysql-test/t/alter_table.test@stripped, 2006-11-29 11:46:23+01:00, andrey@stripped +44 -9
    Error to bug number
        
    Added test case for #22369: Alter table rename combined
    with other alterations causes lost tables

  sql/mysql_priv.h@stripped, 2006-11-29 11:46:23+01:00, andrey@stripped +1 -0
    Add a new flag for mysql_rename_table() and allign properly

  sql/sql_parse.cc@stripped, 2006-11-29 11:46:23+01:00, andrey@stripped +5 -2
    To be consistent with SQLCOM_RENAME_TABLE, SQLCOM_ALTER_TABLE has
    to check for DROP_ACL if there is ALTER_RENAME flag set.

  sql/sql_table.cc@stripped, 2006-11-29 11:46:24+01:00, andrey@stripped +79 -16
    ALTER_RENAME, the data and index files weren't renamed in the engine
    but only the FRM was new, when the tables old and new tables are compatible.
    In the chain of FRM renames we add a call to mysql_rename_table() which should
    instruct the engine to rename the table but not rename the FRM.
    This bug was there only in 5.1 branch. 4.1 and 5.0 always do copy data on RENAME
    if there are more clauses than just rename.

# This is a BitKeeper patch.  What follows are the unified diffs for the
# set of deltas contained in the patch.  The rest of the patch, the part
# that BitKeeper cares about, is below these diffs.
# User:	andrey
# Host:	example.com
# Root:	/work/bug22369/my51

--- 1.454/sql/mysql_priv.h	2006-11-29 11:46:42 +01:00
+++ 1.455/sql/mysql_priv.h	2006-11-29 11:46:42 +01:00
@@ -1831,6 +1831,7 @@ uint build_table_filename(char *buff, si
 #define FN_FROM_IS_TMP  (1 << 0)
 #define FN_TO_IS_TMP    (1 << 1)
 #define FN_IS_TMP       (FN_FROM_IS_TMP | FN_TO_IS_TMP)
+#define NO_FRM_RENAME   (1 << 2)
 
 /* from hostname.cc */
 struct in_addr;

--- 1.597/sql/sql_parse.cc	2006-11-29 11:46:42 +01:00
+++ 1.598/sql/sql_parse.cc	2006-11-29 11:46:42 +01:00
@@ -3085,8 +3085,11 @@ end_with_restore_list:
     {
       ulong priv=0;
       ulong priv_needed= ALTER_ACL;
-      /* We also require DROP priv for ALTER TABLE ... DROP PARTITION */
-      if (lex->alter_info.flags & ALTER_DROP_PARTITION)
+      /*
+        We also require DROP priv for ALTER TABLE ... DROP PARTITION, as well
+        as for RENAME TO, as being done by SQLCOM_RENAME_TABLE
+      */
+      if (lex->alter_info.flags & (ALTER_DROP_PARTITION | ALTER_RENAME))
         priv_needed|= DROP_ACL;
 
       if (lex->name && (!lex->name[0] || strlen(lex->name) > NAME_LEN))

--- 1.374/sql/sql_table.cc	2006-11-29 11:46:42 +01:00
+++ 1.375/sql/sql_table.cc	2006-11-29 11:46:42 +01:00
@@ -3652,10 +3652,12 @@ make_unique_key_name(const char *field_n
       flags                     flags for build_table_filename().
                                 FN_FROM_IS_TMP old_name is temporary.
                                 FN_TO_IS_TMP   new_name is temporary.
+                                NO_FRM_RENAME  Don't rename the FRM file
+                                but only the table in the storage engine.
 
   RETURN
-    0           OK
-    != 0        Error
+    FALSE   OK
+    TRUE    Error
 */
 
 bool
@@ -3704,7 +3706,7 @@ mysql_rename_table(handlerton *base, con
 
   if (!file || !(error=file->rename_table(from_base, to_base)))
   {
-    if (rename_file_ext(from,to,reg_ext))
+    if (!(flags & NO_FRM_RENAME) && rename_file_ext(from,to,reg_ext))
     {
       error=my_errno;
       /* Restore old file name */
@@ -5196,7 +5198,50 @@ static uint compare_tables(TABLE *table,
 
 
 /*
-  Alter table
+  SYNOPSIS
+    mysql_alter_table()
+      thd              Thread handle
+      new_db           If there is a RENAME clause
+      new_name         If there is a RENAME clause
+      lex_create_info  Information from the parsing phase. Since some
+                       clauses are common to CREATE and ALTER TABLE, the
+                       data is stored in lex->create_info. The non-common
+                       is stored in lex->alter_info.
+      table_list       The table to change.
+      fields           lex->create_list - List of fields to be changed,
+                       added or dropped.
+      keys             lex->key_list - List of keys to be changed, added or
+                       dropped.
+      order_num        How many ORDER BY fields has been specified.
+      order            List of fields to ORDER BY.
+      ignore           Whether we have ALTER IGNORE TABLE
+      alter_info       Information from the parsing phase specific to ALTER
+                       TABLE and not shared with CREATE TABLE.
+      do_send_ok       Whether to call send_ok() on success.
+
+  DESCRIPTION
+    This is a veery long function and is everything but the kitchen sink :)
+    It is used to alter a table and not only by ALTER TABLE but also
+    CREATE|DROP INDEX are mapped on this function.
+
+    When the ALTER TABLE statement just does a RENAME or ENABLE|DISABLE KEYS,
+    or both only, then this function short cuts its operation by renaming
+    the table and/or enabling/disabling the keys. In this case, the FRM is
+    not changed. However, if there is a RENAME + change of a field, or an
+    index, the short cut is not used. See how `fields` is used to generate
+    the new FRM regarding the structure of the fields. The same is done
+    for the indices of the table.
+
+    Important is the fact, that this function tries to do as little work as
+    possible, by finding out whether a intermediate table is needed to copy
+    data into and when finishing the altering to use it as the original table.
+    For this reason the function compare_tables() is called, which decides
+    based on all kind of data how similar are the new and the original
+    tables. 
+
+  RETURN VALUES
+    FALSE  OK
+    TRUE   Error
 */
 
 bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
@@ -5215,7 +5260,7 @@ bool mysql_alter_table(THD *thd,char *ne
   char reg_path[FN_REFLEN+1];
   ha_rows copied,deleted;
   uint db_create_options, used_fields;
-  handlerton *old_db_type, *new_db_type;
+  handlerton *old_db_type, *new_db_type, *save_old_db_type;
   legacy_db_type table_type;
   HA_CREATE_INFO *create_info;
   frm_type_enum frm_type;
@@ -5519,7 +5564,7 @@ view_err:
     DBUG_RETURN(error);
   }
 
-  /* Full alter table */
+  /* We have to do full alter table */
 
   /* Let new create options override the old ones */
   if (!(used_fields & HA_CREATE_USED_MIN_ROWS))
@@ -6038,8 +6083,8 @@ view_err:
       old data and index files.  Create also symlinks to point at
       the new tables.
       Copy data.
-      At end, rename temporary tables and symlinks to temporary table
-      to final table name.
+      At end, rename intermediate tables, and symlinks to intermediate
+      table, to final table name.
       Remove old table and old symlinks
 
     If rename is made to another database:
@@ -6100,6 +6145,7 @@ view_err:
       /* table is a normal table: Create temporary table in same directory */
       build_table_filename(path, sizeof(path), new_db, tmp_name, "",
                            FN_IS_TMP);
+      /* Open our intermediate table */
       new_table=open_temporary_table(thd, path, new_db, tmp_name,0);
     }
     if (!new_table)
@@ -6305,7 +6351,7 @@ view_err:
 
   if (new_table)
   {
-    /* close temporary table that will be the new table */
+    /* Close the intermediate table that will be the new table */
     intern_close_table(new_table);
     my_free((gptr) new_table,MYF(0));
   }
@@ -6319,7 +6365,7 @@ view_err:
 
   /*
     Data is copied.  Now we rename the old table to a temp name,
-    rename the new one to the old name, remove all entries from the old table
+    rename the new one to the old name, remove all entries about the old table
     from the cache, free all locks, close the old table and remove it.
   */
 
@@ -6346,7 +6392,7 @@ view_err:
   {
     /*
       Win32 and InnoDB can't drop a table that is in use, so we must
-      close the original table at before doing the rename
+      close the original table before doing the rename
     */
     table->s->version= 0;                	// Force removal of table def
     close_cached_table(thd, table);
@@ -6360,6 +6406,21 @@ view_err:
 
 
   error=0;
+  save_old_db_type= old_db_type;
+
+  /*
+    This leads to the storage engine (SE) not being notified for renames in
+    mysql_rename_table(), because we just juggle with the FRM and nothing
+    more. If we have an intermediate table, then we notify the SE that that
+    it should become the actual table. Later we will recycle the old table.
+    However, in case of ALTER TABLE RENAME there could be no intermediate
+    table. This is when the old and new tables are compatible, according to
+    compare_table(). Then, we need one additional call to
+    mysql_rename_table() with flag NO_FRM_RENAME, which does nothing else but
+    actual rename in the SE and the FRM is not touched. Note that, if the
+    table is renamed and the SE is also changed, then intermediate table is
+    created and the additional call will not take place.
+  */
   if (!need_copy_table)
     new_db_type=old_db_type= NULL; // this type cannot happen in regular ALTER
   if (mysql_rename_table(old_db_type, db, table_name, db, old_name,
@@ -6369,8 +6430,11 @@ view_err:
     VOID(quick_rm_table(new_db_type, new_db, tmp_name, FN_IS_TMP));
   }
   else if (mysql_rename_table(new_db_type,new_db,tmp_name,new_db,
-			      new_alias, FN_FROM_IS_TMP) ||
+                              new_alias, FN_FROM_IS_TMP) ||
            (new_name != table_name || new_db != db) && // we also do rename
+           (need_copy_table ||
+            mysql_rename_table(save_old_db_type, db, table_name, new_db,
+                               new_alias, NO_FRM_RENAME)) &&
            Table_triggers_list::change_table_name(thd, db, table_name,
                                                   new_db, new_alias))
   {
@@ -6381,6 +6445,7 @@ view_err:
     VOID(mysql_rename_table(old_db_type, db, old_name, db, alias,
                             FN_FROM_IS_TMP));
   }
+
   if (error)
   {
     /*
@@ -6412,6 +6477,7 @@ view_err:
       goto err;
     }
   }
+
   if (thd->lock || new_name != table_name || no_table_reopen)  // True if WIN32
   {
     /*
@@ -6478,10 +6544,7 @@ view_err:
   DBUG_ASSERT(!(mysql_bin_log.is_open() && thd->current_stmt_binlog_row_based &&
                 (create_info->options & HA_LEX_CREATE_TMP_TABLE)));
   write_bin_log(thd, TRUE, thd->query, thd->query_length);
-  /*
-    TODO RONM: This problem needs to handled for Berkeley DB partitions
-    as well
-  */
+
   if (ha_check_storage_engine_flag(old_db_type,HTON_FLUSH_AFTER_RENAME))
   {
     /*

--- 1.67/mysql-test/r/alter_table.result	2006-11-29 11:46:42 +01:00
+++ 1.68/mysql-test/r/alter_table.result	2006-11-29 11:46:42 +01:00
@@ -733,3 +733,45 @@ Table	Create Table
   `c1` int(11) DEFAULT NULL
 ) ENGINE=MyISAM DEFAULT CHARSET=latin1
 DROP TABLE `#sql2`, `@0023sql1`;
+DROP TABLE IF EXISTS t1;
+DROP TABLE IF EXISTS t2;
+CREATE TABLE t1 (
+int_field int UNSIGNED NOT NULL,
+char_field CHAR(10),
+INDEX(`int_field`)
+);
+DESCRIBE t1;
+Field	Type	Null	Key	Default	Extra
+int_field	int(10) unsigned	NO	MUL		
+char_field	char(10)	YES		NULL	
+SHOW INDEXES FROM t1;
+Table	Non_unique	Key_name	Seq_in_index	Column_name	Collation	Cardinality	Sub_part	Packed	Null	Index_type	Comment
+t1	1	int_field	1	int_field	A	NULL	NULL	NULL		BTREE	
+INSERT INTO t1 VALUES (1, "edno"), (1, "edno"), (2, "dve"), (3, "tri"), (5, "pet");
+"Non-copy data change - new frm, but old data and index files"
+ALTER TABLE t1
+CHANGE int_field unsigned_int_field INT UNSIGNED NOT NULL,
+RENAME t2;
+SELECT * FROM t1 ORDER BY int_field;
+ERROR 42S02: Table 'test.t1' doesn't exist
+SELECT * FROM t2 ORDER BY unsigned_int_field;
+unsigned_int_field	char_field
+1	edno
+1	edno
+2	dve
+3	tri
+5	pet
+DESCRIBE t2;
+Field	Type	Null	Key	Default	Extra
+unsigned_int_field	int(10) unsigned	NO	MUL		
+char_field	char(10)	YES		NULL	
+DESCRIBE t2;
+Field	Type	Null	Key	Default	Extra
+unsigned_int_field	int(10) unsigned	NO	MUL		
+char_field	char(10)	YES		NULL	
+ALTER TABLE t2 MODIFY unsigned_int_field BIGINT UNSIGNED NOT NULL;
+DESCRIBE t2;
+Field	Type	Null	Key	Default	Extra
+unsigned_int_field	bigint(20) unsigned	NO	MUL		
+char_field	char(10)	YES		NULL	
+DROP TABLE t2;

--- 1.52/mysql-test/t/alter_table.test	2006-11-29 11:46:42 +01:00
+++ 1.53/mysql-test/t/alter_table.test	2006-11-29 11:46:42 +01:00
@@ -101,7 +101,7 @@ create table mysqltest.t1 (name char(15)
 insert into mysqltest.t1 (name) values ("mysqltest");
 select * from t1;
 select * from mysqltest.t1;
---error 1050
+--error ER_TABLE_EXISTS_ERROR
 alter table t1 rename mysqltest.t1;
 select * from t1;
 select * from mysqltest.t1;
@@ -231,9 +231,9 @@ DROP TABLE t1;
 # BUG#4717 - check for valid table names
 #
 create table t1 (a int);
---error 1103
+--error ER_WRONG_TABLE_NAME
 alter table t1 rename to ``;
---error 1103
+--error ER_WRONG_TABLE_NAME
 rename table t1 to ``;
 drop table t1;
 
@@ -325,14 +325,14 @@ drop table t1;
 CREATE TABLE t1 (a int PRIMARY KEY, b INT UNIQUE);
 ALTER TABLE t1 DROP PRIMARY KEY;
 SHOW CREATE TABLE t1;
---error 1091
+--error ER_CANT_DROP_FIELD_OR_KEY
 ALTER TABLE t1 DROP PRIMARY KEY;
 DROP TABLE t1;
 
 # BUG#3899
 create table t1 (a int, b int, key(a));
 insert into t1 values (1,1), (2,2);
---error 1091
+--error ER_CANT_DROP_FIELD_OR_KEY
 alter table t1 drop key no_such_key;
 alter table t1 drop key a;
 drop table t1;
@@ -343,7 +343,7 @@ drop table t1;
 # Some platforms (Mac OS X, Windows) will send the error message using small letters.
 CREATE TABLE T12207(a int) ENGINE=MYISAM;
 --replace_result t12207 T12207
---error 1031
+--error ER_ILLEGAL_HA
 ALTER TABLE T12207 DISCARD TABLESPACE;
 DROP TABLE T12207;
 
@@ -367,7 +367,7 @@ drop table t1;
 # shorter than packed field length.
 #
 create table t1 ( a timestamp );
---error 1089
+--error ER_WRONG_SUB_KEY
 alter table t1 add unique ( a(1) );
 drop table t1;
 
@@ -380,7 +380,7 @@ create table t1 (c1 int);
 # Move table to other database.
 alter table t1 rename mysqltest.t1;
 # Assure that it has moved.
---error 1051
+--error ER_BAD_TABLE_ERROR
 drop table t1;
 # Move table back.
 alter table mysqltest.t1 rename t1;
@@ -394,7 +394,7 @@ use mysqltest;
 # Drop the current db. This de-selects any db.
 drop database mysqltest;
 # Now test for correct message.
---error 1046
+--error ER_NO_DB_ERROR
 alter table test.t1 rename t1;
 # Check that explicit qualifying works even with no selected db.
 alter table test.t1 rename test.t1;
@@ -554,3 +554,38 @@ SHOW CREATE TABLE `#sql2`;
 SHOW CREATE TABLE `@0023sql1`;
 DROP TABLE `#sql2`, `@0023sql1`;
 
+#
+# Bug #22369: Alter table rename combined with other alterations causes lost tables
+#
+# This problem happens if the data change is compatible.
+# Changing to the same type is compatible for example.
+#
+--disable_warnings
+DROP TABLE IF EXISTS t1;
+DROP TABLE IF EXISTS t2;
+--enable_warnings
+CREATE TABLE t1 (
+  int_field int UNSIGNED NOT NULL,
+  char_field CHAR(10),
+  INDEX(`int_field`)
+);
+
+DESCRIBE t1;
+
+SHOW INDEXES FROM t1;
+
+INSERT INTO t1 VALUES (1, "edno"), (1, "edno"), (2, "dve"), (3, "tri"), (5, "pet"); 
+--echo "Non-copy data change - new frm, but old data and index files"
+ALTER TABLE t1
+  CHANGE int_field unsigned_int_field INT UNSIGNED NOT NULL,
+  RENAME t2;
+
+--error ER_NO_SUCH_TABLE
+SELECT * FROM t1 ORDER BY int_field;
+SELECT * FROM t2 ORDER BY unsigned_int_field;
+DESCRIBE t2;
+DESCRIBE t2;
+ALTER TABLE t2 MODIFY unsigned_int_field BIGINT UNSIGNED NOT NULL;
+DESCRIBE t2;
+
+DROP TABLE t2;
Thread
bk commit into 5.1 tree (andrey:1.2386) BUG#22369ahristov29 Nov