Merge ahristov@bk-internal.mysql.com:/home/bk/mysql-5.1-maint

into  example.com:/work/bug24395-v2/my51

mysql-test/r/alter_table.result

@@ -541,6 +541,127 @@ create table t1 ( a timestamp );
 alter table t1 add unique ( a(1) );
 ERROR HY000: Incorrect sub part key; the used key part isn't a string, the used length is longer than the key part, or the storage engine doesn't support unique sub keys
 drop table t1;
+drop table if exists t1;
+create table t1 (a int, key(a));
+show indexes from t1;
+Table	Non_unique	Key_name	Seq_in_index	Column_name	Collation	Cardinality	Sub_part	Packed	Null	Index_type	Comment
+t1	1	a	1	a	A	NULL	NULL	NULL	YES	BTREE	
+"this used not to disable the index"
+alter table t1 modify a int, disable keys;
+show indexes from t1;
+Table	Non_unique	Key_name	Seq_in_index	Column_name	Collation	Cardinality	Sub_part	Packed	Null	Index_type	Comment
+t1	1	a	1	a	A	NULL	NULL	NULL	YES	BTREE	disabled
+alter table t1 enable keys;
+show indexes from t1;
+Table	Non_unique	Key_name	Seq_in_index	Column_name	Collation	Cardinality	Sub_part	Packed	Null	Index_type	Comment
+t1	1	a	1	a	A	NULL	NULL	NULL	YES	BTREE	
+alter table t1 modify a bigint, disable keys;
+show indexes from t1;
+Table	Non_unique	Key_name	Seq_in_index	Column_name	Collation	Cardinality	Sub_part	Packed	Null	Index_type	Comment
+t1	1	a	1	a	A	NULL	NULL	NULL	YES	BTREE	disabled
+alter table t1 enable keys;
+show indexes from t1;
+Table	Non_unique	Key_name	Seq_in_index	Column_name	Collation	Cardinality	Sub_part	Packed	Null	Index_type	Comment
+t1	1	a	1	a	A	NULL	NULL	NULL	YES	BTREE	
+alter table t1 add b char(10), disable keys;
+show indexes from t1;
+Table	Non_unique	Key_name	Seq_in_index	Column_name	Collation	Cardinality	Sub_part	Packed	Null	Index_type	Comment
+t1	1	a	1	a	A	NULL	NULL	NULL	YES	BTREE	disabled
+alter table t1 add c decimal(10,2), enable keys;
+show indexes from t1;
+Table	Non_unique	Key_name	Seq_in_index	Column_name	Collation	Cardinality	Sub_part	Packed	Null	Index_type	Comment
+t1	1	a	1	a	A	NULL	NULL	NULL	YES	BTREE	
+"this however did"
+alter table t1 disable keys;
+show indexes from t1;
+Table	Non_unique	Key_name	Seq_in_index	Column_name	Collation	Cardinality	Sub_part	Packed	Null	Index_type	Comment
+t1	1	a	1	a	A	NULL	NULL	NULL	YES	BTREE	disabled
+desc t1;
+Field	Type	Null	Key	Default	Extra
+a	bigint(20)	YES	MUL	NULL	
+b	char(10)	YES		NULL	
+c	decimal(10,2)	YES		NULL	
+alter table t1 add d decimal(15,5);
+"The key should still be disabled"
+show indexes from t1;
+Table	Non_unique	Key_name	Seq_in_index	Column_name	Collation	Cardinality	Sub_part	Packed	Null	Index_type	Comment
+t1	1	a	1	a	A	NULL	NULL	NULL	YES	BTREE	disabled
+drop table t1;
+"Now will test with one unique index"
+create table t1(a int, b char(10), unique(a));
+show indexes from t1;
+Table	Non_unique	Key_name	Seq_in_index	Column_name	Collation	Cardinality	Sub_part	Packed	Null	Index_type	Comment
+t1	0	a	1	a	A	NULL	NULL	NULL	YES	BTREE	
+alter table t1 disable keys;
+show indexes from t1;
+Table	Non_unique	Key_name	Seq_in_index	Column_name	Collation	Cardinality	Sub_part	Packed	Null	Index_type	Comment
+t1	0	a	1	a	A	NULL	NULL	NULL	YES	BTREE	
+alter table t1 enable keys;
+"If no copy on noop change, this won't touch the data file"
+"Unique index, no change"
+alter table t1 modify a int, disable keys;
+show indexes from t1;
+Table	Non_unique	Key_name	Seq_in_index	Column_name	Collation	Cardinality	Sub_part	Packed	Null	Index_type	Comment
+t1	0	a	1	a	A	NULL	NULL	NULL	YES	BTREE	
+"Change the type implying data copy"
+"Unique index, no change"
+alter table t1 modify a bigint, disable keys;
+show indexes from t1;
+Table	Non_unique	Key_name	Seq_in_index	Column_name	Collation	Cardinality	Sub_part	Packed	Null	Index_type	Comment
+t1	0	a	1	a	A	NULL	NULL	NULL	YES	BTREE	
+alter table t1 modify a bigint;
+show indexes from t1;
+Table	Non_unique	Key_name	Seq_in_index	Column_name	Collation	Cardinality	Sub_part	Packed	Null	Index_type	Comment
+t1	0	a	1	a	A	NULL	NULL	NULL	YES	BTREE	
+alter table t1 modify a int;
+show indexes from t1;
+Table	Non_unique	Key_name	Seq_in_index	Column_name	Collation	Cardinality	Sub_part	Packed	Null	Index_type	Comment
+t1	0	a	1	a	A	NULL	NULL	NULL	YES	BTREE	
+drop table t1;
+"Now will test with one unique and one non-unique index"
+create table t1(a int, b char(10), unique(a), key(b));
+show indexes from t1;
+Table	Non_unique	Key_name	Seq_in_index	Column_name	Collation	Cardinality	Sub_part	Packed	Null	Index_type	Comment
+t1	0	a	1	a	A	NULL	NULL	NULL	YES	BTREE	
+t1	1	b	1	b	A	NULL	NULL	NULL	YES	BTREE	
+alter table t1 disable keys;
+show indexes from t1;
+Table	Non_unique	Key_name	Seq_in_index	Column_name	Collation	Cardinality	Sub_part	Packed	Null	Index_type	Comment
+t1	0	a	1	a	A	NULL	NULL	NULL	YES	BTREE	
+t1	1	b	1	b	A	NULL	NULL	NULL	YES	BTREE	disabled
+alter table t1 enable keys;
+"If no copy on noop change, this won't touch the data file"
+"The non-unique index will be disabled"
+alter table t1 modify a int, disable keys;
+show indexes from t1;
+Table	Non_unique	Key_name	Seq_in_index	Column_name	Collation	Cardinality	Sub_part	Packed	Null	Index_type	Comment
+t1	0	a	1	a	A	NULL	NULL	NULL	YES	BTREE	
+t1	1	b	1	b	A	NULL	NULL	NULL	YES	BTREE	disabled
+alter table t1 enable keys;
+show indexes from t1;
+Table	Non_unique	Key_name	Seq_in_index	Column_name	Collation	Cardinality	Sub_part	Packed	Null	Index_type	Comment
+t1	0	a	1	a	A	NULL	NULL	NULL	YES	BTREE	
+t1	1	b	1	b	A	NULL	NULL	NULL	YES	BTREE	
+"Change the type implying data copy"
+"The non-unique index will be disabled"
+alter table t1 modify a bigint, disable keys;
+show indexes from t1;
+Table	Non_unique	Key_name	Seq_in_index	Column_name	Collation	Cardinality	Sub_part	Packed	Null	Index_type	Comment
+t1	0	a	1	a	A	NULL	NULL	NULL	YES	BTREE	
+t1	1	b	1	b	A	NULL	NULL	NULL	YES	BTREE	disabled
+"Change again the type, but leave the indexes as_is"
+alter table t1 modify a int;
+show indexes from t1;
+Table	Non_unique	Key_name	Seq_in_index	Column_name	Collation	Cardinality	Sub_part	Packed	Null	Index_type	Comment
+t1	0	a	1	a	A	NULL	NULL	NULL	YES	BTREE	
+t1	1	b	1	b	A	NULL	NULL	NULL	YES	BTREE	disabled
+"Try the same. When data is no copied on similar tables, this is noop"
+alter table t1 modify a int;
+show indexes from t1;
+Table	Non_unique	Key_name	Seq_in_index	Column_name	Collation	Cardinality	Sub_part	Packed	Null	Index_type	Comment
+t1	0	a	1	a	A	NULL	NULL	NULL	YES	BTREE	
+t1	1	b	1	b	A	NULL	NULL	NULL	YES	BTREE	disabled
+drop table t1;
 create database mysqltest;
 create table t1 (c1 int);
 alter table t1 rename mysqltest.t1;

