Bug #31233 mysql_alter_table() fails to drop UNIQUE KEY
Submitted: 27 Sep 2007 12:29 Modified: 21 Oct 2008 17:44
Reporter: Marko Mäkelä Email Updates:
Status: Closed Impact on me:
None 
Category:MySQL Server: DDL Severity:S3 (Non-critical)
Version:5.1 OS:Any
Assigned to: Martin Skold CPU Architecture:Any
Triage: Triaged: D2 (Serious)

[27 Sep 2007 12:29] Marko Mäkelä
Description:
The ha_innobase::prepare_drop_index() method in our modified InnoDB will reject the attempt to drop an implicit clustered index (the first UNIQUE NOT NULL index of a table that does not contain a PRIMARY KEY).

This may be a bug in MySQL; it should rebuild the table and indexes in this case, because InnoDB does not advertise HA_ONLINE_DROP_PK_INDEX capability.

How to repeat:
create table t1(a int not null, b int, c char(10) not null) engine = innodb;
alter table t1 add unique index (c);
--error ER_REQUIRES_PRIMARY_KEY
drop index c on t1;

The error is returned by ha_innobase::prepare_drop_index() in our private tree.

Suggested fix:
Instead of calling handler::prepare_drop_index(), MySQL should rebuild the table in the old-fashioned way when the storage engine does not advertise HA_ONLINE_DROP_PK_INDEX capability and the a request to drop the clustered index (PRIMARY KEY, or UNIQUE NOT NULL index in the absence of a PRIMARY KEY) is made.
[27 Sep 2007 13:51] Godofredo Miguel Solorzano
Thank you for the bug report. Looks like it is only is repeatable in
your private tree, so could you please verify this bug yourself?. Thanks
in advance.

[miguel@skybr 5.1]$ bin/mysql -uroot test
Welcome to the MySQL monitor.  Commands end with ; or \g.
Your MySQL connection id is 1
Server version: 5.1.23-beta-debug Source distribution

Type 'help;' or '\h' for help. Type '\c' to clear the buffer.

mysql> create table t1(a int not null, b int, c char(10) not null) engine = innodb;
Query OK, 0 rows affected (0.05 sec)

mysql> alter table t1 add unique index (c);
Query OK, 0 rows affected (0.02 sec)
Records: 0  Duplicates: 0  Warnings: 0

mysql> --error ER_REQUIRES_PRIMARY_KEY
mysql> drop index c on t1;
Query OK, 0 rows affected (0.00 sec)
Records: 0  Duplicates: 0  Warnings: 0
[10 Oct 2007 13:28] Marko Mäkelä
Modifications to InnoDB files for repeating the bug

Attachment: bug31233-repeat.patch (text/x-diff), 3.16 KiB.

[26 Nov 2007 17:23] Heikki Tuuri
Marko, please tell Martin Skold precisely what is wrong.
[27 Nov 2007 8:27] Marko Mäkelä
This bug report should be self-contained. The bug can be reproduced by applying the attached patch to the InnoDB source tree.
[10 Jan 2008 15:08] Martin Skold
Proposed patch for sql_table.cc

Attachment: bug31233-fix.patch (text/x-diff), 4.15 KiB.

[10 Jan 2008 15:10] Martin Skold
Attached proposed patch, please test if it solves problem.
[10 Jan 2008 16:47] Ingo Strüwing
In the patch you have:
+        /*
+           Unique key. Check for "PRIMARY"
+           or if dropping last unique key.
+        */
+        if ((! my_strcasecmp(system_charset_info,
+                             key->name, primary_key_name)) ||
+            (unique_index_count == 1))

Won't it be better to have (!--unique_index_count) here? If the original table has multiple unique indexes and all are dropped in one ALTER, won't the same problem happen?
[10 Jan 2008 17:20] Ingo Strüwing
Yes. I extended Marcos test like so:

create table t1(a int not null, b int, c char(10) not null, unique (c))
engine = innodb;
show create table t1;
drop index c on t1;
drop table t1;

create table t1(a int not null, b int, c char(10) not null, unique (b), unique (c))
engine = innodb;
show create table t1;
alter table t1 drop index c, drop index b;
drop table t1;

With my proposed change, both tests succeed.
[Note that for a successful completion of innodb.test Marcos patch needs to be changed so that all alter_table_flags except of HA_ONLINE_DROP_UNIQUE_INDEX_NO_WRITES have to be deleted.]
[11 Jan 2008 9:38] Martin Skold
Shouldn't it be:
-        /* 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 ((! my_strcasecmp(system_charset_info,
+                             key->name, primary_key_name)) ||
+            (unique_index_count-- == 1))
and
-        /* Unique key. Check for "PRIMARY". */
-        if (! my_strcasecmp(system_charset_info,
-                            key->name, primary_key_name))
+        /*
+           Unique key. Check for "PRIMARY"
+           or if adding first unique key
+        */
+        if ((! my_strcasecmp(system_charset_info,
+                             key->name, primary_key_name)) ||
+            (unique_index_count++ == 0) ||
+            (needed_online_flags & HA_ONLINE_DROP_PK_INDEX))

