List:Commits« Previous MessageNext Message »
From:Tatiana A. Nurnberg Date:June 24 2009 11:02am
Subject:bzr commit into mysql-5.1-bugteam branch (azundris:2973) Bug#43508
View as plain text  
#At file:///misc/mysql/forest/43508/51-43508/ based on revid:aelkin@stripped

 2973 Tatiana A. Nurnberg	2009-06-24
      Bug#43508: Renaming timestamp or date column triggers table copy
      
      Altering a table to update a column with types DATE or TIMESTAMP
      would incorrectly be seen as a significant change that necessitates
      a slow copy+rename operation instead of a fast update.
      
      There were two problems:
      
      The character set is magically set for TIMESTAMP to be "binary",
      but that was done too deep in field use code for ALTER TABLE to
      know of it.  Now, put that in the constructor for Field_timestamp.
      
      Also, when we set the character set for the new replacement/
      comparison field, also raise the "binary" field flag that tells us
      we should compare it exactly.  That is necessary to match the old
      stored definition.
      
      Next is the problem that the default length for TIMESTAMP and DATE
      fields is different than the length read from the .frm .  The
      compressed size is written to the file, but the human-readable,
      part-delimited length is used as default length.  IIRC, for
      timestamp it was 19!=14, and for date it was 8!=10.  Length
      mismatch causes a table copy.
      
      Also, clean up a place where a comparison function alters one of its
      parameters and replace it with an assertion of the condition it
      mutates.

    modified:
      sql/field.cc
      sql/field.h
      sql/sql_table.cc
=== modified file 'sql/field.cc'
--- a/sql/field.cc	2009-06-09 16:44:26 +0000
+++ b/sql/field.cc	2009-06-24 11:02:20 +0000
@@ -1,4 +1,4 @@
-/* Copyright 2000-2008 MySQL AB, 2008 Sun Microsystems, Inc.
+/* Copyright 2000-2008 MySQL AB, 2008-2009 Sun Microsystems, Inc.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -4723,6 +4723,7 @@ Field_timestamp::Field_timestamp(uchar *
 {
   /* For 4.0 MYD and 4.0 InnoDB compatibility */
   flags|= ZEROFILL_FLAG | UNSIGNED_FLAG;
