List:Commits« Previous MessageNext Message »
From:Martin Skold Date:March 12 2008 2:22pm
Subject:bk commit into 5.1 tree (mskold:1.2530) BUG#31233
View as plain text  
Below is the list of changes that have just been committed into a local
5.1 repository of mskold.  When mskold 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@stripped, 2008-03-12 15:22:52+01:00, mskold@stripped +5 -0
  In bug#31233 mysql_alter_table() fails to drop UNIQUE KEY
  it was descovered that the online add/drop index implementation
  does not handle unique indexes that are promoted to primary keys.
  In MySQL, if a table has no primary key the first unique index
  defined over non-nullable columns is promoted to become the primary key.
  Two special cases must be handled:
  
  1. If a table has no primary key and a unique index defined over non-nullable
  columns is added, only storage engines that support adding primary keys online
  should be called.
  2. If a table has no explicit primary key, but a unique index defined over non-nullable
  columns (promoted as implicit primary key) and this index is dropped, only storage engines
  that support dropping primary keys should be called.
  
  To detect these cases the function compare_tables was modified to count
  all candidate_keys (keys that are possible to promote to primary keys) of a table
  to detect:
   1. candidate_key_count == 0, and
   2. candidate_key_count == 1

  mysql-test/suite/ndb/r/ndb_alter_table.result@stripped, 2008-03-12 15:22:51+01:00, mskold@stripped +44 -8
    bug#31233 mysql_alter_table() fails to drop UNIQUE KEY: added test cases

  mysql-test/suite/ndb/t/ndb_alter_table.test@stripped, 2008-03-12 15:22:51+01:00, mskold@stripped +24 -5
    bug#31233 mysql_alter_table() fails to drop UNIQUE KEY: added test cases

  sql/ha_ndbcluster.cc@stripped, 2008-03-12 15:22:51+01:00, mskold@stripped +8 -19
    bug#31233 mysql_alter_table() fails to drop UNIQUE KEY: Removed check for non-pk tables, not needed when mysql_alter_table checks apropriate flags

  sql/mysql_priv.h@stripped, 2008-03-12 15:22:51+01:00, mskold@stripped +1 -0
    bug #31231  mysql_alter_table() tries to drop a non-existing table: added FRM_ONLY flag

  sql/sql_table.cc@stripped, 2008-03-12 15:22:51+01:00, mskold@stripped +99 -12
    bug #31231  mysql_alter_table() tries to drop a non-existing table
    Don't invoke handler for tables defined with FRM_ONLY flag.
    
    bug#31233 mysql_alter_table() fails to drop UNIQUE KEY
    When a table is defined without an explicit primary key
    mysql will choose the first found unique index defined over
    non-nullable fields (if such an index exists). This means
    that if such an index is added (the first) or dropped (the last)
    through an alter table, this equals adding or dropping a primary key.
    The implementation for on-line add/drop index did not consider
    this semantics. This patch ensures that only handlers with the
    correctly defined flags (see handler.h for explanation of the flags):
    HA_ONLINE_ADD_PK_INDEX
    HA_ONLINE_ADD_PK_INDEX_NO_WRITES
    HA_ONLINE_DROP_PK_INDEX
    HA_ONLINE_DROP_PK_INDEX_NO_WRITES
    are invoked for such on-line operations. All others handlers must
    perform a full (offline) alter table.

diff -Nrup a/mysql-test/suite/ndb/r/ndb_alter_table.result b/mysql-test/suite/ndb/r/ndb_alter_table.result
--- a/mysql-test/suite/ndb/r/ndb_alter_table.result	2007-08-03 00:14:02 +02:00
+++ b/mysql-test/suite/ndb/r/ndb_alter_table.result	2008-03-12 15:22:51 +01:00
@@ -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;
diff -Nrup a/mysql-test/suite/ndb/t/ndb_alter_table.test b/mysql-test/suite/ndb/t/ndb_alter_table.test
--- a/mysql-test/suite/ndb/t/ndb_alter_table.test	2007-07-04 22:06:22 +02:00
+++ b/mysql-test/suite/ndb/t/ndb_alter_table.test	2008-03-12 15:22:51 +01:00
@@ -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
diff -Nrup a/sql/ha_ndbcluster.cc b/sql/ha_ndbcluster.cc
--- a/sql/ha_ndbcluster.cc	2008-02-11 17:38:31 +01:00
+++ b/sql/ha_ndbcluster.cc	2008-03-12 15:22:51 +01:00
@@ -9970,35 +9970,24 @@ bool ha_ndbcluster::check_if_incompatibl
 
   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);
 }
 
diff -Nrup a/sql/mysql_priv.h b/sql/mysql_priv.h
--- a/sql/mysql_priv.h	2008-02-08 16:55:02 +01:00
+++ b/sql/mysql_priv.h	2008-03-12 15:22:51 +01:00
@@ -2168,6 +2168,7 @@ uint build_table_shadow_filename(char *b
 #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;
diff -Nrup a/sql/sql_table.cc b/sql/sql_table.cc
--- a/sql/sql_table.cc	2008-02-01 11:50:18 +01:00
+++ b/sql/sql_table.cc	2008-03-12 15:22:51 +01:00
@@ -1831,8 +1831,9 @@ bool quick_rm_table(handlerton *base,con
   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);
 }
 
 /*
@@ -5038,6 +5039,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
@@ -5068,7 +5070,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;
@@ -5082,6 +5085,7 @@ 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");
 
   {
@@ -5220,12 +5224,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++)
     {
@@ -5851,13 +5872,16 @@ bool mysql_alter_table(THD *thd,char *ne
   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
@@ -6268,7 +6292,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)
@@ -6302,20 +6327,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]->maybe_null()));
+          if (is_candidate_key)
+            candidate_key_count--;
         }
       }
       else
@@ -6325,7 +6370,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;
@@ -6335,14 +6381,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
         {
@@ -6359,6 +6429,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.
@@ -6857,7 +6941,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:
   /*
Thread
bk commit into 5.1 tree (mskold:1.2530) BUG#31233Martin Skold12 Mar