List:Internals« Previous MessageNext Message »
From:ingo Date:September 26 2005 7:30pm
Subject:bk commit into 5.1 tree (ingo:1.1909)
View as plain text  
Below is the list of changes that have just been committed into a local
5.1 repository of mydev. When mydev 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
  1.1909 05/09/26 19:30:30 ingo@stripped +5 -0
  WL#1563 - Modify MySQL to support on-line CREATE/DROP INDEX
  Not to be pushed.
  This is the second preliminary prototype for demo purposes.
  It shows the direction of development for early review.
  The new methods in myisam.h are WRONG. They are here to show
  how the new interface can be used in a handler.
  They MUST be removed before pushing.
  The changes in sql_table.cc are incomplete.

  sql/sql_table.cc
    1.273 05/09/26 19:30:24 ingo@stripped +398 -291
    WL#1563 - Modify MySQL to support on-line CREATE/DROP INDEX
    Not to be pushed.
    This is the second preliminary prototype for demo purposes.
    It shows the direction of development for early review.
    In mysql_prepare_table() allow an index to have the name
    "PRIMARY" if it has the key type "Key::PRIMARY".
    Removed old code from the last attempt.
    Preliminary defined a set of key flags which are relevant
    for comparison of key types.
    Some changes to compare_tables():
    - Input parameter "List<Key> *key_list" is replaced by
      "KEY *key_info_buffer, uint key_count".
    - Output parameters added: "index_drop_buffer/index_drop_count"
      and "index_add_buffer/index_add_count".
    - Key comparison must now find matching keys in changed
      old and new key lists.
    - Key comparison of a key is easier now because both old
      and new keys are of type 'KEY'.
    Call mysql_prepare_table() before compare_tables(). The
    translated KEY structs are needed at some places now.

  sql/handler.h
    1.160 05/09/26 19:30:24 ingo@stripped +28 -15
    WL#1563 - Modify MySQL to support on-line CREATE/DROP INDEX
    Added new flags.
    Removed old flags and methods (from the last attempt).

  sql/ha_myisam.h
    1.70 05/09/26 19:30:24 ingo@stripped +35 -0
    WL#1563 - Modify MySQL to support on-line CREATE/DROP INDEX
    Not to be pushed.
    This is the second preliminary prototype for demo purposes.
    It shows the direction of development for early review.
    The new methods in myisam.h are WRONG. They are here to show
    how the new interface can be used in a handler.
    They MUST be removed before pushing.

  mysql-test/t/key.test
    1.25 05/09/26 19:30:24 ingo@stripped +50 -0
    WL#1563 - Modify MySQL to support on-line CREATE/DROP INDEX
    The test case.

  mysql-test/r/key.result
    1.29 05/09/26 19:30:24 ingo@stripped +63 -0
    WL#1563 - Modify MySQL to support on-line CREATE/DROP INDEX
    The test result.

# 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:	ingo
# Host:	chilla.local
# Root:	/home/mydev/mysql-5.1-wl1563

--- 1.69/sql/ha_myisam.h	2005-07-22 22:47:35 +02:00
+++ 1.70/sql/ha_myisam.h	2005-09-26 19:30:24 +02:00
@@ -109,6 +109,41 @@
   int create(const char *name, TABLE *form, HA_CREATE_INFO *create_info);
   THR_LOCK_DATA **store_lock(THD *thd, THR_LOCK_DATA **to,
 			     enum thr_lock_type lock_type);
+
+  /*
+    The following are DUMMY function for test and demo purposes only.
+    They MUST be deleted before pushing.
+    They show, how the interface can be used in the handler.
+  */
+  ulong alter_table_flags(void) const
+  { return HA_ONLINE_ADD_INDEX_NO_WRITES | HA_ONLINE_DROP_INDEX_NO_WRITES |
+           HA_ONLINE_ADD_UNIQUE_INDEX | HA_ONLINE_DROP_PK_INDEX ; }
+  int add_index(TABLE *table_arg, KEY *key_info, uint num_of_keys)
+  {
+    uint idx;
+    for (idx= 0; idx < num_of_keys; idx++)
+    {
+      DBUG_PRINT("ingo", ("myisam_add_index: '%s'", key_info[idx].name));
+      KEY_PART_INFO *part_end= key_info->key_part + key_info->key_parts;
+      KEY_PART_INFO *key_part;
+      for (key_part= key_info->key_part; key_part < part_end; key_part++)
+        DBUG_PRINT("ingo", ("myisam_add_index: field: '%s'",
+                            key_part->field->field_name));
+    }
+    return (0);
+  }
+  int prepare_drop_index(TABLE *table_arg, uint *key_num,
+                                 uint num_of_keys)
+  {
+    uint idx;
+    for (idx= 0; idx < num_of_keys; idx++)
+      DBUG_PRINT("ingo", ("myisam_drop_index: '%s'",
+                          table_arg->key_info[key_num[idx]].name));
+    return (0);
+  }
+  int final_drop_index(TABLE *table_arg)
+  { return (0); }
+
   ulonglong get_auto_increment();
   int rename_table(const char * from, const char * to);
   int delete_table(const char *name);

--- 1.159/sql/handler.h	2005-09-20 16:29:53 +02:00
+++ 1.160/sql/handler.h	2005-09-26 19:30:24 +02:00
@@ -104,8 +104,29 @@
 #define HA_KEYREAD_ONLY         64	/* Support HA_EXTRA_KEYREAD */
 
 /* bits in alter_table_flags */