+  field_charset= &my_charset_bin;
   if (!share->timestamp_field && unireg_check != NONE)
   {
     /* This timestamp has auto-update */
@@ -4743,6 +4744,7 @@ Field_timestamp::Field_timestamp(bool ma
 {
   /* For 4.0 MYD and 4.0 InnoDB compatibility */
   flags|= ZEROFILL_FLAG | UNSIGNED_FLAG;
+  field_charset= &my_charset_bin;
     if (unireg_check != TIMESTAMP_DN_FIELD)
       flags|= ON_UPDATE_NOW_FLAG;
 }
@@ -6504,20 +6506,9 @@ uint Field::is_equal(Create_field *new_f
 }
 
 
-/* If one of the fields is binary and the other one isn't return 1 else 0 */
-
-bool Field_str::compare_str_field_flags(Create_field *new_field, uint32 flag_arg)
-{
-  return (((new_field->flags & (BINCMP_FLAG | BINARY_FLAG)) &&
-          !(flag_arg & (BINCMP_FLAG | BINARY_FLAG))) ||
-         (!(new_field->flags & (BINCMP_FLAG | BINARY_FLAG)) &&
-          (flag_arg & (BINCMP_FLAG | BINARY_FLAG))));
-}
-
-
 uint Field_str::is_equal(Create_field *new_field)
 {
-  if (compare_str_field_flags(new_field, flags))
+  if (field_flags_are_binary() != new_field->field_flags_are_binary())
     return 0;
 
   return ((new_field->sql_type == real_type()) &&
@@ -8283,7 +8274,7 @@ uint Field_blob::max_packed_col_length(u
 
 uint Field_blob::is_equal(Create_field *new_field)
 {
-  if (compare_str_field_flags(new_field, flags))
+  if (field_flags_are_binary() != new_field->field_flags_are_binary())
     return 0;
 
   return ((new_field->sql_type == get_blob_type_from_length(max_data_length()))
@@ -9569,7 +9560,7 @@ bool Create_field::init(THD *thd, char *
     }
 
     if (length == 0)
-      fld_length= 0; /* purecov: inspected */
+      fld_length= NULL; /* purecov: inspected */
   }
 
   sign_len= fld_type_modifier & UNSIGNED_FLAG ? 0 : 1;
@@ -9721,8 +9712,7 @@ bool Create_field::init(THD *thd, char *
   case MYSQL_TYPE_TIMESTAMP:
     if (fld_length == NULL)
     {
-      /* Compressed date YYYYMMDDHHMMSS */
-      length= MAX_DATETIME_COMPRESSED_WIDTH;
+      length= MAX_DATETIME_WIDTH;
     }
     else if (length != MAX_DATETIME_WIDTH)
     {
@@ -9787,7 +9777,7 @@ bool Create_field::init(THD *thd, char *
       sql_type= MYSQL_TYPE_NEWDATE;
     /* fall trough */
   case MYSQL_TYPE_NEWDATE:
-    length= 10;
+    length= MAX_DATE_WIDTH;
     break;
   case MYSQL_TYPE_TIME:
     length= 10;
@@ -9868,6 +9858,17 @@ bool Create_field::init(THD *thd, char *
     DBUG_RETURN(TRUE);
   }
 
+  switch (fld_type) {
+  case MYSQL_TYPE_DATE:
+  case MYSQL_TYPE_NEWDATE:
+  case MYSQL_TYPE_TIME:
+  case MYSQL_TYPE_DATETIME:
+  case MYSQL_TYPE_TIMESTAMP:
+    charset= &my_charset_bin;
+    flags|= BINCMP_FLAG;
+  default: break;
+  }
+
   DBUG_RETURN(FALSE); /* success */
 }
 
@@ -9976,16 +9977,6 @@ Field *make_field(TABLE_SHARE *share, uc
     null_bit= ((uchar) 1) << null_bit;
   }
 
-  switch (field_type) {
-  case MYSQL_TYPE_DATE:
-  case MYSQL_TYPE_NEWDATE:
-  case MYSQL_TYPE_TIME:
-  case MYSQL_TYPE_DATETIME:
-  case MYSQL_TYPE_TIMESTAMP:
-    field_charset= &my_charset_bin;
-  default: break;
-  }
-
   if (f_is_alpha(pack_flag))
   {
     if (!f_is_packed(pack_flag))

=== modified file 'sql/field.h'
--- a/sql/field.h	2009-06-09 16:44:26 +0000
+++ b/sql/field.h	2009-06-24 11:02:20 +0000
@@ -1,4 +1,4 @@
-/* Copyright 2000-2008 MySQL AB, 2008 Sun Microsystems, Inc.
+/* Copyright 2000-2008 MySQL AB, 2008, 2009 Sun Microsystems, Inc.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -603,6 +603,12 @@ protected:
     handle_int64(to, from, low_byte_first_from, table->s->db_low_byte_first);
     return from + sizeof(int64);
   }
+
+  bool field_flags_are_binary()
+  {
+    return (flags & (BINCMP_FLAG | BINARY_FLAG)) != 0;
+  }
+
 };
 
 
@@ -658,7 +664,6 @@ public:
   friend class Create_field;
   my_decimal *val_decimal(my_decimal *);
   virtual bool str_needs_quotes() { return TRUE; }
-  bool compare_str_field_flags(Create_field *new_field, uint32 flags);
   uint is_equal(Create_field *new_field);
 };
 
@@ -1268,12 +1273,12 @@ public:
   Field_date(uchar *ptr_arg, uchar *null_ptr_arg, uchar null_bit_arg,
 	     enum utype unireg_check_arg, const char *field_name_arg,
 	     CHARSET_INFO *cs)
-    :Field_str(ptr_arg, 10, null_ptr_arg, null_bit_arg,
+    :Field_str(ptr_arg, MAX_DATE_WIDTH, null_ptr_arg, null_bit_arg,
 	       unireg_check_arg, field_name_arg, cs)
     {}
   Field_date(bool maybe_null_arg, const char *field_name_arg,
              CHARSET_INFO *cs)
-    :Field_str((uchar*) 0,10, maybe_null_arg ? (uchar*) "": 0,0,
+    :Field_str((uchar*) 0, MAX_DATE_WIDTH, maybe_null_arg ? (uchar*) "": 0,0,
 	       NONE, field_name_arg, cs) {}
   enum_field_types type() const { return MYSQL_TYPE_DATE;}
   enum ha_base_keytype key_type() const { return HA_KEYTYPE_ULONG_INT; }
@@ -1383,12 +1388,12 @@ public:
   Field_datetime(uchar *ptr_arg, uchar *null_ptr_arg, uchar null_bit_arg,
 		 enum utype unireg_check_arg, const char *field_name_arg,
 		 CHARSET_INFO *cs)
-    :Field_str(ptr_arg, 19, null_ptr_arg, null_bit_arg,
+    :Field_str(ptr_arg, MAX_DATETIME_WIDTH, null_ptr_arg, null_bit_arg,
 	       unireg_check_arg, field_name_arg, cs)
     {}
   Field_datetime(bool maybe_null_arg, const char *field_name_arg,
 		 CHARSET_INFO *cs)
-    :Field_str((uchar*) 0,19, maybe_null_arg ? (uchar*) "": 0,0,
+    :Field_str((uchar*) 0, MAX_DATETIME_WIDTH, maybe_null_arg ? (uchar*) "": 0,0,
 	       NONE, field_name_arg, cs) {}
   enum_field_types type() const { return MYSQL_TYPE_DATETIME;}
 #ifdef HAVE_LONG_LONG
@@ -2061,6 +2066,11 @@ public:
             Item *on_update_value, LEX_STRING *comment, char *change,
             List<String> *interval_list, CHARSET_INFO *cs,
             uint uint_geom_type);
+
+  bool field_flags_are_binary()
+  {
+    return (flags & (BINCMP_FLAG | BINARY_FLAG)) != 0;
+  }
 };
 
 

=== modified file 'sql/sql_table.cc'
--- a/sql/sql_table.cc	2009-06-19 08:24:43 +0000
+++ b/sql/sql_table.cc	2009-06-24 11:02:20 +0000
@@ -2468,8 +2468,10 @@ mysql_prepare_create_table(THD *thd, HA_
       executing a prepared statement for the second time.
     */
     sql_field->length= sql_field->char_length;
-    if (!sql_field->charset)
+
+    if (sql_field->charset == NULL)
       sql_field->charset= create_info->default_table_charset;
+
     /*
       table_charset is set in ALTER TABLE if we want change character set
       for all varchar/char columns.
@@ -5470,8 +5472,8 @@ compare_tables(TABLE *table,
   Field **f_ptr, *field;
   uint changes= 0, tmp;
   uint key_count;
-  List_iterator_fast<Create_field> new_field_it, tmp_new_field_it;
-  Create_field *new_field, *tmp_new_field;
+  List_iterator_fast<Create_field> tmp_new_field_it;
+  Create_field *tmp_new_field;
   KEY_PART_INFO *key_part;
   KEY_PART_INFO *end;
   THD *thd= table->in_use;
@@ -5497,6 +5499,9 @@ compare_tables(TABLE *table,
     pass it to mysql_prepare_create_table, then use the result
     to evaluate possibility of fast ALTER TABLE, and then
     destroy the copy.
+
+    We shouldn't need to refer later in the function to alter_info 
+    after this step.
   */
   Alter_info tmp_alter_info(*alter_info, thd->mem_root);
   uint db_options= 0; /* not used */
@@ -5508,6 +5513,7 @@ compare_tables(TABLE *table,
                                  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)) ||
@@ -5541,7 +5547,7 @@ compare_tables(TABLE *table,
     prior to 5.0 branch.
     See BUG#6236.
   */
-  if (table->s->fields != alter_info->create_list.elements ||
+  if (table->s->fields != tmp_alter_info.create_list.elements ||
       table->s->db_type() != create_info->db_type ||
       table->s->tmp_table ||
       create_info->used_fields & HA_CREATE_USED_ENGINE ||
@@ -5550,7 +5556,7 @@ compare_tables(TABLE *table,
       (table->s->row_type != create_info->row_type) ||
       create_info->used_fields & HA_CREATE_USED_PACK_KEYS ||
       create_info->used_fields & HA_CREATE_USED_MAX_ROWS ||
-      (alter_info->flags & (ALTER_RECREATE | ALTER_FOREIGN_KEY)) ||
+      (tmp_alter_info.flags & (ALTER_RECREATE | ALTER_FOREIGN_KEY)) ||
       order_num ||
       !table->s->mysql_version ||
       (table->s->frm_version < FRM_VER_TRUE_VARCHAR && varchar))
@@ -5563,22 +5569,19 @@ 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++,
-       tmp_new_field= tmp_new_field_it++;
+
+
+  for (f_ptr= table->field, tmp_new_field= tmp_new_field_it++;
        (field= *f_ptr);
-       f_ptr++, new_field= new_field_it++,
-       tmp_new_field= tmp_new_field_it++)
+       f_ptr++, 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;
+    DBUG_ASSERT(tmp_new_field->charset != NULL);
 
     /* Check that NULL behavior is same for old and new fields */
     if ((tmp_new_field->flags & NOT_NULL_FLAG) !=


Attachment: [text/bzr-bundle] bzr/azundris@mysql.com-20090624110220-nafuyo7xbpf3ngg1.bundle
Thread
bzr commit into mysql-5.1-bugteam branch (azundris:2973) Bug#43508Tatiana A. Nurnberg24 Jun
  • Re: bzr commit into mysql-5.1-bugteam branch (azundris:2973) Bug#43508Gleb Shchepa29 Jul
    • Re: bzr commit into mysql-5.1-bugteam branch (azundris:2973) Bug#43508Tatiana Azundris9 Oct