mysql-test/t/alter_table.test

@@ -371,6 +371,103 @@ create table t1 ( a timestamp );
 alter table t1 add unique ( a(1) );
 drop table t1;
 
+#
+# Bug #24395: ALTER TABLE DISABLE KEYS doesn't work when modifying the table
+#
+# This problem happens if the data change is compatible.
+# Changing to the same type is compatible for example.
+#
+--disable_warnings
+drop table if exists t1;
+--enable_warnings
+create table t1 (a int, key(a));
+show indexes from t1;
+--echo "this used not to disable the index"
+alter table t1 modify a int, disable keys;
+show indexes from t1;
+
+alter table t1 enable keys;
+show indexes from t1;
+
+alter table t1 modify a bigint, disable keys;
+show indexes from t1;
+
+alter table t1 enable keys;
+show indexes from t1;
+
+alter table t1 add b char(10), disable keys;
+show indexes from t1;
+
+alter table t1 add c decimal(10,2), enable keys;
+show indexes from t1;
+
+--echo "this however did"
+alter table t1 disable keys;
+show indexes from t1;
+
+desc t1;
+
+alter table t1 add d decimal(15,5);
+--echo "The key should still be disabled"
+show indexes from t1;
+
+drop table t1;
+
+--echo "Now will test with one unique index"
+create table t1(a int, b char(10), unique(a));
+show indexes from t1;
+alter table t1 disable keys;
+show indexes from t1;
+alter table t1 enable keys;
+
+--echo "If no copy on noop change, this won't touch the data file"
+--echo "Unique index, no change"
+alter table t1 modify a int, disable keys;
+show indexes from t1;
+
+--echo "Change the type implying data copy"
+--echo "Unique index, no change"
+alter table t1 modify a bigint, disable keys;
+show indexes from t1;
+
+alter table t1 modify a bigint;
+show indexes from t1;
+
+alter table t1 modify a int;
+show indexes from t1;
+
+drop table t1;
+
+--echo "Now will test with one unique and one non-unique index"
+create table t1(a int, b char(10), unique(a), key(b));
+show indexes from t1;
+alter table t1 disable keys;
+show indexes from t1;
+alter table t1 enable keys;
+
+
+--echo "If no copy on noop change, this won't touch the data file"
+--echo "The non-unique index will be disabled"
+alter table t1 modify a int, disable keys;
+show indexes from t1;
+alter table t1 enable keys;
+show indexes from t1;
+
+--echo "Change the type implying data copy"
+--echo "The non-unique index will be disabled"
+alter table t1 modify a bigint, disable keys;
+show indexes from t1;
+
+--echo "Change again the type, but leave the indexes as_is"
+alter table t1 modify a int;
+show indexes from t1;
+--echo "Try the same. When data is no copied on similar tables, this is noop"
+alter table t1 modify a int;
+show indexes from t1;
+
+drop table t1;
+
+
 #
 # Bug#11493 - Alter table rename to default database does not work without
 #             db name qualifying

