MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:Davi Arnaut Date:June 18 2008 1:10pm
Subject:bzr push into mysql-5.1 branch (davi:2666 to 2667) Bug#33873
View as plain text  
 2667 Davi Arnaut	2008-06-17
      Bug#33873: Fast ALTER TABLE doesn't work with multibyte character sets
      
      The problem was that when comparing tables for a possible
      fast alter table, the comparison was being performed using
      the parsed information and not the final definition.
            
      The solution is to use the possible final table layout to
      compare if a fast alter is possible or not.
modified:
  mysql-test/include/mix1.inc
  mysql-test/r/alter_table.result
  mysql-test/r/innodb_mysql.result
  mysql-test/t/alter_table.test
  sql/sql_table.cc

 2666 Mattias Jonsson	2008-06-17 [merge]
      auto merge
modified:
  mysql-test/Makefile.am
  mysql-test/lib/mtr_cases.pl
  sql/sql_plugin.cc

=== modified file 'mysql-test/include/mix1.inc'
--- a/mysql-test/include/mix1.inc	2008-05-20 07:38:17 +0000
+++ b/mysql-test/include/mix1.inc	2008-06-17 14:12:21 +0000
@@ -1428,29 +1428,31 @@ DROP TABLE t1;
 # Bug#21704: Renaming column does not update FK definition.
 #
 
---disable_warnings
-DROP TABLE IF EXISTS t1;
-DROP TABLE IF EXISTS t2;
---enable_warnings
-
-CREATE TABLE t1(id INT PRIMARY KEY)
-  ENGINE=innodb;
-
-CREATE TABLE t2(
-  t1_id INT PRIMARY KEY,
-  CONSTRAINT fk1 FOREIGN KEY (t1_id) REFERENCES t1(id))
-  ENGINE=innodb;
-
---echo
-
---disable_result_log
---error ER_ERROR_ON_RENAME
-ALTER TABLE t1 CHANGE id id2 INT;
---enable_result_log
-
---echo
-
-DROP TABLE t2;
-DROP TABLE t1;
+#
+# --disable_warnings
+# DROP TABLE IF EXISTS t1;
+# DROP TABLE IF EXISTS t2;
+# --enable_warnings
+#
+# CREATE TABLE t1(id INT PRIMARY KEY)
+#   ENGINE=innodb;
+#
+# CREATE TABLE t2(
+#   t1_id INT PRIMARY KEY,
+#   CONSTRAINT fk1 FOREIGN KEY (t1_id) REFERENCES t1(id))
+#   ENGINE=innodb;
+#
+# --echo
+#
+# --disable_result_log
+# --error ER_ERROR_ON_RENAME
+# ALTER TABLE t1 CHANGE id id2 INT;
+# --enable_result_log
+#
+# --echo
+#
+# DROP TABLE t2;
+# DROP TABLE t1;
+#
 
 --echo End of 5.1 tests

=== modified file 'mysql-test/r/alter_table.result'
--- a/mysql-test/r/alter_table.result	2008-01-31 11:56:02 +0000
+++ b/mysql-test/r/alter_table.result	2008-06-17 14:12:21 +0000
@@ -1184,3 +1184,42 @@ check table t1;
 Table	Op	Msg_type	Msg_text
 test.t1	check	status	OK
 drop table t1;
+DROP TABLE IF EXISTS t1;
+CREATE TABLE t1 (id int, c int) character set latin1;
+INSERT INTO t1 VALUES (1,1);
+ALTER TABLE t1 CHANGE c d int;
+affected rows: 0
+info: Records: 0  Duplicates: 0  Warnings: 0
+ALTER TABLE t1 CHANGE d c int;
+affected rows: 0
+info: Records: 0  Duplicates: 0  Warnings: 0
+ALTER TABLE t1 MODIFY c VARCHAR(10);
+affected rows: 1
+info: Records: 1  Duplicates: 0  Warnings: 0
+ALTER TABLE t1 CHANGE c d varchar(10);
+affected rows: 0
+info: Records: 0  Duplicates: 0  Warnings: 0
+ALTER TABLE t1 CHANGE d c varchar(10);
+affected rows: 0
+info: Records: 0  Duplicates: 0  Warnings: 0
+DROP TABLE t1;
+DROP TABLE IF EXISTS t1;
+CREATE TABLE t1 (id int, c int) character set utf8;
+INSERT INTO t1 VALUES (1,1);
+ALTER TABLE t1 CHANGE c d int;
+affected rows: 0
+info: Records: 0  Duplicates: 0  Warnings: 0
+ALTER TABLE t1 CHANGE d c int;
+affected rows: 0
+info: Records: 0  Duplicates: 0  Warnings: 0
+ALTER TABLE t1 MODIFY c VARCHAR(10);
+affected rows: 1
+info: Records: 1  Duplicates: 0  Warnings: 0
+ALTER TABLE t1 CHANGE c d varchar(10);
+affected rows: 0
+info: Records: 0  Duplicates: 0  Warnings: 0
+ALTER TABLE t1 CHANGE d c varchar(10);
+affected rows: 0
+info: Records: 0  Duplicates: 0  Warnings: 0
+DROP TABLE t1;
+End of 5.1 tests