-#define HA_ONLINE_ADD_EMPTY_PARTITION 1
-#define HA_ONLINE_DROP_PARTITION 2
+#define HA_ONLINE_ADD_EMPTY_PARTITION           0x00000001
+#define HA_ONLINE_DROP_PARTITION                0x00000002
+/*
+  These bits are set if different kinds of indexes can be created
+  off-line without re-create of the table (but with a table lock).
+*/
+#define HA_ONLINE_ADD_INDEX_NO_WRITES           0x00000004 /*add index w/lock*/
+#define HA_ONLINE_DROP_INDEX_NO_WRITES          0x00000008 /*drop index w/lock*/
+#define HA_ONLINE_ADD_UNIQUE_INDEX_NO_WRITES    0x00000010 /*add unique w/lock*/
+#define HA_ONLINE_DROP_UNIQUE_INDEX_NO_WRITES   0x00000020 /*drop uniq. w/lock*/
+#define HA_ONLINE_ADD_PK_INDEX_NO_WRITES        0x00000040 /*add prim. w/lock*/
+#define HA_ONLINE_DROP_PK_INDEX_NO_WRITES       0x00000080 /*drop prim. w/lock*/
+/*
+  These are set if different kinds of indexes can be created on-line
+  (without a table lock). If a handler is capable of one or more of
+  these, it should also set the corresponding *_NO_WRITES bit(s).
+*/
+#define HA_ONLINE_ADD_INDEX                     0x00000100 /*add index online*/
+#define HA_ONLINE_DROP_INDEX                    0x00000200 /*drop index online*/
+#define HA_ONLINE_ADD_UNIQUE_INDEX              0x00000400 /*add unique online*/
+#define HA_ONLINE_DROP_UNIQUE_INDEX             0x00000800 /*drop uniq. online*/
+#define HA_ONLINE_ADD_PK_INDEX                  0x00001000 /*add prim. online*/
+#define HA_ONLINE_DROP_PK_INDEX                 0x00002000 /*drop prim. online*/
 
 /* operations for disable/enable indexes */
 #define HA_KEY_SWITCH_NONUNIQ      0
@@ -122,16 +143,6 @@
 #define MAX_HA 14
 
 /*
-  Bits in index_ddl_flags(KEY *wanted_index)
-  for what ddl you can do with index
-  If none is set, the wanted type of index is not supported
-  by the handler at all. See WorkLog 1563.
-*/
-#define HA_DDL_SUPPORT   1 /* Supported by handler */
-#define HA_DDL_WITH_LOCK 2 /* Can create/drop with locked table */
-#define HA_DDL_ONLINE    4 /* Can create/drop without lock */
-
-/*
   Parameters for open() (in register form->filestat)
   HA_GET_INFO does an implicit HA_ABORT_IF_LOCKED
 */
@@ -1215,11 +1226,13 @@
   virtual int get_default_no_partitions(ulonglong max_rows) { return 1;}
 #endif
   virtual ulong index_flags(uint idx, uint part, bool all_parts) const =0;
-  virtual ulong index_ddl_flags(KEY *wanted_index) const
-  { return (HA_DDL_SUPPORT); }
+
   virtual int add_index(TABLE *table_arg, KEY *key_info, uint num_of_keys)
   { return (HA_ERR_WRONG_COMMAND); }
-  virtual int drop_index(TABLE *table_arg, uint *key_num, uint num_of_keys)
+  virtual int prepare_drop_index(TABLE *table_arg, uint *key_num,
+                                 uint num_of_keys)
+  { return (HA_ERR_WRONG_COMMAND); }
+  virtual int final_drop_index(TABLE *table_arg)
   { return (HA_ERR_WRONG_COMMAND); }
 
   uint max_record_length() const

--- 1.272/sql/sql_table.cc	2005-09-20 16:29:53 +02:00
+++ 1.273/sql/sql_table.cc	2005-09-26 19:30:24 +02:00
@@ -691,10 +691,16 @@
 
   SYNOPSIS
     mysql_prepare_table()
-    thd			Thread object
-    create_info		Create information (like MAX_ROWS)
-    fields		List of fields to create
-    keys		List of keys to create
+      thd                       Thread object.
+      create_info               Create information (like MAX_ROWS).
+      fields                    List of fields to create.
+      keys                      List of keys to create.
+      tmp_table                 If a temporary table is to be created.
+      db_options          INOUT Table options (like HA_OPTION_PACK_RECORD).
+      file                      The handler for the new table.
+      key_info_buffer     OUT   An array of KEY structs for the indexes.
+      key_count           OUT   The number of elements in the array.
+      select_field_count        The number of fields coming from a select table.
 
   DESCRIPTION
     Prepares the table and key structures for table creation.
