bug #31231 mysql_alter_table() tries to drop a non-existing table

bug#31233 mysql_alter_table() fails to drop UNIQUE KEY

mysql-test/suite/ndb/r/ndb_alter_table.result

@@ -354,16 +354,52 @@ select * from t1 where a = 12;
 a	b	c
 12	403	NULL
 drop table t1;
-create table t1 (a int not null, b varchar(10)) engine=ndb;
-show index from t1;
-Table	Non_unique	Key_name	Seq_in_index	Column_name	Collation	Cardinality	Sub_part	Packed	Null	Index_type	Comment
+create table t1(a int not null) engine=ndb;
+$PK Bigunsigned PRIMARY KEY DISTRIBUTION KEY AT=FIXED ST=MEMORY
+PRIMARY KEY($PK) - UniqueHashIndex
+insert into t1 values (1),(2),(3);
 alter table t1 add primary key (a);
-show index from t1;
-Table	Non_unique	Key_name	Seq_in_index	Column_name	Collation	Cardinality	Sub_part	Packed	Null	Index_type	Comment
-t1	0	PRIMARY	1	a	A	0	NULL	NULL		BTREE	
+a Int PRIMARY KEY DISTRIBUTION KEY AT=FIXED ST=MEMORY
+PRIMARY KEY(a) - UniqueHashIndex
+PRIMARY(a) - OrderedIndex
+update t1 set a = 17 where a = 1;
+select * from t1 order by a;
+a
+2
+3
+17
 alter table t1 drop primary key;
-show index from t1;
-Table	Non_unique	Key_name	Seq_in_index	Column_name	Collation	Cardinality	Sub_part	Packed	Null	Index_type	Comment
+$PK Bigunsigned PRIMARY KEY DISTRIBUTION KEY AT=FIXED ST=MEMORY
+PRIMARY KEY($PK) - UniqueHashIndex
+update t1 set a = 1 where a = 17;
+select * from t1 order by a;
+a
+1
+2
+3
+drop table t1;
+create table t1(a int not null) engine=ndb;
+$PK Bigunsigned PRIMARY KEY DISTRIBUTION KEY AT=FIXED ST=MEMORY
+PRIMARY KEY($PK) - UniqueHashIndex
+insert into t1 values (1),(2),(3);
+create unique index pk on t1(a);
+a Int PRIMARY KEY DISTRIBUTION KEY AT=FIXED ST=MEMORY
+PRIMARY KEY(a) - UniqueHashIndex
+update t1 set a = 17 where a = 1;
+select * from t1 order by a;
+a
+2
+3
+17
+alter table t1 drop index pk;
+$PK Bigunsigned PRIMARY KEY DISTRIBUTION KEY AT=FIXED ST=MEMORY
+PRIMARY KEY($PK) - UniqueHashIndex
+update t1 set a = 1 where a = 17;
+select * from t1 order by a;
+a
+1
+2
+3
 drop table t1;
 create table t1 (a int not null primary key, b int not null default 0, c varchar(254)) engine=ndb;
 show create table t1;

mysql-test/suite/ndb/t/ndb_alter_table.test

@@ -411,13 +411,32 @@ select * from t1 where a = 12;
 drop table t1;
 
 # some other ALTER combinations
-# add/drop pk
-create table t1 (a int not null, b varchar(10)) engine=ndb;
-show index from t1;
+# Check add/drop primary key (not supported on-line)
+create table t1(a int not null) engine=ndb;
+--exec $NDB_TOOLS_DIR/ndb_desc --no-defaults -d test t1 | grep PRIMARY
+insert into t1 values (1),(2),(3);
 alter table t1 add primary key (a);
-show index from t1;
+--exec $NDB_TOOLS_DIR/ndb_desc --no-defaults -d test t1 | grep PRIMARY
+update t1 set a = 17 where a = 1;
+select * from t1 order by a;
 alter table t1 drop primary key;