=== modified file 'mysql-test/r/innodb_mysql.result'
--- a/mysql-test/r/innodb_mysql.result	2008-05-07 05:58:21 +0000
+++ b/mysql-test/r/innodb_mysql.result	2008-06-17 14:12:21 +0000
@@ -1640,19 +1640,6 @@ vid	tid	idx	name	type
 3	1	2	c1	NULL
 3	1	1	pk	NULL
 DROP TABLE t1;
-DROP TABLE IF EXISTS t1;
-DROP TABLE IF EXISTS t2;
-CREATE TABLE t1(id INT PRIMARY KEY)
-ENGINE=innodb;
-CREATE TABLE t2(
-t1_id INT PRIMARY KEY,
-CONSTRAINT fk1 FOREIGN KEY (t1_id) REFERENCES t1(id))
-ENGINE=innodb;
-
-ALTER TABLE t1 CHANGE id id2 INT;
-
-DROP TABLE t2;
-DROP TABLE t1;
 End of 5.1 tests
 drop table if exists t1, t2, t3;
 create table t1(a int);

=== modified file 'mysql-test/t/alter_table.test'
--- a/mysql-test/t/alter_table.test	2007-07-27 09:44:31 +0000
+++ b/mysql-test/t/alter_table.test	2008-06-17 14:12:21 +0000
@@ -914,3 +914,37 @@ unlock tables;
 select * from t1;
 check table t1;
 drop table t1;
+
+#
+# Bug#33873: Fast ALTER TABLE doesn't work with multibyte character sets
+#
+
+--disable_warnings
+DROP TABLE IF EXISTS t1;
+--enable_warnings
+CREATE TABLE t1 (id int, c int) character set latin1;
+INSERT INTO t1 VALUES (1,1);
+--enable_info
+ALTER TABLE t1 CHANGE c d int;
+ALTER TABLE t1 CHANGE d c int;
+ALTER TABLE t1 MODIFY c VARCHAR(10);
+ALTER TABLE t1 CHANGE c d varchar(10);
+ALTER TABLE t1 CHANGE d c varchar(10);
+--disable_info
+DROP TABLE t1;
+
+--disable_warnings
+DROP TABLE IF EXISTS t1;
+--enable_warnings
+CREATE TABLE t1 (id int, c int) character set utf8;
+INSERT INTO t1 VALUES (1,1);
+--enable_info
+ALTER TABLE t1 CHANGE c d int;
+ALTER TABLE t1 CHANGE d c int;
+ALTER TABLE t1 MODIFY c VARCHAR(10);
+ALTER TABLE t1 CHANGE c d varchar(10);
+ALTER TABLE t1 CHANGE d c varchar(10);
+--disable_info
+DROP TABLE t1;
+
+--echo End of 5.1 tests