@@ -979,6 +985,8 @@
 
   while ((key=key_iterator++))
   {
+    DBUG_PRINT("info", ("key name: '%s'  type: %d", key->name ? key->name :
+                        "(none)" , key->type));
     if (key->type == Key::FOREIGN_KEY)
     {
       fk_key_count++;
@@ -1039,7 +1047,7 @@
       key_parts+=key->columns.elements;
     else
       (*key_count)--;
-    if (key->name && !tmp_table &&
+    if (key->name && !tmp_table && (key->type != Key::PRIMARY)
&&
 	!my_strcasecmp(system_charset_info,key->name,primary_key_name))
     {
       my_error(ER_WRONG_NAME_FOR_INDEX, MYF(0), key->name);
@@ -1053,7 +1061,7 @@
     DBUG_RETURN(-1);
   }
 
-  (*key_info_buffer) = key_info= (KEY*) sql_calloc(sizeof(KEY)* *key_count);
+  (*key_info_buffer)= key_info= (KEY*) sql_calloc(sizeof(KEY)* *key_count);
   key_part_info=(KEY_PART_INFO*) sql_calloc(sizeof(KEY_PART_INFO)*key_parts);
   if (!*key_info_buffer || ! key_part_info)
     DBUG_RETURN(-1);				// Out of memory
@@ -2987,219 +2995,30 @@
 }
 
 
-#ifdef NOT_USED
-/*
-  CREATE INDEX and DROP INDEX are implemented by calling ALTER TABLE with
-  the proper arguments.  This isn't very fast but it should work for most
-  cases.
-  One should normally create all indexes with CREATE TABLE or ALTER TABLE.
-*/
-
-int mysql_create_indexes(THD *thd, TABLE_LIST *table_list, List<Key> &keys)
-{
-  List<create_field> fields;
-  List<Alter_drop>   drop;
-  List<Alter_column> alter;
-  HA_CREATE_INFO     create_info;
-  int		     rc;
-  uint		     idx;
-  uint		     db_options;
-  uint		     key_count;
-  TABLE		     *table;
-  Field		     **f_ptr;
-  KEY		     *key_info_buffer;
-  char		     path[FN_REFLEN+1];
-  DBUG_ENTER("mysql_create_index");
-
-  /*
-    Try to use online generation of index.
-    This requires that all indexes can be created online.
-    Otherwise, the old alter table procedure is executed.
-
-    Open the table to have access to the correct table handler.
-  */
-  if (!(table=open_ltable(thd,table_list,TL_WRITE_ALLOW_READ)))
-    DBUG_RETURN(-1);
-
-  /*
-    The add_index method takes an array of KEY structs for the new indexes.
-    Preparing a new table structure generates this array.
-    It needs a list with all fields of the table, which does not need to
-    be correct in every respect. The field names are important.
-  */
-  for (f_ptr= table->field; *f_ptr; f_ptr++)
-  {
-    create_field *c_fld= new create_field(*f_ptr, *f_ptr);
-    c_fld->unireg_check= Field::NONE; /*avoid multiple auto_increments*/
-    fields.push_back(c_fld);
-  }
-  bzero((char*) &create_info,sizeof(create_info));
-  create_info.db_type=DB_TYPE_DEFAULT;
-  create_info.default_table_charset= thd->variables.collation_database;
-  db_options= 0;
-  if (mysql_prepare_table(thd, &create_info, &fields,
-			  &keys, /*tmp_table*/ 0, &db_options, table->file,
-			  &key_info_buffer, key_count,
-			  /*select_field_count*/ 0))
-    DBUG_RETURN(-1);
-
-  /*
-    Check if all keys can be generated with the add_index method.
-    If anyone cannot, then take the old way.
-  */
-  for (idx=0; idx< key_count; idx++)
-  {
-    DBUG_PRINT("info", ("creating index %s", key_info_buffer[idx].name));
-    if (!(table->file->index_ddl_flags(key_info_buffer+idx)&
-	  (HA_DDL_ONLINE| HA_DDL_WITH_LOCK)))
-      break ;
-  }
-  if ((idx < key_count)|| !key_count)
-  {
-    /* Re-initialize the create_info, which was changed by prepare table. */
-    bzero((char*) &create_info,sizeof(create_info));
-    create_info.db_type=DB_TYPE_DEFAULT;
-    create_info.default_table_charset= thd->variables.collation_database;
-    /* Cleanup the fields list. We do not want to create existing fields. */
-    fields.delete_elements();
-    if (real_alter_table(thd, table_list->db, table_list->table_name,
-			 &create_info, table_list, table,
-			 fields, keys, drop, alter, 0, (ORDER*)0,
-			 ALTER_ADD_INDEX, DUP_ERROR))
-      /* Don't need to free((gptr) key_info_buffer);*/
-      DBUG_RETURN(-1);
-  }
-  else
-  {
-    if (table->file->add_index(table, key_info_buffer, key_count)||
-        build_table_path(path, sizeof(path), table_list->db,
-                         (lower_case_table_names == 2) ?
-                         table_list->alias : table_list->table_name,
-                         reg_ext) == 0 ||
-	mysql_create_frm(thd, path, &create_info,
-			 fields, key_count, key_info_buffer, table->file))
-      /* don't need to free((gptr) key_info_buffer);*/
-      DBUG_RETURN(-1);
-  }
-  /* don't need to free((gptr) key_info_buffer);*/
-  DBUG_RETURN(0);
-}
-
-
-int mysql_drop_indexes(THD *thd, TABLE_LIST *table_list,
-		       List<Alter_drop> &drop)
-{
-  List<create_field> fields;
-  List<Key>	     keys;
-  List<Alter_column> alter;
-  HA_CREATE_INFO     create_info;
-  uint		     idx;
-  uint		     db_options;
-  uint		     key_count;
-  uint		     *key_numbers;
-  TABLE		     *table;
-  Field		     **f_ptr;
-  KEY		     *key_info;
-  KEY		     *key_info_buffer;
-  char		     path[FN_REFLEN];
-  DBUG_ENTER("mysql_drop_index");
-
-  /*
-    Try to use online generation of index.
-    This requires that all indexes can be created online.
-    Otherwise, the old alter table procedure is executed.
-
-    Open the table to have access to the correct table handler.
-  */
-  if (!(table=open_ltable(thd,table_list,TL_WRITE_ALLOW_READ)))
-    DBUG_RETURN(-1);
-
-  /*
-    The drop_index method takes an array of key numbers.
-    It cannot get more entries than keys in the table.
-  */
-  key_numbers= (uint*) thd->alloc(sizeof(uint*)*table->keys);
-  key_count= 0;
-
-  /*
-    Get the number of each key and check if it can be created online.
-  */
-  List_iterator<Alter_drop> drop_it(drop);
-  Alter_drop *drop_key;
-  while ((drop_key= drop_it++))
-  {
-    /* Find the key in the table. */
-    key_info=table->key_info;
-    for (idx=0; idx< table->keys; idx++, key_info++)
-    {
-      if (!my_strcasecmp(system_charset_info, key_info->name, drop_key->name))
-	break;
-    }
-    if (idx>= table->keys)
-    {
-      my_error(ER_CANT_DROP_FIELD_OR_KEY, MYF(0), drop_key->name);
-      /*don't need to free((gptr) key_numbers);*/
-      DBUG_RETURN(-1);
-    }
-    /*
-      Check if the key can be generated with the add_index method.
-      If anyone cannot, then take the old way.
-    */
-    DBUG_PRINT("info", ("dropping index %s", table->key_info[idx].name));
-    if (!(table->file->index_ddl_flags(table->key_info+idx)&
-	  (HA_DDL_ONLINE| HA_DDL_WITH_LOCK)))
-      break ;
-    key_numbers[key_count++]= idx;
-  }
-
-  bzero((char*) &create_info,sizeof(create_info));
-  create_info.db_type=DB_TYPE_DEFAULT;
-  create_info.default_table_charset= thd->variables.collation_database;
-
-  if ((drop_key)|| (drop.elements<= 0))
-  {
-    if (real_alter_table(thd, table_list->db, table_list->table_name,
-			 &create_info, table_list, table,
-			 fields, keys, drop, alter, 0, (ORDER*)0,
-			 ALTER_DROP_INDEX, DUP_ERROR))
-      /*don't need to free((gptr) key_numbers);*/
-      DBUG_RETURN(-1);
-  }
-  else
-  {
-    db_options= 0;
-    if (table->file->drop_index(table, key_numbers, key_count)||
-	mysql_prepare_table(thd, &create_info, &fields,
-			    &keys, /*tmp_table*/ 0, &db_options, table->file,
-			    &key_info_buffer, key_count,
-			    /*select_field_count*/ 0)||
-        build_table_path(path, sizeof(path), table_list->db,
-                         (lower_case_table_names == 2) ?
-                         table_list->alias : table_list->table_name,
-                         reg_ext) == 0 ||
-	mysql_create_frm(thd, path, &create_info,
-			 fields, key_count, key_info_buffer, table->file))
-      /*don't need to free((gptr) key_numbers);*/
-      DBUG_RETURN(-1);
-  }
-
-  /*don't need to free((gptr) key_numbers);*/
-  DBUG_RETURN(0);
-}
-#endif /* NOT_USED */
-
-
-
 #define ALTER_TABLE_DATA_CHANGED  1
 #define ALTER_TABLE_INDEX_CHANGED 2
+/*
+  This should go to include/my_base.h later.
+  A set of flags which is relevant for comparison of key types.
+ */
+#define HA_KEYFLAG_MASK (HA_NOSAME | HA_PACK_KEY | HA_AUTO_KEY | \
+                         HA_BINARY_PACK_KEY | HA_FULLTEXT | HA_UNIQUE_CHECK | \
+                         HA_SPATIAL | HA_NULL_ARE_EQUAL | HA_GENERATED_KEY)
 
 /*
   SYNOPSIS
-    compare tables()
-    table       original table
-    create_list fields in new table
-    key_list    keys in new table
-    create_info create options in new table
+    compare_tables()
+      table                     The original table.
+      create_list               The fields for the new table.
+      key_info_buffer           An array of KEY structs for the new indexes.
+      key_count                 The number of elements in the array.
+      create_info               Create options for the new table.
+      alter_info                Alter options.
+      order_num                 Number of order list elements.
+      index_drop_buffer   OUT   An array of offsets into table->key_info.
+      index_drop_count    OUT   The number of elements in the array.
+      index_add_buffer    OUT   An array of offsets into key_info_buffer.
+      index_add_count     OUT   The number of elements in the array.
 
   DESCRIPTION
     'table' (first argument) contains information of the original
@@ -3211,20 +3030,29 @@
     and whether we need to make a copy of the table, or just change
     the .frm file.
 
+    If there are no data changes, but index changes, 'index_drop_buffer'
+    and/or 'index_add_buffer' are populated with offsets into
+    table->key_info or key_info_buffer respectively for the indexes
+    that need to be dropped and/or (re-)created.
+
   RETURN VALUES
-    0 No copy needed
-    1 Data changes, copy needed
-    2 Index changes, copy needed   
+    0                           No copy needed
+    ALTER_TABLE_DATA_CHANGED    Data changes, copy needed
+    ALTER_TABLE_INDEX_CHANGED   Index changes, copy might be needed
 */
 
 uint compare_tables(TABLE *table, List<create_field> *create_list,
-		    List<Key> *key_list, HA_CREATE_INFO *create_info,
-		    ALTER_INFO *alter_info, uint order_num)
+                    KEY *key_info_buffer, uint key_count,
+                    HA_CREATE_INFO *create_info,
+                    ALTER_INFO *alter_info, uint order_num,
+                    uint *index_drop_buffer, uint *index_drop_count,
+                    uint *index_add_buffer, uint *index_add_count)
 {
   Field **f_ptr, *field;
   uint changes= 0, tmp;
   List_iterator_fast<create_field> new_field_it(*create_list);
   create_field *new_field;
+  DBUG_ENTER("compare_tables");
 
   /*
     Some very basic checks. If number of fields changes, or the
@@ -3252,7 +3080,7 @@
       create_info->used_fields & HA_CREATE_USED_DEFAULT_CHARSET ||
       (alter_info->flags & ALTER_RECREATE) ||
       order_num)
-    return ALTER_TABLE_DATA_CHANGED;
+    DBUG_RETURN(ALTER_TABLE_DATA_CHANGED);
 
   /*
     Go through fields and check if the original ones are compatible
@@ -3264,11 +3092,11 @@
     /* 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) !=
 	(uint) (field->flags & NOT_NULL_FLAG))
-      return ALTER_TABLE_DATA_CHANGED;
+      DBUG_RETURN(ALTER_TABLE_DATA_CHANGED);
 
     /* Don't pack rows in old tables if the user has requested this. */
     if (create_info->row_type == ROW_TYPE_DYNAMIC ||
@@ -3279,73 +3107,120 @@
 
     /* Evaluate changes bitmap and send to check_if_incompatible_data() */
     if (!(tmp= field->is_equal(new_field)))
-      return ALTER_TABLE_DATA_CHANGED;
+      DBUG_RETURN(ALTER_TABLE_DATA_CHANGED);
 
     changes|= tmp;
   }
   /* Check if changes are compatible with current handler without a copy */
   if (table->file->check_if_incompatible_data(create_info, changes))
-    return ALTER_TABLE_DATA_CHANGED;
+    DBUG_RETURN(ALTER_TABLE_DATA_CHANGED);
 
   /*
     Go through keys and check if the original ones are compatible
     with new table.
   */
-  KEY *table_key_info= table->key_info;
-  List_iterator_fast<Key> key_it(*key_list);
-  Key *key= key_it++;
-
-  /* Check if the number of key elements has changed */
-  if  (table->s->keys != key_list->elements)
-    return ALTER_TABLE_INDEX_CHANGED;
+  KEY *table_key;
+  KEY *table_key_end= table->key_info + table->s->keys;
+  KEY *new_key;
+  KEY *new_key_end= key_info_buffer + key_count;
 
-  for (uint i= 0; i < table->s->keys; i++, table_key_info++, key= key_it++)
+  DBUG_PRINT("ingo", ("index count old: %d  new: %d",
+                      table->s->keys, key_count));
+  /*
+    Step through all keys of the old table and search matching new keys.
+  */
+  *index_drop_count= 0;
+  *index_add_count= 0;
+  for (table_key= table->key_info; table_key < table_key_end; table_key++)
   {
-    /*
-      Check that the key types are compatible between old and new tables.
-    */
-    if (table_key_info->algorithm != key->algorithm ||
-	((key->type == Key::PRIMARY || key->type == Key::UNIQUE) &&
-	 !(table_key_info->flags & HA_NOSAME)) ||
-	(!(key->type == Key::PRIMARY || key->type == Key::UNIQUE) &&
-	 (table_key_info->flags & HA_NOSAME)) ||
-	((key->type == Key::SPATIAL) &&
-	 !(table_key_info->flags & HA_SPATIAL)) ||
-	(!(key->type == Key::SPATIAL) &&
-	 (table_key_info->flags & HA_SPATIAL)) ||
-	((key->type == Key::FULLTEXT) &&
-	 !(table_key_info->flags & HA_FULLTEXT)) ||
-	(!(key->type == Key::FULLTEXT) &&
-	 (table_key_info->flags & HA_FULLTEXT)))
-      return ALTER_TABLE_INDEX_CHANGED;
-    
-    if  (table_key_info->key_parts != key->columns.elements)
-      return ALTER_TABLE_INDEX_CHANGED;
+    KEY_PART_INFO *table_part;
+    KEY_PART_INFO *table_part_end= table_key->key_part + table_key->key_parts;
+    KEY_PART_INFO *new_part;
+
+    /* Search a new key with the same name. */
+    for (new_key= key_info_buffer; new_key < new_key_end; new_key++)
+    {
+      if (! strcmp(table_key->name, new_key->name))
+        break;
+    }
+    if (new_key >= new_key_end)
+    {
+      DBUG_PRINT("ingo", ("index dropped: '%s'", table_key->name));
+      /* Key not found. Add the offset of the key to the drop buffer. */
+      index_drop_buffer[(*index_drop_count)++]= table_key - table->key_info;
+      continue;
+    }
+
+    /* Check that the key types are compatible between old and new tables. */
+    if ((table_key->algorithm != new_key->algorithm) ||
+	((table_key->flags & HA_KEYFLAG_MASK) !=
+         (new_key->flags & HA_KEYFLAG_MASK)) ||
+        (table_key->key_parts != new_key->key_parts))
+    {
+      DBUG_PRINT("ingo", ("algorithm %7d %7d",
+                          table_key->algorithm, new_key->algorithm));
+      DBUG_PRINT("ingo", ("flags     %7x %7x",
+                          table_key->flags & HA_KEYFLAG_MASK,
+                          new_key->flags & HA_KEYFLAG_MASK));
+      DBUG_PRINT("ingo", ("parts     %7d %7d",
+                          table_key->key_parts, new_key->key_parts));
+      goto index_changed;
+    }
 
     /*
       Check that the key parts remain compatible between the old and
       new tables.
     */
-    KEY_PART_INFO *table_key_part= table_key_info->key_part;
-    List_iterator_fast<key_part_spec> key_part_it(key->columns);
-    key_part_spec *key_part= key_part_it++;
-    for (uint j= 0; j < table_key_info->key_parts; j++,
-	   table_key_part++, key_part= key_part_it++)
+    for (table_part= table_key->key_part, new_part= new_key->key_part;
+         table_part < table_part_end;
+         table_part++, new_part++)
     {
       /*
 	Key definition has changed if we are using a different field or
-	if the used key length is different
-	(If key_part->length == 0 it means we are using the whole field)
+	if the used key part length is different. We know that the fields
+        did not change. Comparing field numbers is sufficient.
       */
-      if (strcmp(key_part->field_name, table_key_part->field->field_name) ||
-	  (key_part->length && key_part->length != table_key_part->length) ||
-	  (key_part->length == 0 && table_key_part->length !=
-	   table_key_part->field->pack_length()))
-	return ALTER_TABLE_INDEX_CHANGED;	
+      if ((table_part->length != new_part->length) ||
+          (table_part->fieldnr - 1 != new_part->fieldnr))
+      {
+        DBUG_PRINT("ingo", ("length    %7u %7u",
+                            table_part->length, new_part->length));
+        DBUG_PRINT("ingo", ("fieldnr   %7u %7u",
+                            table_part->fieldnr - 1, new_part->fieldnr));
+	goto index_changed;
+      }
     }
+    continue;
+
+  index_changed:
+    DBUG_PRINT("ingo", ("index changed: '%s'", table_key->name));
+    /* Key modified. Add the offset of the key to both buffers. */
+    index_drop_buffer[(*index_drop_count)++]= table_key - table->key_info;
+    index_add_buffer[(*index_add_count)++]= new_key - key_info_buffer;
   }
 
-  return 0; // Tables are compatible
+  /*
+    Step through all keys of the new table and find matching old keys.
+  */
+  for (new_key= key_info_buffer; new_key < new_key_end; new_key++)
+  {
+    /* Search an old key with the same name. */
+    for (table_key= table->key_info; table_key < table_key_end; table_key++)
+    {
+      if (! strcmp(table_key->name, new_key->name))
+        break;
+    }
+    if (table_key >= table_key_end)
+    {
+      DBUG_PRINT("ingo", ("index added: '%s'", new_key->name));
+      /* Key not found. Add the offset of the key to the add buffer. */
+      index_add_buffer[(*index_add_count)++]= new_key - key_info_buffer;
+    }
+  }
+  if (*index_drop_count || *index_add_count)
+    DBUG_RETURN(ALTER_TABLE_INDEX_CHANGED);
+
+  DBUG_RETURN(0); // Tables are compatible
 }
 
 