Don't follow the comment that flags need to be deleted,
having both HA_ONLINE_DROP_PK_INDEX and HA_ONLINE_ADD_PK_INDEX
signals change of PK (in 5.2 with the new interface I actually
added HA_ALTER_PK_INDEX, but I will need to merge this bugfix
with that).
[11 Jan 2008 9:49] Martin Skold
I correct myself, should of course be be:
+        /*
+           Unique key. Check for "PRIMARY"
+           or if adding first unique key
+        */
+        if ((! my_strcasecmp(system_charset_info,
+                             key->name, primary_key_name)) ||
+            (unique_index_count++ == 0))
since the counter alone is now enough.
[11 Jan 2008 10:08] Martin Skold
In the case of explicit drop of primary key when there are
other unique indexes the following code needs to be added too:

    if ((unique_index_count > 0) && 
        (needed_online_flags &  HA_ONLINE_DROP_PK_INDEX))
    {
      /*
        Dropped primary key when there is some other unique 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;
    }
[8 Feb 2008 16:12] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/41946

ChangeSet@1.2649, 2008-02-08 17:11:33+01:00, mskold@mysql.com +2 -0
  bug#31233  mysql_alter_table() fails to drop UNIQUE KEY
[19 Feb 2008 17:07] Heikki Tuuri
Marko, you could test the patch?

This is needed in 5.1.
[27 Feb 2008 15:30] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/43070

ChangeSet@1.2650, 2008-02-27 16:29:43+01:00, mskold@mysql.com +5 -0
  bug#31233 mysql_alter_table() fails to drop UNIQUE KEY: post review fixes
[27 Feb 2008 16:11] Martin Skold
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/43079

ChangeSet@1.2649, 2008-02-27 17:07:37+01:00, mskold@mysql.com +5 -0
  mysql_priv.h:
    bug #31231  mysql_alter_table() tries to drop a non-existing table: added FRM_ONLY
flag
  ha_ndbcluster.cc:
    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
  ndb_alter_table3.test, ndb_alter_table3.result:
    bug#31233 mysql_alter_table() fails to drop UNIQUE KEY: added test cases
  sql_table.cc:
    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.
[28 Feb 2008 8:53] Marko Mäkelä
The patch http://lists.mysql.com/commits/43079 seems to fix both Bug #31231 and Bug #31233.
[12 Mar 2008 13:43] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/43840

ChangeSet@1.2531, 2008-03-12 14:43:09+01:00, mskold@mysql.com +1 -0
  bug#31233 mysql_alter_table() fails to drop UNIQUE KEY: Post-review fixes
[12 Mar 2008 14:23] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/43848

ChangeSet@1.2530, 2008-03-12 15:22:52+01:00, mskold@mysql.com +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
[12 Mar 2008 14:59] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/43854

ChangeSet@1.2549, 2008-03-12 15:58:39+01:00, mskold@mysql.com +3 -0
  bug#31233 mysql_alter_table() fails to drop UNIQUE KEY: Adpted patch to new online alter interface
[12 Mar 2008 18:56] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/43874

ChangeSet@1.2552, 2008-03-12 19:55:57+01:00, mskold@mysql.com +3 -0
  bug#31233 mysql_alter_table() fails to drop UNIQUE KEY: Adpted patch to new online alter interface
[12 Mar 2008 19:01] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/43875

ChangeSet@1.2549, 2008-03-12 20:00:49+01:00, mskold@mysql.com +4 -0
  bug#31233 mysql_alter_table() fails to drop UNIQUE KEY: Adpted patch to new online alter interface
[13 Mar 2008 7:42] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/43899

ChangeSet@1.2553, 2008-03-13 08:41:47+01:00, mskold@mysql.com +1 -0
  ndb_alter_table.result:
    bug#31233 mysql_alter_table() fails to drop UNIQUE KEY: Re-generated result
[13 Mar 2008 14:28] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/43921

ChangeSet@1.2554, 2008-03-13 15:27:42+01:00, mskold@mysql.com +1 -0
  bug#31233 mysql_alter_table() fails to drop UNIQUE KEY: Wrong key read
[13 Mar 2008 14:32] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/43922

ChangeSet@1.2555, 2008-03-13 15:31:33+01:00, mskold@mysql.com +1 -0
  bug#31233 mysql_alter_table() fails to drop UNIQUE KEY: Removed compiler warning
[13 Mar 2008 14:43] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/43923

ChangeSet@1.2556, 2008-03-13 15:42:38+01:00, mskold@mysql.com +1 -0
  bug#31233 mysql_alter_table() fails to drop UNIQUE KEY: Wrong offset
[4 Apr 2008 22:33] Jon Stephens
Pushed to 5.1.23-ndb-6.2.15 and 5.1.23-ndb-6.3.11.
[17 Sep 2008 12:12] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/54239

2836 Martin Skold	2008-09-15 [merge]
      Null merge, patch will be merged from mysql-6.0-ndb

 2835 Georgi Kodinov	2008-09-12
       Bug#38342 -backup_myisam1 test fails on Windows
       The problem is that  this test, uses file locking and myisam unlocks 
       the same file region twice. Unix my_lock implementation based on fcntl() 
       does not return error in this case, but Windows UnlockFile() returns 
       ERROR_NOT_LOCKED.
            
       Fixed by ignoring ERROR_NOT_LOCKED from UnlockFile(). This fix does 
       not make myisam  better, but at least the implementation of my_lock() 
       compatible to *nix.
[17 Sep 2008 12:12] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/54240

2739 Martin Skold	2008-09-15
      bug #31231  mysql_alter_table() tries to drop a non-existing table
      bug#31233 mysql_alter_table() fails to drop UNIQUE KEY
[17 Sep 2008 12:12] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/54241

2836 Martin Skold	2008-09-15 [merge]
      Null merge, patch will be merged from mysql-6.0-ndb

-- 
MySQL Code Commits Mailing List
For list archives: http://lists.mysql.com/commits
To unsubscribe:    http://lists.mysql.com/commits?unsub=commits@bugs.mysql.com
[17 Sep 2008 12:12] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/54242

2739 Martin Skold	2008-09-15
      bug #31231  mysql_alter_table() tries to drop a non-existing table
      bug#31233 mysql_alter_table() fails to drop UNIQUE KEY
[8 Oct 2008 7:57] Jon Stephens
Hi,

AFAICT, there are no user-facing changes to document (which is why there were no
Cluster-6.2/6.3 changelog entries for this bug).

If this is correct, then this bug can be closed following the merge to 5.1-main.

Otherwise, please indicate what user-facing change(s) need to be noted in the docs.

Thanks!
[9 Oct 2008 18:17] Bugs System
Pushed into 5.1.30  (revid:martin.skold@mysql.com-20080915091956-kv0n9bexs08cdlup) (version source revid:kpettersson@mysql.com-20080915214458-ejxj7ltezohabr3z) (pib:4)
[15 Oct 2008 15:02] Paul Dubois
This is actually pushed to 5.1.29, not 5.1.30.
[17 Oct 2008 16:44] Bugs System
Pushed into 6.0.8-alpha  (revid:martin.skold@mysql.com-20080915091956-kv0n9bexs08cdlup) (version source revid:kpettersson@mysql.com-20080915213305-1ljm3tx7tgsdrne9) (pib:5)
[20 Oct 2008 20:18] Martin Skold
Jon Stephens wrote:
>Hi,
>
>AFAICT, there are no user-facing changes to document (which is why there were >no
>Cluster-6.2/6.3 changelog entries for this bug).
>
>If this is correct, then this bug can be closed following the merge to >5.1-main.
>
>Otherwise, please indicate what user-facing change(s) need to be noted in the >docs.
>
>Thanks!

This is correct, the bug fix does not change anything a user would notice, it
just fixes incorrect behavior/semantics of the new on-line alter table.
The behavior is now what should be expected and identical to the current off-line alter table.
[21 Oct 2008 17:44] Paul Dubois
No changelog entry needed.
[28 Oct 2008 21:02] Bugs System
Pushed into 5.1.29-ndb-6.2.17  (revid:martin.skold@mysql.com-20080915091956-kv0n9bexs08cdlup) (version source revid:tomas.ulin@sun.com-20081028140209-u4emkk1xphi5tkfb) (pib:5)
[28 Oct 2008 22:21] Bugs System
Pushed into 5.1.29-ndb-6.3.19  (revid:martin.skold@mysql.com-20080915091956-kv0n9bexs08cdlup) (version source revid:tomas.ulin@sun.com-20081028194045-0353yg8cvd2c7dd1) (pib:5)
[1 Nov 2008 9:48] Bugs System
Pushed into 5.1.29-ndb-6.4.0  (revid:martin.skold@mysql.com-20080915091956-kv0n9bexs08cdlup) (version source revid:jonas@mysql.com-20081101082305-qx5a1bj0z7i8ueys) (pib:5)
[12 Dec 2008 23:29] Bugs System
Pushed into 6.0.6-alpha  (revid:sp1r-mskold/marty@mysql.com/quadfish.(none)-20080313144238-63397) (version source revid:sp1r-tomas@poseidon.ndb.mysql.com-20080516085603-30848) (pib:5)