-show index from t1;
+--exec $NDB_TOOLS_DIR/ndb_desc --no-defaults -d test t1 | grep PRIMARY
+update t1 set a = 1 where a = 17;
+select * from t1 order by a;
+drop table t1;
+
+# bug#31233 mysql_alter_table() fails to drop UNIQUE KEY
+create table t1(a int not null) engine=ndb;
+--exec $NDB_TOOLS_DIR/ndb_desc --no-defaults -d test t1 | grep PRIMARY
+insert into t1 values (1),(2),(3);
+create unique index pk on t1(a);
+--exec $NDB_TOOLS_DIR/ndb_desc --no-defaults -d test t1 | grep PRIMARY
+update t1 set a = 17 where a = 1;
+select * from t1 order by a;
+alter table t1 drop index pk;
+--exec $NDB_TOOLS_DIR/ndb_desc --no-defaults -d test t1 | grep PRIMARY
+update t1 set a = 1 where a = 17;
+select * from t1 order by a;
 drop table t1;
 
 # alter .. alter

sql/ha_ndbcluster.cc

@@ -9971,34 +9971,23 @@ bool ha_ndbcluster::check_if_incompatible_data(HA_CREATE_INFO *create_info,
   if (table_changes != IS_EQUAL_YES)
     DBUG_RETURN(COMPATIBLE_DATA_NO);
   
-  /**
-   * Changing from/to primary key
-   *
-   * This is _not_ correct, but check_if_incompatible_data-interface
-   *   doesnt give more info, so I guess that we can't do any
-   *   online add index if not using primary key
-   *
-   *   This as mysql will handle a unique not null index as primary 
-   *     even wo/ user specifiying it... :-(
-   *   
-   */
-  if ((table_share->primary_key == MAX_KEY && pk) ||
-      (table_share->primary_key != MAX_KEY && !pk) ||
-      (table_share->primary_key == MAX_KEY && !pk && ai))
-  {
-    DBUG_RETURN(COMPATIBLE_DATA_NO);
-  }
-  
   /* Check that auto_increment value was not changed */
   if ((create_info->used_fields & HA_CREATE_USED_AUTO) &&
       create_info->auto_increment_value != 0)
+  {
+    DBUG_PRINT("info", ("auto_increment value changed"));
     DBUG_RETURN(COMPATIBLE_DATA_NO);
+  }
   
   /* Check that row format didn't change */
   if ((create_info->used_fields & HA_CREATE_USED_AUTO) &&
       get_row_type() != create_info->row_type)
+  {
+    DBUG_PRINT("info", ("row format changed"));
     DBUG_RETURN(COMPATIBLE_DATA_NO);
+  }
 
+  DBUG_PRINT("info", ("new table seems compatible"));
   DBUG_RETURN(COMPATIBLE_DATA_YES);
 }
 

sql/mysql_priv.h