@@ -3371,6 +3246,10 @@
   uint db_create_options, used_fields;
   enum db_type old_db_type,new_db_type;
   uint need_copy_table= 0;
+  bool need_lock_for_indexes= FALSE;
+  uint key_count;
+  KEY  *key_info_buffer;
+  uint db_options= 0;
 #ifdef HAVE_PARTITION_DB
   bool online_add_empty_partition= FALSE;
   bool online_drop_partition= FALSE;
@@ -4078,6 +3957,10 @@
   List_iterator<Key> key_it(keys);
   List_iterator<create_field> field_it(create_list);
   List<key_part_spec> key_parts;
+  uint *index_drop_buffer;
+  uint index_drop_count;
+  uint *index_add_buffer;
+  uint index_add_count;
 
   KEY *key_info=table->key_info;
   for (uint i=0 ; i < table->s->keys ; i++,key_info++)
@@ -4221,22 +4104,150 @@
 
   set_table_default_charset(thd, create_info, db);
 
+  if (thd->variables.old_alter_table
+      || (table->s->db_type != create_info->db_type)
 #ifdef HAVE_PARTITION_DB
-  if (thd->variables.old_alter_table || partition_changed)
-#else
-  if (thd->variables.old_alter_table)
+      || partition_changed
 #endif
+     )
     need_copy_table= 1;
   else