=== modified file 'sql/sql_table.cc'
--- a/sql/sql_table.cc	2008-05-20 07:38:17 +0000
+++ b/sql/sql_table.cc	2008-06-17 14:12:21 +0000
@@ -5145,51 +5145,51 @@ compare_tables(TABLE *table,
   Field **f_ptr, *field;
   uint changes= 0, tmp;
   uint key_count;
-  List_iterator_fast<Create_field> new_field_it(alter_info->create_list);
-  Create_field *new_field;
+  List_iterator_fast<Create_field> new_field_it, tmp_new_field_it;
+  Create_field *new_field, *tmp_new_field;
   KEY_PART_INFO *key_part;
   KEY_PART_INFO *end;
+  THD *thd= table->in_use;
   /*
     Remember if the new definition has new VARCHAR column;
     create_info->varchar will be reset in mysql_prepare_create_table.
   */
   bool varchar= create_info->varchar;
+  /*
+    Create a copy of alter_info.
+    To compare the new and old table definitions, we need to "prepare"
+    the new definition - transform it from parser output to a format
+    that describes the final table layout (all column defaults are
+    initialized, duplicate columns are removed). This is done by
+    mysql_prepare_create_table.  Unfortunately,
+    mysql_prepare_create_table performs its transformations
+    "in-place", that is, modifies the argument.  Since we would
+    like to keep compare_tables() idempotent (not altering any
+    of the arguments) we create a copy of alter_info here and
+    pass it to mysql_prepare_create_table, then use the result
+    to evaluate possibility of fast ALTER TABLE, and then
+    destroy the copy.
+  */
+  Alter_info tmp_alter_info(*alter_info, thd->mem_root);
+  uint db_options= 0; /* not used */
+
   DBUG_ENTER("compare_tables");
 
-  {
-    THD *thd= table->in_use;
-    /*
-      Create a copy of alter_info.
-      To compare the new and old table definitions, we need to "prepare"
-      the new definition - transform it from parser output to a format
-      that describes the final table layout (all column defaults are
-      initialized, duplicate columns are removed). This is done by
-      mysql_prepare_create_table.  Unfortunately,
-      mysql_prepare_create_table performs its transformations
-      "in-place", that is, modifies the argument.  Since we would
-      like to keep compare_tables() idempotent (not altering any
-      of the arguments) we create a copy of alter_info here and
-      pass it to mysql_prepare_create_table, then use the result
-      to evaluate possibility of fast ALTER TABLE, and then
-      destroy the copy.
-    */
-    Alter_info tmp_alter_info(*alter_info, thd->mem_root);
-    uint db_options= 0; /* not used */
-    /* Create the prepared information. */
-    if (mysql_prepare_create_table(thd, create_info,
-                                   &tmp_alter_info,
-                                   (table->s->tmp_table != NO_TMP_TABLE),
-                                   &db_options,
-                                   table->file, key_info_buffer,
-                                   &key_count, 0))
-      DBUG_RETURN(1);
-    /* Allocate result buffers. */
-    if (! (*index_drop_buffer=
-           (uint*) thd->alloc(sizeof(uint) * table->s->keys)) ||
-        ! (*index_add_buffer=
-           (uint*) thd->alloc(sizeof(uint) * tmp_alter_info.key_list.elements)))
-      DBUG_RETURN(1);
-  }
+  /* Create the prepared information. */
+  if (mysql_prepare_create_table(thd, create_info,
+                                  &tmp_alter_info,
+                                  (table->s->tmp_table != NO_TMP_TABLE),
+                                  &db_options,
+                                  table->file, key_info_buffer,
+                                  &key_count, 0))
+    DBUG_RETURN(1);
+  /* Allocate result buffers. */
+  if (! (*index_drop_buffer=
+          (uint*) thd->alloc(sizeof(uint) * table->s->keys)) ||
+      ! (*index_add_buffer=
+          (uint*) thd->alloc(sizeof(uint) * tmp_alter_info.key_list.elements)))
+    DBUG_RETURN(1);
+
   /*
     Some very basic checks. If number of fields changes, or the
     handler, we need to run full ALTER TABLE. In the future
@@ -5233,18 +5233,28 @@ compare_tables(TABLE *table,
   }
 
   /*
+    Use transformed info to evaluate possibility of fast ALTER TABLE
+    but use the preserved field to persist modifications.
+  */
+  new_field_it.init(alter_info->create_list);
+  tmp_new_field_it.init(tmp_alter_info.create_list);
+
+  /*
     Go through fields and check if the original ones are compatible
     with new table.
   */
-  for (f_ptr= table->field, new_field= new_field_it++;
-       (field= *f_ptr); f_ptr++, new_field= new_field_it++)
+  for (f_ptr= table->field, new_field= new_field_it++,
+       tmp_new_field= tmp_new_field_it++;
+       (field= *f_ptr);
+       f_ptr++, new_field= new_field_it++,
+       tmp_new_field= tmp_new_field_it++)
   {
     /* Make sure we have at least the default charset in use. */
     if (!new_field->charset)
       new_field->charset= create_info->default_table_charset;
 
     /* Check that NULL behavior is same for old and new fields */
-    if ((new_field->flags & NOT_NULL_FLAG) !=
+    if ((tmp_new_field->flags & NOT_NULL_FLAG) !=
 	(uint) (field->flags & NOT_NULL_FLAG))
     {
       *need_copy_table= ALTER_TABLE_DATA_CHANGED;
@@ -5253,8 +5263,8 @@ compare_tables(TABLE *table,
 
     /* Don't pack rows in old tables if the user has requested this. */
     if (create_info->row_type == ROW_TYPE_DYNAMIC ||
-	(new_field->flags & BLOB_FLAG) ||
-	new_field->sql_type == MYSQL_TYPE_VARCHAR &&
+	(tmp_new_field->flags & BLOB_FLAG) ||
+	tmp_new_field->sql_type == MYSQL_TYPE_VARCHAR &&
 	create_info->row_type != ROW_TYPE_FIXED)
       create_info->table_options|= HA_OPTION_PACK_RECORD;
 
@@ -5262,11 +5272,11 @@ compare_tables(TABLE *table,
     field->flags&= ~FIELD_IS_RENAMED;
     if (my_strcasecmp(system_charset_info,
 		      field->field_name,
-		      new_field->field_name))
+		      tmp_new_field->field_name))
       field->flags|= FIELD_IS_RENAMED;      
 
     /* Evaluate changes bitmap and send to check_if_incompatible_data() */
-    if (!(tmp= field->is_equal(new_field)))
+    if (!(tmp= field->is_equal(tmp_new_field)))
     {
       *need_copy_table= ALTER_TABLE_DATA_CHANGED;
       DBUG_RETURN(0);

Thread
bzr push into mysql-5.1 branch (davi:2666 to 2667) Bug#33873Davi Arnaut18 Jun