List:Commits« Previous MessageNext Message »
From:Dmitry Lenev Date:November 22 2011 8:56am
Subject:bzr push into mysql-trunk-wl5534 branch (Dmitry.Lenev:3431 to 3432) WL#5534
View as plain text  
 3432 Dmitry Lenev	2011-11-22
      WL#5534 Online ALTER, Phase 1.
      
      Patch #53: Review changes.
      
      * Marked FIELD_IN_ADD_INDEX flag as deprecated and
        limited its usage only to legacy in-place ALTER API.
      * Removed separate CHANGE_CHARACTER_SET and
        SET_DEFAULT_CHARACTER_SET flags from in-place
        ALTER API.
      * Introduced new ALTER_COLUMN_EQUAL_PACK_LENGTH flag
        in order to support optimization of changing column
        types which keep packed representation compatible.

    modified:
      include/mysql_com.h
      sql/handler.cc
      sql/handler.h
      sql/sql_table.cc
 3431 Jon Olav Hauglid	2011-11-21 [merge]
      Merge from mysql-trunk to mysql-trunk-wl5534
      Text conflict in sql/sql_table.cc

    added:
      mysql-test/suite/binlog/r/binlog_hexdump.result
      mysql-test/suite/binlog/std_data/bug11747887-bin.000003
      mysql-test/suite/binlog/t/binlog_hexdump.test
    modified:
      mysql-test/suite/innodb_fts/r/innodb_fts_misc.result
      mysql-test/suite/innodb_fts/r/innodb_fts_misc_1.result
      mysql-test/suite/innodb_fts/t/innodb_fts_misc.test
      mysql-test/suite/innodb_fts/t/innodb_fts_misc_1.test
      sql/handler.cc
      sql/log_event.cc
      storage/innobase/fts/fts0que.c
      storage/innobase/row/row0ftsort.c
      storage/innobase/row/row0merge.c
      storage/innobase/srv/srv0start.c
=== modified file 'include/mysql_com.h'
--- a/include/mysql_com.h	2011-11-03 16:03:34 +0000
+++ b/include/mysql_com.h	2011-11-22 08:55:49 +0000
@@ -112,7 +112,9 @@ enum enum_server_command
 #define BINCMP_FLAG	131072		/* Intern: Used by sql_yacc */
 #define GET_FIXED_FIELDS_FLAG (1 << 18) /* Used to get fields in item tree */
 #define FIELD_IN_PART_FUNC_FLAG (1 << 19)/* Field part of partition func */
-#define FIELD_IN_ADD_INDEX (1<< 20)	/* Intern: Field used in ADD INDEX */
+#define FIELD_IN_ADD_INDEX (1<< 20)     /* Intern: Field used in ADD INDEX.
+                                           Deprecated. Used by old in-place
+                                           ALTER TABLE API only. */
 #define FIELD_IS_RENAMED (1<< 21)       /* Intern: Field is being renamed */
 #define FIELD_FLAGS_STORAGE_MEDIA 22    /* Field storage media, bit 22-23,
                                            reserved by MySQL Cluster */

=== modified file 'sql/handler.cc'
--- a/sql/handler.cc	2011-11-21 19:02:06 +0000
+++ b/sql/handler.cc	2011-11-22 08:55:49 +0000
@@ -3806,6 +3806,7 @@ handler::check_if_supported_inplace_alte
     Alter_inplace_info::ADD_PK_INDEX |
     Alter_inplace_info::DROP_PK_INDEX;
   Alter_inplace_info::HA_ALTER_FLAGS inplace_offline_operations=
+    Alter_inplace_info::ALTER_COLUMN_EQUAL_PACK_LENGTH |
     Alter_inplace_info::ALTER_COLUMN_NAME |
     Alter_inplace_info::COLUMN_DEFAULT_VALUE |
     Alter_inplace_info::CHANGE_CREATE_OPTION;
@@ -3823,10 +3824,50 @@ handler::check_if_supported_inplace_alte
   if (is_index_maintenance_unique(ha_alter_info, table))
     DBUG_RETURN(HA_ALTER_INPLACE_NOT_SUPPORTED);
 