-    need_copy_table= compare_tables(table, &create_list, &key_list,
-				    create_info, alter_info, order_num);
+  {
+    /*
+      Try to optimize ALTER TABLE. For this purpose we need prepared
+      table structures and translated key descriptions with proper
+      default key name assignment.
+    */
+    if (mysql_prepare_table(thd, create_info, &create_list, &key_list,
+                            (table->s->tmp_table != NO_TMP_TABLE), &db_options,
+                            table->file, &key_info_buffer, &key_count, 0))
+      goto err;
+    /* Allocate result buffers. */
+    if (! (index_drop_buffer=
+           (uint*) thd->alloc(sizeof(uint) * table->s->keys)) ||
+        ! (index_add_buffer=
+           (uint*) thd->alloc(sizeof(uint) * key_list.elements)))
+      goto err;
+    /* Check how much the tables differ. */
+    need_copy_table= compare_tables(table, &create_list,
+                                    key_info_buffer, key_count,
+                                    create_info, alter_info, order_num,
+                                    index_drop_buffer, &index_drop_count,
+                                    index_add_buffer, &index_add_count);
+  }
+
+  /*
+    If there are index changes only, try to do them online. "Index
+    changes only" means also that the handler for the table does not
+    change. The table is open and locked. The handler can be accessed.
+  */
+  if (need_copy_table == ALTER_TABLE_INDEX_CHANGED)
+  {
+    bool  need_recreate= FALSE;
+    bool  pk_dropped= FALSE;
+    ulong alter_flags= table->file->alter_table_flags();
+    KEY   *key;
+    uint  *idx_p;
+    uint  *idx_end_p;
+    DBUG_PRINT("ingo", ("alter_flags: %lu", alter_flags));
+
+    /* Check dropped indexes. */
+    for (idx_p= index_drop_buffer, idx_end_p= idx_p + index_drop_count;
+         idx_p < idx_end_p;
+         idx_p++)
+    {
+      key= table->key_info + *idx_p;
+      DBUG_PRINT("ingo", ("index dropped: '%s'", key->name));
+      if (key->flags & HA_NOSAME)
+      {
+        /* Unique key. Check for "PRIMARY". */
+        if (! my_strcasecmp(system_charset_info,
+                            key->name, primary_key_name))
+        {
+          /* Primary key. */
+          if (! (alter_flags & (HA_ONLINE_DROP_PK_INDEX |
+                                HA_ONLINE_DROP_PK_INDEX_NO_WRITES)))
+            need_recreate= TRUE;
+          else if (! (alter_flags & HA_ONLINE_DROP_PK_INDEX))
+            need_lock_for_indexes= TRUE;
+          pk_dropped= TRUE;
+        }
+        else
+        {
+          /* Non-primary unique key. */
+          if (! (alter_flags & (HA_ONLINE_DROP_UNIQUE_INDEX |
+                                HA_ONLINE_DROP_UNIQUE_INDEX_NO_WRITES)))
+            need_recreate= TRUE;
+          else if (! (alter_flags & HA_ONLINE_DROP_UNIQUE_INDEX))
+            need_lock_for_indexes= TRUE;
+        }
+      }
+      else
+      {
+        /* Non-unique key. */
+        if (! (alter_flags & (HA_ONLINE_DROP_INDEX |
+                              HA_ONLINE_DROP_INDEX_NO_WRITES)))
+          need_recreate= TRUE;
+        else if (! (alter_flags & HA_ONLINE_DROP_INDEX))
+          need_lock_for_indexes= TRUE;
+      }
+    }
+
+    /* Check added indexes. */
+    for (idx_p= index_add_buffer, idx_end_p= idx_p + index_add_count;
+         idx_p < idx_end_p;
+         idx_p++)
+    {
+      key= key_info_buffer + *idx_p;
+      DBUG_PRINT("ingo", ("index added: '%s'", key->name));
+      if (key->flags & HA_NOSAME)
+      {
+        /* Unique key. Check for "PRIMARY". */
+        if (! my_strcasecmp(system_charset_info,
+                            key->name, primary_key_name))
+        {
+          /* Primary key. */
+          if (! (alter_flags & (HA_ONLINE_ADD_PK_INDEX |
+                                HA_ONLINE_ADD_PK_INDEX_NO_WRITES)))
+            need_recreate= TRUE;
+          else if (! (alter_flags & HA_ONLINE_ADD_PK_INDEX))
+            need_lock_for_indexes= TRUE;
+          /* If the primary key is dropped and added, we need re-create. */
+          if (pk_dropped)
+            need_recreate= TRUE;
+        }
+        else
+        {
+          /* Non-primary unique key. */
+          if (! (alter_flags & (HA_ONLINE_ADD_UNIQUE_INDEX |
+                                HA_ONLINE_ADD_UNIQUE_INDEX_NO_WRITES)))
+            need_recreate= TRUE;
+          else if (! (alter_flags & HA_ONLINE_ADD_UNIQUE_INDEX))
+            need_lock_for_indexes= TRUE;
+        }
+      }
+      else
+      {
+        /* Non-unique key. */
+        if (! (alter_flags & (HA_ONLINE_ADD_INDEX |
+                              HA_ONLINE_ADD_INDEX_NO_WRITES)))
+          need_recreate= TRUE;
+        else if (! (alter_flags & HA_ONLINE_ADD_INDEX))
+          need_lock_for_indexes= TRUE;
+      }
+    }
+
+    if (! need_recreate)
+      need_copy_table= 0;
+    DBUG_PRINT("ingo", ("need_recreate: %d  need_lock: %d  need_copy_table: %d",
+                        need_recreate, need_lock_for_indexes, need_copy_table));
+  }
 
   /*
     better have a negative test here, instead of positive, like
     alter_info->flags & ALTER_ADD_COLUMN|ALTER_ADD_INDEX|...
     so that ALTER TABLE won't break when somebody will add new flag
   */