@@ -2240,6 +2240,7 @@ uint build_table_shadow_filename(char *buff, size_t bufflen,
 #define FN_TO_IS_TMP    (1 << 1)
 #define FN_IS_TMP       (FN_FROM_IS_TMP | FN_TO_IS_TMP)
 #define NO_FRM_RENAME   (1 << 2)
+#define FRM_ONLY        (1 << 3)
 
 /* from hostname.cc */
 struct in_addr;

sql/sql_table.cc

@@ -1835,8 +1835,9 @@ bool quick_rm_table(handlerton *base,const char *db,
   if (my_delete(path,MYF(0)))
     error= 1; /* purecov: inspected */
   path[path_length - reg_ext_length]= '\0'; // Remove reg_ext
-  DBUG_RETURN(ha_delete_table(current_thd, base, path, db, table_name, 0) ||
-              error);
+  if (!(flags & FRM_ONLY))
+    error|= ha_delete_table(current_thd, base, path, db, table_name, 0);
+  DBUG_RETURN(error);
 }
 
 /*
@@ -5163,6 +5164,7 @@ err:
       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.
+      candidate_key_count OUT   The number of candidate keys in original table.
 
   DESCRIPTION
     'table' (first argument) contains information of the original
@@ -5193,7 +5195,8 @@ compare_tables(TABLE *table,
                enum_alter_table_change_level *need_copy_table,
                KEY **key_info_buffer,
                uint **index_drop_buffer, uint *index_drop_count,
-               uint **index_add_buffer, uint *index_add_count)
+               uint **index_add_buffer, uint *index_add_count,
+               uint *candidate_key_count)
 {
   Field **f_ptr, *field;
   uint changes= 0, tmp;
@@ -5208,6 +5211,9 @@ compare_tables(TABLE *table,
     create_info->varchar will be reset in mysql_prepare_create_table.
   */
   bool varchar= create_info->varchar;
+  bool not_nullable= true;
+  DBUG_ENTER("compare_tables");
+
   /*
     Create a copy of alter_info.
     To compare the new and old table definitions, we need to "prepare"
@@ -5225,24 +5231,21 @@ compare_tables(TABLE *table,
   */
   Alter_info tmp_alter_info(*alter_info, thd->mem_root);
   uint db_options= 0; /* not used */
-
-  DBUG_ENTER("compare_tables");
-
   /* Create the prepared information. */
   if (mysql_prepare_create_table(thd, create_info,
-                                  &tmp_alter_info,
-                                  (table->s->tmp_table != NO_TMP_TABLE),
-                                  &db_options,
-                                  table->file, key_info_buffer,
-                                  &key_count, 0))
+                                 &tmp_alter_info,
+                                 (table->s->tmp_table != NO_TMP_TABLE),
+                                 &db_options,
+                                 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)) ||
+         (uint*) thd->alloc(sizeof(uint) * table->s->keys)) ||
       ! (*index_add_buffer=
-          (uint*) thd->alloc(sizeof(uint) * tmp_alter_info.key_list.elements)))
+         (uint*) thd->alloc(sizeof(uint) * tmp_alter_info.key_list.elements)))
     DBUG_RETURN(1);
-
+  
   /*
     Some very basic checks. If number of fields changes, or the
     handler, we need to run full ALTER TABLE. In the future
@@ -5355,12 +5358,29 @@ compare_tables(TABLE *table,
   */
   *index_drop_count= 0;
   *index_add_count= 0;
+  *candidate_key_count= 0;
   for (table_key= table->key_info; table_key < table_key_end; table_key++)
   {
     KEY_PART_INFO *table_part;
     KEY_PART_INFO *table_part_end= table_key->key_part + table_key->key_parts;
     KEY_PART_INFO *new_part;
 
+   /*
+      Check if key is a candidate key, i.e. a unique index with no index
+      fields nullable, then key is either already primary key or could
+      be promoted to primary key if the original primary key is dropped.
+      Count all candidate keys.
+    */
+    not_nullable= true;
+    for (table_part= table_key->key_part;
+         table_part < table_part_end;
+         table_part++)
+    {
+      not_nullable= not_nullable && (! table_part->field->maybe_null());
+    }
+    if ((table_key->flags & HA_NOSAME) && not_nullable)
+      (*candidate_key_count)++;
+
     /* Search a new key with the same name. */
     for (new_key= *key_info_buffer; new_key < new_key_end; new_key++)
     {
@@ -5986,13 +6006,16 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
   uint *index_drop_buffer;
   uint index_add_count;
   uint *index_add_buffer;
+  uint candidate_key_count;
   bool committed= 0;
+  bool no_pk;
   DBUG_ENTER("mysql_alter_table");
 
   LINT_INIT(index_add_count);
   LINT_INIT(index_drop_count);
   LINT_INIT(index_add_buffer);
   LINT_INIT(index_drop_buffer);
+  LINT_INIT(candidate_key_count);
 
   /*
     Check if we attempt to alter mysql.slow_log or
@@ -6403,7 +6426,8 @@ view_err:
                        &need_copy_table_res,
                        &key_info_buffer,
                        &index_drop_buffer, &index_drop_count,
-                       &index_add_buffer, &index_add_count))
+                       &index_add_buffer, &index_add_count,
+                       &candidate_key_count))
       goto err;
    
     if (need_copy_table == ALTER_TABLE_METADATA_ONLY)
@@ -6437,20 +6461,40 @@ view_err:
       DBUG_PRINT("info", ("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))
+        /* 
+           Unique key. Check for "PRIMARY". 
+           or if dropping last unique key
+        */
+        if ((uint) (key - table->key_info) == table->s->primary_key)
         {
+          DBUG_PRINT("info", ("Dropping primary key"));
           /* Primary key. */
           needed_online_flags|=  HA_ONLINE_DROP_PK_INDEX;
           needed_fast_flags|= HA_ONLINE_DROP_PK_INDEX_NO_WRITES;
           pk_changed++;
+          candidate_key_count--;
         }
         else
         {
+          KEY_PART_INFO *part_end= key->key_part + key->key_parts;
+          bool is_candidate_key= true;
+
           /* Non-primary unique key. */
           needed_online_flags|=  HA_ONLINE_DROP_UNIQUE_INDEX;
           needed_fast_flags|= HA_ONLINE_DROP_UNIQUE_INDEX_NO_WRITES;
+
+          /*
+            Check if all fields in key are declared
+            NOT NULL and adjust candidate_key_count
+          */
+          for (KEY_PART_INFO *key_part= key->key_part;
+               key_part < part_end;
+               key_part++)
+            is_candidate_key=
+              (is_candidate_key && 
+               (! table->field[key_part->fieldnr-1]->maybe_null()));
+          if (is_candidate_key)
+            candidate_key_count--;
         }
       }
       else
@@ -6460,7 +6504,8 @@ view_err:
         needed_fast_flags|= HA_ONLINE_DROP_INDEX_NO_WRITES;
       }
     }