sql/sql_table.cc

@@ -37,7 +37,9 @@ static char *make_unique_key_name(const char *field_name,KEY *start,KEY *end);
 static int copy_data_between_tables(TABLE *from,TABLE *to,
                                     List<create_field> &create, bool ignore,
 				    uint order_num, ORDER *order,
-				    ha_rows *copied,ha_rows *deleted);
+				    ha_rows *copied,ha_rows *deleted,
+                                    enum enum_enable_or_disable keys_onoff);
+
 static bool prepare_blob_field(THD *thd, create_field *sql_field);
 static bool check_engine(THD *thd, const char *table_name,
                          HA_CREATE_INFO *create_info);                             
@@ -5194,6 +5196,54 @@ static uint compare_tables(TABLE *table, List<create_field> *create_list,
 }
 
 
+/*
+  Manages enabling/disabling of indexes for ALTER TABLE
+
+  SYNOPSIS
+    alter_table_manage_keys()
+      table                  Target table
+      indexes_were_disabled  Whether the indexes of the from table
+                             were disabled
+      keys_onoff             ENABLE | DISABLE | LEAVE_AS_IS
+
+  RETURN VALUES
+    FALSE  OK
+    TRUE   Error
+*/
+
+static
+bool alter_table_manage_keys(TABLE *table, int indexes_were_disabled,
+                             enum enum_enable_or_disable keys_onoff)
+{
+  int error= 0;
+  DBUG_ENTER("alter_table_manage_keys");
+  DBUG_PRINT("enter", ("table=%p were_disabled=%d on_off=%d",
+             table, indexes_were_disabled, keys_onoff));
+
+  switch (keys_onoff) {
+  case ENABLE:
+    error= table->file->enable_indexes(HA_KEY_SWITCH_NONUNIQ_SAVE);
+    break;
+  case LEAVE_AS_IS:
+    if (!indexes_were_disabled)
+      break;
+    /* fall-through: disabled indexes */
+  case DISABLE:
+    error= table->file->disable_indexes(HA_KEY_SWITCH_NONUNIQ_SAVE);
+  }
+
+  if (error == HA_ERR_WRONG_COMMAND)
+  {
+    push_warning_printf(current_thd, MYSQL_ERROR::WARN_LEVEL_NOTE,
+                        ER_ILLEGAL_HA, ER(ER_ILLEGAL_HA), table->s->table_name);
+    error= 0;
+  } else if (error)
+    table->file->print_error(error, MYF(0));
+
+  DBUG_RETURN(error);
+}
+
+
 /*
   Alter table
 */
