MariaDB / mariadb-docker

Docker Official Image packaging for MariaDB
https://mariadb.org
GNU General Public License v2.0
755 stars 436 forks source link

MDEV-32018 Changing column to autoincrement after upgrade 10.5.21 to 10.5.22 #528

Closed horrockss closed 7 months ago

horrockss commented 10 months ago

Hi,

We've just upgraded to 10.5.22 and have found a problem with changing an existing column to autoincrement.

SET FOREIGN_KEY_CHECKS=0; ALTER TABLE device_type ADD UNIQUE(name); ALTER TABLE device_type MODIFY id INT unsigned AUTO_INCREMENT;

Lookup Error - MySQL Database Error: Cannot change column 'id': used in a foreign key constraint 'FK_device_type' of table 'sandbox.device_field'

I've traced it to the upgrade to 10.5.22, as it isn't seen when using 10.5.21.

Do you have any idea what is going wrong?

Thanks.

horrockss commented 10 months ago

Please Note this works if you explicitly drop the constraint before changing the column to autoincrement

horrockss commented 10 months ago

The following script will recreate the problem:

CREATE TABLE `device_type` (
  `id` int(10) unsigned NOT NULL,
  `name` varchar(128) DEFAULT NULL,
  PRIMARY KEY (`id`)
) ENGINE=InnoDB DEFAULT CHARSET=latin1 COLLATE=latin1_swedish_ci;

CREATE TABLE `device_field` (
  `id` int(10) unsigned NOT NULL AUTO_INCREMENT,
  `name` varchar(128) DEFAULT NULL,
  `device_type_id` int(10) unsigned DEFAULT NULL,
  PRIMARY KEY (`id`),
  CONSTRAINT `FK_device_field_device_type` FOREIGN KEY (`device_type_id`) REFERENCES `device_type` (`id`)
) ENGINE=InnoDB DEFAULT CHARSET=latin1 COLLATE=latin1_swedish_ci;

SET FOREIGN_KEY_CHECKS=0;

select @@foreign_key_checks;

ALTER TABLE device_type MODIFY id INT unsigned AUTO_INCREMENT;
grooverdan commented 10 months ago

I traced this down to https://github.com/MariaDB/server/commit/f1746ad6dd2ff1849ac#diff-f223b918b8e982bb3edaed26dc567ac653c0cf35f5ca624e2e3b664d4be5d49d. I've asked its author if intentional .

grooverdan commented 10 months ago

The removal of the ability to use FOREIGN_KEY_CHECK to potentially corrupt data was intended in MDEV-31086.

The result of this is the error is cause by https://github.com/MariaDB/server/blob/mariadb-10.5.22/sql/field.cc#L9624 because its adding an auto_increment field this turns out to be not equal.

I tried removing the check, which incidently worked perfectly on InnoDB on your test case, but its original cause https://bugs.mysql.com/bug.php?id=14573 still fails on Aria.

The workaround MDEV-31987 is to do the painfully verbose, DROP FK key, add AI, re-add FK key.

grooverdan commented 10 months ago

tested the following as a server change that seems to work:

diff --git a/sql/sql_table.cc b/sql/sql_table.cc
index 8366e571f26..0476d76bcb6 100644
--- a/sql/sql_table.cc
+++ b/sql/sql_table.cc
@@ -9476,6 +9476,8 @@ fk_check_column_changes(THD *thd, Alter_info *alter_info,
         return FK_COLUMN_RENAMED;
       }

+      bool ai = new_field->flags & AUTO_INCREMENT_FLAG;
+      new_field->flags&= ~AUTO_INCREMENT_FLAG;
       if ((old_field->is_equal(*new_field) == IS_EQUAL_NO) ||
           ((new_field->flags & NOT_NULL_FLAG) &&
            !(old_field->flags & NOT_NULL_FLAG)))
@@ -9487,6 +9489,8 @@ fk_check_column_changes(THD *thd, Alter_info *alter_info,
         *bad_column_name= column->str;
         return FK_COLUMN_DATA_CHANGE;
       }
+      if (ai)
+        new_field->flags|= AUTO_INCREMENT_FLAG;
     }
     else
     {
grooverdan commented 10 months ago

MDEV-32018 written and https://github.com/MariaDB/server/pull/2739 as an improved version of the above codefix.

@FooBarrior if you wish to review that's fine by me :-).

Thanks for the bug report @horrockss.

grooverdan commented 8 months ago

As above, upstream fix has been committed and will be out next release (hopefully last next week or soon there after).

grooverdan commented 7 months ago

10.5.23 is now available that has this fixed. Appoligies for the delay. A few last minute stability problems where found and corrected.