-
   if (!need_copy_table)
     create_info->frm_only= 1;
 
@@ -4262,8 +4273,7 @@
       uint old_lock_type;
       partition_info *part_info= table->s->part_info;
       char path[FN_REFLEN+1];
-      uint db_options= 0, key_count, syntax_len;
-      KEY *key_info_buffer;
+      uint syntax_len;
       char *part_syntax_buf;
 
       VOID(pthread_mutex_lock(&LOCK_open));
@@ -4272,10 +4282,6 @@
         DBUG_RETURN(TRUE);
       }
       VOID(pthread_mutex_unlock(&LOCK_open));
-      mysql_prepare_table(thd, create_info, &create_list,
-                          &key_list, /*tmp_table*/ 0, &db_options,
-                          table->file, &key_info_buffer, &key_count,
-                          /*select_field_count*/ 0);
       if (!(part_syntax_buf= generate_partition_syntax(part_info,
                                                        &syntax_len,
                                                        TRUE,TRUE)))
@@ -4360,7 +4366,6 @@
       Copy data.
       Remove old table and symlinks.
   */
-
   if (!strcmp(db, new_db))		// Ignore symlink if db changed
   {
     if (create_info->index_file_name)
@@ -4383,15 +4388,19 @@
   else
     create_info->data_file_name=create_info->index_file_name=0;
 
-  /* We don't log the statement, it will be logged later. */
-  {
-    tmp_disable_binlog(thd);
-    error= mysql_create_table(thd, new_db, tmp_name,
-                              create_info,create_list,key_list,1,0);
-    reenable_binlog(thd);
-    if (error)
-      DBUG_RETURN(error);
-  }
+  /*
+    Create a table with a temporary name.
+    With create_info->frm_only == 1 this creates a .frm file only.
+    We don't log the statement, it will be logged later.
+  */
+  tmp_disable_binlog(thd);
+  error= mysql_create_table(thd, new_db, tmp_name,
+                            create_info,create_list,key_list,1,0);
+  reenable_binlog(thd);
+  if (error)
+    DBUG_RETURN(error);
+
+  /* Open the table if we need to copy the data. */
   if (need_copy_table)
   {
     if (table->s->tmp_table)
@@ -4418,7 +4427,7 @@
     }
   }
 