+  /*
+    ALTER TABLE tbl_name CONVERT TO CHARACTER SET .. and
+    ALTER TABLE table_name DEFAULT CHARSET = .. most likely
+    change column charsets and so not supported in-place through
+    old API.
+  */
+  if (create_info->used_fields & (HA_CREATE_USED_CHARSET |
+                                  HA_CREATE_USED_DEFAULT_CHARSET))
+    DBUG_RETURN(HA_ALTER_INPLACE_NOT_SUPPORTED);
+
+  /*
+    At this point we know that we won't be adding, dropping or
+    reordering columns so we can use simple algorithm to properly
+    set FIELD_IN_ADD_INDEX flag needed by old version of in-place
+    API.
+  */
+  DBUG_ASSERT(!(ha_alter_info->handler_flags &
+                (Alter_inplace_info::ADD_COLUMN |
+                 Alter_inplace_info::DROP_COLUMN |
+                 Alter_inplace_info::ALTER_COLUMN_ORDER)));
+  /* Start by clearing this marker for all fields. */
+  for (uint i= 0; i < table->s->fields; i++)
+    table->field[i]->flags&= ~FIELD_IN_ADD_INDEX;
+  /*
+    Then go through all new indexes and use the fact that
+    fields position has not changed to mark fields belonging
+    to newly added indexes.
+  */
+  for (uint i= 0; i < ha_alter_info->index_add_count; i++)
+  {
+    KEY *k= ha_alter_info->key_info_buffer + ha_alter_info->index_add_buffer[i];
+    KEY_PART_INFO *key_part, *end;
+    for(key_part= k->key_part, end= key_part + k->key_parts;
+        key_part != end; key_part++)
+      table->field[key_part->fieldnr]->flags|= FIELD_IN_ADD_INDEX;
+  }
+
   /* Is there at least one in-place operation that must be done offline? */
   if (ha_alter_info->handler_flags & inplace_offline_operations)
   {
-    if (table->file->check_if_incompatible_data(create_info, IS_EQUAL_YES)
+    uint table_changes= (ha_alter_info->handler_flags &
+                         Alter_inplace_info::ALTER_COLUMN_EQUAL_PACK_LENGTH) ?
+                        IS_EQUAL_PACK_LENGTH : IS_EQUAL_YES;
+    if (table->file->check_if_incompatible_data(create_info, table_changes)
         == COMPATIBLE_DATA_YES)
       DBUG_RETURN(HA_ALTER_INPLACE_EXCLUSIVE_LOCK);
     else

=== modified file 'sql/handler.h'
--- a/sql/handler.h	2011-11-21 18:15:24 +0000
+++ b/sql/handler.h	2011-11-22 08:55:49 +0000
@@ -985,27 +985,31 @@ public:
   // Change column datatype
   static const HA_ALTER_FLAGS ALTER_COLUMN_TYPE          = 1L << 9;
 
+  /**
+    Change column datatype in such way that new type has compatible
+    packed representation with old type, so it is theoretically
+    possible to perform change by only updating data dictionary
+    without changing table rows.
+  */
+  static const HA_ALTER_FLAGS ALTER_COLUMN_EQUAL_PACK_LENGTH = 1L << 10;
+
   // Reorder column
-  static const HA_ALTER_FLAGS ALTER_COLUMN_ORDER         = 1L << 10;
+  static const HA_ALTER_FLAGS ALTER_COLUMN_ORDER         = 1L << 11;
 
   // Change column from NULL to NOT NULL or vice versa
-  static const HA_ALTER_FLAGS ALTER_COLUMN_NULLABLE      = 1L << 11;
+  static const HA_ALTER_FLAGS ALTER_COLUMN_NULLABLE      = 1L << 12;
 
   // Set or remove default column value
-  static const HA_ALTER_FLAGS COLUMN_DEFAULT_VALUE       = 1L << 12;
+  static const HA_ALTER_FLAGS COLUMN_DEFAULT_VALUE       = 1L << 13;
 
   // Add foreign key
-  static const HA_ALTER_FLAGS ADD_FOREIGN_KEY            = 1L << 13;
+  static const HA_ALTER_FLAGS ADD_FOREIGN_KEY            = 1L << 14;
 
   // Drop foreign key
-  static const HA_ALTER_FLAGS DROP_FOREIGN_KEY           = 1L << 14;
-
-  // Change character set
-  static const HA_ALTER_FLAGS CHANGE_CHARACTER_SET       = 1L << 15;
-  static const HA_ALTER_FLAGS SET_DEFAULT_CHARACTER_SET  = 1L << 16;
+  static const HA_ALTER_FLAGS DROP_FOREIGN_KEY           = 1L << 15;
 
   // table_options changed, see HA_CREATE_INFO::used_fields for details.
-  static const HA_ALTER_FLAGS CHANGE_CREATE_OPTION       = 1L << 17;
+  static const HA_ALTER_FLAGS CHANGE_CREATE_OPTION       = 1L << 16;
 
   KEY  *key_info_buffer;
   uint key_count;

=== modified file 'sql/sql_table.cc'
--- a/sql/sql_table.cc	2011-11-21 19:02:06 +0000
+++ b/sql/sql_table.cc	2011-11-22 08:55:49 +0000
@@ -5065,31 +5065,6 @@ static Create_field *get_field_by_index(
 
 
 /**
-  Auxiliary function that marks all existing fields in newly added key
-  with FIELD_IN_ADD_INDEX flag.
-*/
-
-static void mark_old_fields_in_added_key(Alter_info *alter_info, const KEY *key)
-{
-  const KEY_PART_INFO *key_part, *end;
-  Create_field *field;
-
-  key_part= key->key_part;
-  end= key_part + key->key_parts;
-  for(; key_part != end; key_part++)
-  {
-    field= get_field_by_index(alter_info, key_part->fieldnr);
-    /*
-      If the field has existed in old version of table mark it to
-      be part of new key.
-    */
-    if (field->field)
-      field->field->flags|= FIELD_IN_ADD_INDEX;
-  }
-}
-
-
-/**
    Compare original and new versions of a table and fill Alter_inplace_info
    describing differences between those versions.
 
@@ -5210,12 +5185,6 @@ static bool fill_alter_inplace_info(THD
   if (alter_info->flags & Alter_info::ALTER_OPTIONS)
     ha_alter_info->handler_flags|= Alter_inplace_info::CHANGE_CREATE_OPTION;
 
-  /* TODO: do we want to have separate SE flags for the below cases? */
-  if (create_info->used_fields & HA_CREATE_USED_CHARSET)
-    ha_alter_info->handler_flags|= Alter_inplace_info::CHANGE_CHARACTER_SET;
-  if (create_info->used_fields & HA_CREATE_USED_DEFAULT_CHARSET)
-    ha_alter_info->handler_flags|= Alter_inplace_info::SET_DEFAULT_CHARACTER_SET;
-
   /*
     If we altering table with old VARCHAR fields we will be automatically
     upgrading VARCHAR column types.
@@ -5236,11 +5205,8 @@ static bool fill_alter_inplace_info(THD
   */
   for (f_ptr= table->field; (field= *f_ptr); f_ptr++)
   {
-    /*
-      Clear markers for renamed and indexed fields which we are going to
-      set later.
-    */
-    field->flags&= ~(FIELD_IS_RENAMED|FIELD_IN_ADD_INDEX);
+    /* Clear marker for renamed field which we are going to set later. */
+    field->flags&= ~FIELD_IS_RENAMED;
 
     /* Use transformed info to evaluate flags for storage engine. */
     uint new_field_index= 0;
@@ -5258,18 +5224,37 @@ static bool fill_alter_inplace_info(THD
 
       /*
         Check if type of column has changed to some incompatible type.
-
-        We don't have separate flag for case when IS_EQUAL_PACK_LENGTH
-        is returned as this value is a part of legacy
-        handler::check_if_incompatible_data() API and almost no storage
-        engine uses it at the moment.
-        The only exception is CSV engine that treats all tables passing
-        SQL-layer checks as compatible. I.e. it is possible to change
-        length of VARCHAR() columns for CSV engine without rebuild.
-        TODO: Does it worth separate flag?
       */
-      if ((field->is_equal(tmp_new_field) != IS_EQUAL_YES))
+      switch (field->is_equal(tmp_new_field))
+      {
+      case IS_EQUAL_NO:
+        /* New column type is incompatible with old one. */
         ha_alter_info->handler_flags|= Alter_inplace_info::ALTER_COLUMN_TYPE;
+        break;
+      case IS_EQUAL_YES:
+        /*
+          New column is the same as the old one or the fully compatible with
+          it (for example, ENUM('a','b') was changed to ENUM('a','b','c')).
+          Such a change if any can ALWAYS be carried out by simply updating
+          data-dictionary without even informing storage engine.
+          No flag is set in this case.
+        */
+        break;
+      case IS_EQUAL_PACK_LENGTH:
+        /*
+          New column type differs from the old one, but has compatible packed
+          data representation. Depending on storage engine, such a change can
+          be carried out by simply updating data dictionary without changing
+          actual data (for example, VARCHAR(300) is changed to VARCHAR(400)).
+        */
+        ha_alter_info->handler_flags|= Alter_inplace_info::
+                                         ALTER_COLUMN_EQUAL_PACK_LENGTH;
+        break;
+      default:
+        DBUG_ASSERT(0);
+        /* Safety. */
+        ha_alter_info->handler_flags|= Alter_inplace_info::ALTER_COLUMN_TYPE;
+      }
 
       /* Check if field was renamed */
       if (my_strcasecmp(system_charset_info, field->field_name,
@@ -5407,11 +5392,6 @@ static bool fill_alter_inplace_info(THD
       [ha_alter_info->index_add_count++]=
       new_key - ha_alter_info->key_info_buffer;
     /* Mark all old fields which are used in newly created index. */
-    /*
-      TODO: Check with Cluster if this step is still necessary.
-      It doesn't make much sense to me...
-    */
-    mark_old_fields_in_added_key(&tmp_alter_info, new_key);
     DBUG_PRINT("info", ("index changed: '%s'", table_key->name));
   }
   /*end of for (; table_key < table_key_end;) */
@@ -5435,8 +5415,6 @@ static bool fill_alter_inplace_info(THD
       ha_alter_info->index_add_buffer
         [ha_alter_info->index_add_count++]=
         new_key - ha_alter_info->key_info_buffer;
-      /* Mark all old fields which are used in newly created index. */
-      mark_old_fields_in_added_key(&tmp_alter_info, new_key);
       DBUG_PRINT("info", ("index added: '%s'", new_key->name));
     }
   }

No bundle (reason: useless for push emails).
Thread
bzr push into mysql-trunk-wl5534 branch (Dmitry.Lenev:3431 to 3432) WL#5534Dmitry Lenev22 Nov