@@ -6116,7 +6166,18 @@ view_err:
     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, ignore,
-				   order_num, order, &copied, &deleted);
+                                   order_num, order, &copied, &deleted,
+                                   alter_info->keys_onoff);
+  }
+  else
+  {
+    VOID(pthread_mutex_lock(&LOCK_open));
+    wait_while_table_is_used(thd, table, HA_EXTRA_FORCE_REOPEN);
+    table->file->ha_external_lock(thd, F_WRLCK);
+    alter_table_manage_keys(table, table->file->indexes_are_disabled(),
+                            alter_info->keys_onoff);
+    table->file->ha_external_lock(thd, F_UNLCK);
+    VOID(pthread_mutex_unlock(&LOCK_open));
   }
   thd->count_cuted_fields= CHECK_FIELD_IGNORE;
 
@@ -6533,7 +6594,8 @@ copy_data_between_tables(TABLE *from,TABLE *to,
                          bool ignore,
 			 uint order_num, ORDER *order,
 			 ha_rows *copied,
-			 ha_rows *deleted)
+			 ha_rows *deleted,
+                         enum enum_enable_or_disable keys_onoff)
 {
   int error;
   Copy_field *copy,*copy_end;
@@ -6567,6 +6629,9 @@ copy_data_between_tables(TABLE *from,TABLE *to,
   if (to->file->ha_external_lock(thd, F_WRLCK))
     DBUG_RETURN(-1);
 
+  /* We need external lock before we can disable/enable keys */
+  alter_table_manage_keys(to, from->file->indexes_are_disabled(), keys_onoff);
+
   /* We can abort alter table for any table type */
   thd->no_trans_update= 0;
   thd->abort_on_warning= !ignore && test(thd->variables.sql_mode &

storage/myisam/mi_open.c

@@ -1282,13 +1282,30 @@ int mi_enable_indexes(MI_INFO *info)
   RETURN
     0  indexes are not disabled
     1  all indexes are disabled
-   [2  non-unique indexes are disabled - NOT YET IMPLEMENTED]
+    2  non-unique indexes are disabled
 */
 
 int mi_indexes_are_disabled(MI_INFO *info)
 {
   MYISAM_SHARE *share= info->s;
 
-  return (! mi_is_any_key_active(share->state.key_map) && share->base.keys);
+  /*
+    No keys or all are enabled. keys is the number of keys. Left shifted
+    gives us only one bit set. When decreased by one, gives us all all bits
+    up to this one set and it gets unset.
+  */
+  if (!share->base.keys ||
+      (mi_is_all_keys_active(share->state.key_map, share->base.keys)))
+    return 0;
+
+  /* All are disabled */
+  if (mi_is_any_key_active(share->state.key_map))
+    return 1;
+
+  /*
+    We have keys. Some enabled, some disabled.
+    Don't check for any non-unique disabled but return directly 2
+  */
+  return 2;
 }