-  /* We don't want update TIMESTAMP fields during ALTER TABLE. */
+  /* Copy the data if necessary. */
   thd->count_cuted_fields= CHECK_FIELD_WARN;	// calc cuted fields
   thd->cuted_fields=0L;
   thd->proc_info="copy to tmp table";
@@ -4426,6 +4435,7 @@
   copied=deleted=0;
   if (new_table && !new_table->s->is_view)
   {
+    /* We don't want update TIMESTAMP fields during ALTER TABLE. */
     new_table->timestamp_field_type= TIMESTAMP_NO_AUTO_SET;
     new_table->next_number_field=new_table->found_next_number_field;
     error=copy_data_between_tables(table,new_table,create_list,
@@ -4435,6 +4445,95 @@
   thd->last_insert_id=next_insert_id;		// Needed for correct log
   thd->count_cuted_fields= CHECK_FIELD_IGNORE;
 
+  /* If we did not need to copy, we might still need to add/drop indexes. */
+  if (! new_table)
+  {
+    uint  *key_numbers;
+    uint  count;
+    KEY   *key_info;
+    KEY   *key;
+    uint  *idx_p;
+    uint  *idx_end_p;
+    KEY_PART_INFO *key_part;
+    KEY_PART_INFO *part_end;
+
+    /* Create a new .frm file for crash recovery. */
+    /* Not yet implemented. */
+
+    if (! need_lock_for_indexes)
+    {
+      /* Replicate (binlog) the start of ADD INDEX. */
+      /* Downgrade the write lock. */
+      /* Not yet implemented. */
+    }
+
+    /* The add_index() method takes an array of KEY structs. */
+    key_info= (KEY*) thd->alloc(sizeof(KEY) * index_add_count);
+    key= key_info;
+    for (idx_p= index_add_buffer, idx_end_p= idx_p + index_add_count;
+         idx_p < idx_end_p;
+         idx_p++, key++)
+    {
+      /* Copy the KEY struct. */
+      *key= key_info_buffer[*idx_p];
+      /* Fix the key parts. */
+      part_end= key->key_part + key->key_parts;
+      for (key_part= key->key_part; key_part < part_end; key_part++)
+        key_part->field= table->field[key_part->fieldnr];
+    }
+    /* Add the indexes. */
+    if (table->file->add_index(table, key_info, index_add_count))
+      goto err;
+
+    if (! need_lock_for_indexes)
+    {
+      /* Re-acquire the exclusive lock. */
+      /* Not yet implemented. */
+    }
+
+    /* Create a new .frm file for crash recovery. */
+    /* Not yet implemented. */
+
+    if (! need_lock_for_indexes)
+    {
+      /* Replicate (binlog) the real table change. */
+      /* Not yet implemented. */
+    }
+
+    /* The prepare_drop_index() method takes an array of key numbers. */
+    key_numbers= (uint*) thd->alloc(sizeof(uint) * index_drop_count);
+    count= 0;
+    /* Get the number of each key. */
+    for (idx_p= index_drop_buffer, idx_end_p= idx_p + index_drop_count;
+         idx_p < idx_end_p;
+         idx_p++)
+    {
+      key_numbers[count++]= *idx_p;
+    }
+    /*
+      Tell the handler to preliminarily drop the indexes.
+      This re-numbers the indexes to get rid of gaps.
+    */
+    if (table->file->prepare_drop_index(table, key_numbers, count))
+      goto err;
+
+    if (! need_lock_for_indexes)
+    {
+      /* Downgrade the lock again. */
+      /* Not yet implemented. */
+    }
+
+    /* Tell the handler to finally drop the indexes. */
+    if (table->file->final_drop_index(table))
+      goto err;
+
+    if (! need_lock_for_indexes)
+    {
+      /* Re-acquire the exclusive lock for the final .frm file writing. */
+      /* Not yet implemented. */
+    }
+  }
+
   if (table->s->tmp_table)
   {
     /* We changed a temporary table */
@@ -4444,7 +4543,10 @@
 	The following function call will free the new_table pointer,
 	in close_temporary_table(), so we can safely directly jump to err
       */
-      close_temporary_table(thd,new_db,tmp_name);
+      if (new_table)
+        close_temporary_table(thd,new_db,tmp_name);
+      else
+        VOID(quick_rm_table(new_db_type,new_db,tmp_name));
       goto err;
     }
     /* Close lock if this is a transactional table */
@@ -4458,8 +4560,13 @@
     /* Should pass the 'new_name' as we store table name in the cache */
     if (rename_temporary_table(thd, new_table, new_db, new_name))
     {						// Fatal error
-      close_temporary_table(thd,new_db,tmp_name);
-      my_free((gptr) new_table,MYF(0));
+      if (new_table)
+      {
+        close_temporary_table(thd,new_db,tmp_name);
+        my_free((gptr) new_table,MYF(0));
+      }
+      else
+        VOID(quick_rm_table(new_db_type,new_db,tmp_name));
       goto err;
     }
     write_bin_log(thd, TRUE);

--- 1.28/mysql-test/r/key.result	2005-08-29 21:06:39 +02:00
+++ 1.29/mysql-test/r/key.result	2005-09-26 19:30:24 +02:00
@@ -396,3 +396,66 @@
 b	varchar(20)	NO	MUL		
 c	varchar(20)	NO			
 drop table t1;
+create table t1 (
+c1 int,
+c2 char(12),
+c3 varchar(123),
+c4 timestamp,
+index (c1),
+index i1 (c1),
+index i2 (c2),
+index i3 (c3),
+unique i4 (c4),
+index i5 (c1, c2, c3, c4),
+primary key (c2, c3),
+index (c2, c4));
+show create table t1;
+Table	Create Table
+t1	CREATE TABLE `t1` (
+  `c1` int(11) default NULL,
+  `c2` char(12) NOT NULL default '',
+  `c3` varchar(123) NOT NULL default '',
+  `c4` timestamp NOT NULL default CURRENT_TIMESTAMP on update CURRENT_TIMESTAMP,
+  PRIMARY KEY  (`c2`,`c3`),
+  UNIQUE KEY `i4` (`c4`),
+  KEY `c1` (`c1`),
+  KEY `i1` (`c1`),
+  KEY `i2` (`c2`),
+  KEY `i3` (`c3`),
+  KEY `i5` (`c1`,`c2`,`c3`,`c4`),
+  KEY `c2` (`c2`,`c4`)
+) ENGINE=MyISAM DEFAULT CHARSET=latin1
+alter table t1 drop index c1;
+alter table t1 add index (c1);
+alter table t1 add index (c1);
+alter table t1 drop index i3;
+alter table t1 add index i3 (c3);
+alter table t1 drop index i2, drop index i4;
+alter table t1 add index i2 (c2), add index i4 (c4);
+alter table t1 drop index i2, drop index i4, add index i6 (c2, c4);
+alter table t1 add index i2 (c2), add index i4 (c4), drop index i6;
+alter table t1 drop index i2, drop index i4, add unique i4 (c4);
+alter table t1 add index i2 (c2), drop index i4, add index i4 (c4);
+alter table t1 drop index c2, add index (c2(4),c3(7));
+alter table t1 drop index c2, add index (c2(4),c3(7));
+alter table t1 add primary key (c1, c2), drop primary key;
+alter table t1 drop primary key;
+alter table t1 add primary key (c1, c2), drop primary key;
+ERROR 42000: Can't DROP 'PRIMARY'; check that column/key exists
+show create table t1;
+Table	Create Table
+t1	CREATE TABLE `t1` (
+  `c1` int(11) NOT NULL default '0',
+  `c2` char(12) NOT NULL default '',
+  `c3` varchar(123) NOT NULL default '',
+  `c4` timestamp NOT NULL default CURRENT_TIMESTAMP on update CURRENT_TIMESTAMP,
+  KEY `i1` (`c1`),
+  KEY `i5` (`c1`,`c2`,`c3`,`c4`),
+  KEY `c1` (`c1`),
+  KEY `c1_2` (`c1`),
+  KEY `i3` (`c3`),
+  KEY `i2` (`c2`),
+  KEY `i4` (`c4`),
+  KEY `c2` (`c2`(4),`c3`(7))
+) ENGINE=MyISAM DEFAULT CHARSET=latin1
+drop table t1;

--- 1.24/mysql-test/t/key.test	2005-08-29 21:06:39 +02:00
+++ 1.25/mysql-test/t/key.test	2005-09-26 19:30:24 +02:00
@@ -372,3 +372,53 @@
 drop table t1;
 
 # End of 4.1 tests
+
+#
+# WL#1563 - Modify MySQL to support on-line CREATE/DROP INDEX
+# To test if this really works, you need to run with --debug
+# and check the trace file.
+#
+# Create a table with named and unnamed indexes.
+create table t1 (
+    c1 int,
+    c2 char(12),
+    c3 varchar(123),
+    c4 timestamp,
+    index (c1),
+    index i1 (c1),
+    index i2 (c2),
+    index i3 (c3),
+    unique i4 (c4),
+    index i5 (c1, c2, c3, c4),
+    primary key (c2, c3),
+    index (c2, c4));
+show create table t1;
+# Some simple tests.
+alter table t1 drop index c1;
+alter table t1 add index (c1);
+# This creates index 'c1_2'.
+alter table t1 add index (c1);
+alter table t1 drop index i3;
+alter table t1 add index i3 (c3);
+# Two indexes at the same time.
+alter table t1 drop index i2, drop index i4;
+alter table t1 add index i2 (c2), add index i4 (c4);
+# Three indexes, one of them reversely.
+alter table t1 drop index i2, drop index i4, add index i6 (c2, c4);
+alter table t1 add index i2 (c2), add index i4 (c4), drop index i6;
+# include an unique index.
+alter table t1 drop index i2, drop index i4, add unique i4 (c4);
+alter table t1 add index i2 (c2), drop index i4, add index i4 (c4);
+# Modify an index by changing its definition.
+alter table t1 drop index c2, add index (c2(4),c3(7));
+# Change nothing. The new key definition is the same as the old one.
+alter table t1 drop index c2, add index (c2(4),c3(7));
+# Test primary key handling.
+alter table t1 add primary key (c1, c2), drop primary key;
+alter table t1 drop primary key;
+# Drop is checked first. Primary key must exist.
+--error 1091
+alter table t1 add primary key (c1, c2), drop primary key;
+show create table t1;
+drop table t1;
+
Thread
bk commit into 5.1 tree (ingo:1.1909)ingo26 Sep