-
+    no_pk= ((table->s->primary_key == MAX_KEY) ||
+            (needed_online_flags & HA_ONLINE_DROP_PK_INDEX));
     /* Check added indexes. */
     for (idx_p= index_add_buffer, idx_end_p= idx_p + index_add_count;
          idx_p < idx_end_p;
@@ -6470,14 +6515,38 @@ view_err:
       DBUG_PRINT("info", ("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))
+        /* Unique key */
+
+        KEY_PART_INFO *part_end= key->key_part + key->key_parts;    
+        bool is_candidate_key= true;
+
+        /*
+          Check if all fields in key are declared
+          NOT NULL
+         */
+        for (KEY_PART_INFO *key_part= key->key_part;
+             key_part < part_end;
+             key_part++)
+          is_candidate_key=
+            (is_candidate_key && 
+             (! table->field[key_part->fieldnr]->maybe_null()));
+
+        /*
+           Check for "PRIMARY"
+           or if adding first unique key
+           defined on non-nullable fields
+        */
+
+        if ((!my_strcasecmp(system_charset_info,
+                            key->name, primary_key_name)) ||
+            (no_pk && candidate_key_count == 0 && is_candidate_key))
         {
+          DBUG_PRINT("info", ("Adding primary key"));
           /* Primary key. */
           needed_online_flags|=  HA_ONLINE_ADD_PK_INDEX;
           needed_fast_flags|= HA_ONLINE_ADD_PK_INDEX_NO_WRITES;
           pk_changed++;
+          no_pk= false;
         }
         else
         {
@@ -6494,6 +6563,20 @@ view_err:
       }
     }
 
+    if ((candidate_key_count > 0) && 
+        (needed_online_flags & HA_ONLINE_DROP_PK_INDEX))
+    {
+      /*
+        Dropped primary key when there is some other unique 
+        not null key that should be converted to primary key
+      */
+      needed_online_flags|=  HA_ONLINE_ADD_PK_INDEX;
+      needed_fast_flags|= HA_ONLINE_ADD_PK_INDEX_NO_WRITES;
+      pk_changed= 2;
+    }
+
+    DBUG_PRINT("info", ("needed_online_flags: 0x%lx, needed_fast_flags: 0x%lx",
+                        needed_online_flags, needed_fast_flags));
     /*
       Online or fast add/drop index is possible only if
       the primary key is not added and dropped in the same statement.
@@ -6992,7 +7075,10 @@ err1:
     close_temporary_table(thd, new_table, 1, 1);
   }
   else
-    VOID(quick_rm_table(new_db_type, new_db, tmp_name, FN_IS_TMP));
+    VOID(quick_rm_table(new_db_type, new_db, tmp_name, 
+                        create_info->frm_only
+                        ? FN_IS_TMP | FRM_ONLY
+                        : FN_IS_TMP));
 
 err:
   /*