List:Commits« Previous MessageNext Message »
From:Davi Arnaut Date:September 29 2009 10:58am
Subject:bzr commit into mysql-5.1-bugteam branch (davi:3146) Bug#45567
View as plain text  
# At a local mysql-5.1-bugteam repository of davi

 3146 Davi Arnaut	2009-09-29
      Bug#45567: Fast ALTER TABLE broken for enum and set
      
      The problem was that appending values to the end of an existing
      ENUM or SET column was being treated as table data modification,
      preventing a immediately (fast) table alteration that occurs when
      only table metadata is being modified.
      
      The cause was twofold: adding a enumeration or set members to the 
      end of the list of valid member values was not being considered
      a "compatible" table alteration, and for SET columns, the check
      was being done upon the max display length and not the underlying
      (pack) length of the field.
      
      The solution is to augment the function that checks wether two ENUM
      or SET fields are compatible -- by comparing the pack lengths and
      performing a limited comparison of the member values.
     @ mysql-test/r/alter_table.result
        Add test case result for Bug#45567
     @ mysql-test/t/alter_table.test
        Add test case for Bug#45567
     @ sql/field.cc
        Check whether two fields can be considered 'equal' for table
        alteration purposes. Fields are equal if they retain the same
        pack length and if new members are added to the end of the list.
     @ sql/field.h
        Add comment and remove method.

    modified:
      mysql-test/r/alter_table.result
      mysql-test/t/alter_table.test
      sql/field.cc
      sql/field.h
=== modified file 'mysql-test/r/alter_table.result'
--- a/mysql-test/r/alter_table.result	2008-12-09 13:31:22 +0000
+++ b/mysql-test/r/alter_table.result	2009-09-29 10:58:42 +0000
@@ -1268,4 +1268,66 @@ a	b
 4	b
 5	a
 DROP TABLE t1;
+#
+# Bug#45567: Fast ALTER TABLE broken for enum and set
+#
+DROP TABLE IF EXISTS t1;
+CREATE TABLE t1 (a ENUM('a1','a2'));
+INSERT INTO t1 VALUES ('a1'),('a2');
+# No copy: No modification
+ALTER TABLE t1 MODIFY COLUMN a ENUM('a1','a2');
+affected rows: 0
+info: Records: 0  Duplicates: 0  Warnings: 0
+# No copy: Add new enumeration to the end
+ALTER TABLE t1 MODIFY COLUMN a ENUM('a1','a2','a3');
+affected rows: 0
+info: Records: 0  Duplicates: 0  Warnings: 0
+# Copy: Modify and add new to the end
+ALTER TABLE t1 MODIFY COLUMN a ENUM('a1','a2','xx','a5');
+affected rows: 2
+info: Records: 2  Duplicates: 0  Warnings: 0
+# Copy: Remove from the end
+ALTER TABLE t1 MODIFY COLUMN a ENUM('a1','a2','xx');
+affected rows: 2
+info: Records: 2  Duplicates: 0  Warnings: 0
+# Copy: Add new enumeration
+ALTER TABLE t1 MODIFY COLUMN a ENUM('a1','a2','a0','xx');
+affected rows: 2
+info: Records: 2  Duplicates: 0  Warnings: 0
+# No copy: Add new enumerations to the end
+ALTER TABLE t1 MODIFY COLUMN a ENUM('a1','a2','a0','xx','a5','a6');
+affected rows: 0
+info: Records: 0  Duplicates: 0  Warnings: 0
+DROP TABLE t1;
+CREATE TABLE t1 (a SET('a1','a2'));
+INSERT INTO t1 VALUES ('a1'),('a2');
+# No copy: No modification
+ALTER TABLE t1 MODIFY COLUMN a SET('a1','a2');
+affected rows: 0
+info: Records: 0  Duplicates: 0  Warnings: 0
+# No copy: Add new to the end
+ALTER TABLE t1 MODIFY COLUMN a SET('a1','a2','a3');
+affected rows: 0
+info: Records: 0  Duplicates: 0  Warnings: 0
+# Copy: Modify and add new to the end
+ALTER TABLE t1 MODIFY COLUMN a SET('a1','a2','xx','a5');
+affected rows: 2
+info: Records: 2  Duplicates: 0  Warnings: 0
+# Copy: Remove from the end
+ALTER TABLE t1 MODIFY COLUMN a SET('a1','a2','xx');
+affected rows: 2
+info: Records: 2  Duplicates: 0  Warnings: 0
+# Copy: Add new member
+ALTER TABLE t1 MODIFY COLUMN a SET('a1','a2','a0','xx');
+affected rows: 2
+info: Records: 2  Duplicates: 0  Warnings: 0
+# No copy: Add new to the end
+ALTER TABLE t1 MODIFY COLUMN a SET('a1','a2','a0','xx','a5','a6');
+affected rows: 0
+info: Records: 0  Duplicates: 0  Warnings: 0
+# Copy: Numerical incrase (pack lenght)
+ALTER TABLE t1 MODIFY COLUMN a SET('a1','a2','a0','xx','a5','a6','a7','a8','a9','a10');
+affected rows: 2
+info: Records: 2  Duplicates: 0  Warnings: 0
+DROP TABLE t1;
 End of 5.1 tests

=== modified file 'mysql-test/t/alter_table.test'
--- a/mysql-test/t/alter_table.test	2008-12-09 13:31:22 +0000
+++ b/mysql-test/t/alter_table.test	2009-09-29 10:58:42 +0000
@@ -1000,4 +1000,50 @@ ALTER TABLE t1 MODIFY b ENUM('a', 'z', '
 SELECT * FROM t1;
 DROP TABLE t1;
 
+--echo #
+--echo # Bug#45567: Fast ALTER TABLE broken for enum and set
+--echo #
+
+--disable_warnings
+DROP TABLE IF EXISTS t1;
+--enable_warnings
+
+CREATE TABLE t1 (a ENUM('a1','a2'));
+INSERT INTO t1 VALUES ('a1'),('a2');
+--enable_info
+--echo # No copy: No modification
+ALTER TABLE t1 MODIFY COLUMN a ENUM('a1','a2');
+--echo # No copy: Add new enumeration to the end
+ALTER TABLE t1 MODIFY COLUMN a ENUM('a1','a2','a3');
+--echo # Copy: Modify and add new to the end
+ALTER TABLE t1 MODIFY COLUMN a ENUM('a1','a2','xx','a5');
+--echo # Copy: Remove from the end
+ALTER TABLE t1 MODIFY COLUMN a ENUM('a1','a2','xx');
+--echo # Copy: Add new enumeration
+ALTER TABLE t1 MODIFY COLUMN a ENUM('a1','a2','a0','xx');
+--echo # No copy: Add new enumerations to the end
+ALTER TABLE t1 MODIFY COLUMN a ENUM('a1','a2','a0','xx','a5','a6');
+--disable_info
+DROP TABLE t1;
+
+CREATE TABLE t1 (a SET('a1','a2'));
+INSERT INTO t1 VALUES ('a1'),('a2');
+--enable_info
+--echo # No copy: No modification
+ALTER TABLE t1 MODIFY COLUMN a SET('a1','a2');
+--echo # No copy: Add new to the end
+ALTER TABLE t1 MODIFY COLUMN a SET('a1','a2','a3');
+--echo # Copy: Modify and add new to the end
+ALTER TABLE t1 MODIFY COLUMN a SET('a1','a2','xx','a5');
+--echo # Copy: Remove from the end
+ALTER TABLE t1 MODIFY COLUMN a SET('a1','a2','xx');
+--echo # Copy: Add new member
+ALTER TABLE t1 MODIFY COLUMN a SET('a1','a2','a0','xx');
+--echo # No copy: Add new to the end
+ALTER TABLE t1 MODIFY COLUMN a SET('a1','a2','a0','xx','a5','a6');
+--echo # Copy: Numerical incrase (pack lenght)
+ALTER TABLE t1 MODIFY COLUMN a SET('a1','a2','a0','xx','a5','a6','a7','a8','a9','a10');
+--disable_info
+DROP TABLE t1;
+
 --echo End of 5.1 tests

=== modified file 'sql/field.cc'
--- a/sql/field.cc	2009-08-28 16:21:54 +0000
+++ b/sql/field.cc	2009-09-29 10:58:42 +0000
@@ -8833,38 +8833,81 @@ bool Field::eq_def(Field *field)
 
 
 /**
+  Compare the first t1::count type names.
+
+  @return TRUE if the type names of t1 match those of t2. FALSE otherwise.
+*/
+
+static bool compare_type_names(CHARSET_INFO *charset, TYPELIB *t1, TYPELIB *t2)
+{
+  for (uint i= 0; i < t1->count; i++)
+    if (my_strnncoll(charset,
+                     (const uchar*) t1->type_names[i],
+                     t1->type_lengths[i],
+                     (const uchar*) t2->type_names[i],
+                     t2->type_lengths[i]))
+      return FALSE;
+  return TRUE;
+}
+
+/**
   @return
   returns 1 if the fields are equally defined
 */
 
 bool Field_enum::eq_def(Field *field)
 {
+  TYPELIB *values;
+
   if (!Field::eq_def(field))
-    return 0;
-  return compare_enum_values(((Field_enum*) field)->typelib);
-}
+    return FALSE;
 
+  values= ((Field_enum*) field)->typelib;
 
-bool Field_enum::compare_enum_values(TYPELIB *values)
-{
+  /* Definition must be strictly equal. */
   if (typelib->count != values->count)
     return FALSE;
-  for (uint i= 0; i < typelib->count; i++)
-    if (my_strnncoll(field_charset,
-                     (const uchar*) typelib->type_names[i],
-                     typelib->type_lengths[i],
-                     (const uchar*) values->type_names[i],
-                     values->type_lengths[i]))
-      return FALSE;
-  return TRUE;
+
+  return compare_type_names(field_charset, typelib, values);
 }
 
 
+/**
+  Check whether two fields can be considered 'equal' for table
+  alteration purposes. Fields are equal if they retain the same
+  pack length and if new members are added to the end of the list.
+
+  @return IS_EQUAL_YES if fields are compatible.
+          IS_EQUAL_NO otherwise.
+*/
+
 uint Field_enum::is_equal(Create_field *new_field)
 {
-  if (!Field_str::is_equal(new_field))
-    return 0;
-  return compare_enum_values(new_field->interval);
+  TYPELIB *values= new_field->interval;
+
+  /*
+    The fields are compatible if they have the same flags,
+    type, charset and have the same underlying length.
+  */
+  if (compare_str_field_flags(new_field, flags) ||
+      new_field->sql_type != real_type() ||
+      new_field->charset != field_charset ||
+      new_field->pack_length != pack_length())
+    return IS_EQUAL_NO;
+
+  /*
+    Changing the definition of an ENUM or SET column by adding a new
+    enumeration or set members to the end of the list of valid member
+    values only alters table metadata and not table data.
+  */
+  if (typelib->count > values->count)
+    return IS_EQUAL_NO;
+
+  /* Check whether there are modification before the end. */
+  if (! compare_type_names(field_charset, typelib, new_field->interval))
+    return IS_EQUAL_NO;
+
+  return IS_EQUAL_YES;
 }
 
 

=== modified file 'sql/field.h'
--- a/sql/field.h	2009-08-24 19:47:08 +0000
+++ b/sql/field.h	2009-09-29 10:58:42 +0000
@@ -472,6 +472,13 @@ public:
   /* maximum possible display length */
   virtual uint32 max_display_length()= 0;
 
+  /**
+    Whether a field being created is compatible with a existing one.
+
+    Used by the ALTER TABLE code to evaluate whether the new definition
+    of a table is compatible with the old definition so that it can
+    determine if data needs to be copied over (table data change).
+  */
   virtual uint is_equal(Create_field *new_field);
   /* convert decimal to longlong with overflow check */
   longlong convert_decimal2longlong(const my_decimal *val, bool unsigned_flag,
@@ -1862,7 +1869,6 @@ public:
   CHARSET_INFO *sort_charset(void) const { return &my_charset_bin; }
 private:
   int do_save_field_metadata(uchar *first_byte);
-  bool compare_enum_values(TYPELIB *values);
   uint is_equal(Create_field *new_field);
 };
 


Attachment: [text/bzr-bundle] bzr/davi.arnaut@sun.com-20090929105842-lg3x1tusk1g6481f.bundle
Thread
bzr commit into mysql-5.1-bugteam branch (davi:3146) Bug#45567Davi Arnaut29 Sep