[MDEV-22934] Table disappear after two alter table command Created: 2020-06-18  Updated: 2023-11-27  Resolved: 2020-08-18

Status: Closed
Project: MariaDB Server
Component/s: Data Definition - Alter Table
Affects Version/s: 10.1, 10.3.23, 10.2, 10.3, 10.4, 10.5
Fix Version/s: 10.1.48, 10.2.35, 10.3.26, 10.4.16, 10.5.7

Type: Bug Priority: Critical
Reporter: Lay András Assignee: Thirunarayanan Balathandayuthapani
Resolution: Fixed Votes: 0
Labels: None
Environment:

Raspberry pi 2B
10.3.22-MariaDB-1ubuntu1

Issue Links:
Blocks

 Description   

I create three tables:

CREATE TABLE `parent1` (
 
  `id1` smallint(5) unsigned NOT NULL AUTO_INCREMENT,
 
  PRIMARY KEY (`id1`)
 
) ENGINE=InnoDB DEFAULT CHARSET=utf8;
 
 
 
CREATE TABLE `parent2` (
 
  `id2` smallint(5) unsigned NOT NULL AUTO_INCREMENT,
 
  PRIMARY KEY (`id2`)
 
) ENGINE=InnoDB DEFAULT CHARSET=utf8;
 
 
 
CREATE TABLE `child` (
 
  `id1` smallint(5) unsigned NOT NULL,
 
  `id2` smallint(5) unsigned NOT NULL,
 
  PRIMARY KEY (`id1`,`id2`),
 
  UNIQUE KEY `id2` (`id2`) USING BTREE,
 
  CONSTRAINT `child_ibfk_1` FOREIGN KEY (`id2`) REFERENCES `parent2` (`id2`) ON DELETE CASCADE,
 
  CONSTRAINT `child_ibfk_2` FOREIGN KEY (`id1`) REFERENCES `parent1` (`id1`) ON DELETE CASCADE
 
) ENGINE=InnoDB DEFAULT CHARSET=utf8;

and running theese commands:

MariaDB [LUFI]> SET FOREIGN_KEY_CHECKS=0;
 
Query OK, 0 rows affected (0.001 sec)
 
 
 
MariaDB [LUFI]> ALTER TABLE `child` DROP PRIMARY KEY;
 
Query OK, 0 rows affected (0.276 sec)
 
Records: 0  Duplicates: 0  Warnings: 0
 
 
 
MariaDB [LUFI]> ALTER TABLE `child` DROP INDEX `id2`;
 
ERROR 1025 (HY000): Error on rename of './LUFI/#sql-511_321' to './LUFI/child' (errno: 150 "Foreign key constraint is incorrectly formed")
 
MariaDB [LUFI]> SHOW TABLES; 
+----------------+
 
| Tables_in_LUFI |
 
+----------------+
 
| parent1        |
 
| parent2        |
 
+----------------+
 
2 rows in set (0.002 sec)
 
 
 
MariaDB [LUFI]> CREATE TABLE `child` (
 
    ->   `id` smallint(5) unsigned NOT NULL
 
    -> ) ENGINE=InnoDB DEFAULT CHARSET=utf8;
 
ERROR 1005 (HY000): Can't create table `LUFI`.`child` (errno: 150 "Foreign key constraint is incorrectly formed")
 
MariaDB [LUFI]>

As seen the table "child" disappeared, and can't recreate.

 Comments   
Comment by Elena Stepanova [ 2020-06-18 ]

Thanks for the report. Reproducible as described.

Comment by Elena Stepanova [ 2020-06-18 ]

The problem was first introduced in 10.0.38 by this commit:

commit a0f3b9f94f3094ccb9ce75c53b25d36c4c78e922
 
Author: Thirunarayanan Balathandayuthapani <thiru@mariadb.com>
 
Date:   Thu Jan 24 13:52:51 2019 +0530
 
 
 
    MDEV-17376 Server fails to set ADD_PK_INDEX, DROP_PK_INDEX if unique index nominated as PK

At the beginning there was a debug assertion happening even earlier, on DROP PRIMARY KEY, but the release build would eventually end up with the lost table.

Comment by Thirunarayanan Balathandayuthapani [ 2020-06-30 ]

The following test case repeats the same scenaro without triggering MDEV-17376 Scenario:

--source include/have_innodb.inc
 
CREATE TABLE t1(f1 INT NOT NULL AUTO_INCREMENT,
 
                f2 INT NOT NULL,
 
                PRIMARY KEY (f1), INDEX (f2))ENGINE=InnoDB;
 
 
 
CREATE TABLE t2(f1 INT NOT NULL,
 
f2 INT NOT NULL, f3 INT NOT NULL, PRIMARY KEY(f1, f2), UNIQUE KEY(f2),
 
CONSTRAINT `t2_ibfk_1` FOREIGN KEY (f2) REFERENCES t1(f2) ON DELETE CASCADE,
 
CONSTRAINT `t2_ibfk_2` FOREIGN KEY (f1) REFERENCES t1(f1) ON DELETE CASCADE
 
) ENGINE=InnoDB;
 
 
 
SET FOREIGN_KEY_CHECKS=0;
 
 
 
ALTER TABLE t2 DROP PRIMARY KEY, ADD PRIMARY KEY(f3), ALGORITHM=INPLACE;
 
 
 
# There are 3 renames could happen in copy algorithm
 
# rename t2 to #sql2
 
# rename #sql to t2 (it fails to detect the index for t2_ibfk_1, fails)
 
# rename #sql2 to t2 to keep the original table.
 
# dict_load_foreign() checks for the first index and able to find index for it
 
# It detects t2_ibfk_2 and fails to find the index for it. It leads to
 
# the failure of rename operation.
 
 
 
--error ER_ERROR_ON_RENAME
 
ALTER TABLE t2 DROP INDEX `f2`, algorithm=copy;
 
 
 
--error ER_NO_SUCH_TABLE
 
SHOW CREATE TABLE t2;
 
 
 
--error ER_CANT_CREATE_TABLE
 
CREATE TABLE t2 (f1 INT NOT NULL)ENGINE=InnoDB;

I added the root cause in test case. If foreign_key_check is disabled then inplace algorithm doesn't check for FK relation. It leads to contain
unused foreign key relation in SYS_FOREIGN table. Concurrent copy triggers rename and checks foreign key relation irrespective of the
foreign_key_check variable. It leads to the issue. It happens in 10.2.21 too

Comment by Thirunarayanan Balathandayuthapani [ 2020-07-06 ]

Patch is in bb-10.1-thiru

Comment by Marko Mäkelä [ 2020-07-13 ]

Can we fix this by passing a different parameter to the following call, to also evaluate new_is_tmp?

		/* We only want to switch off some of the type checking in
 
		an ALTER, not in a RENAME. */
 
 
 
		err = dict_load_foreigns(
 
			new_name, NULL,
 
			false, !old_is_tmp || trx->check_foreigns,
 
			DICT_ERR_IGNORE_NONE);

I think that we should try to avoid the following change:

diff --git a/mysql-test/suite/innodb/t/innodb.test b/mysql-test/suite/innodb/t/innodb.test
 
index 8d1004e679f..48834d529f1 100644
 
--- a/mysql-test/suite/innodb/t/innodb.test
 
+++ b/mysql-test/suite/innodb/t/innodb.test
 
@@ -1626,7 +1626,6 @@ drop table t1;
 
 set foreign_key_checks=0;
 
 create table t2 (a varchar(10), foreign key (a) references t1(a)) engine = innodb;
 
 create table t1(a varchar(10) primary key) engine = innodb;
 
--- error 1025,1025
 
 alter table t1 modify column a int;
 
 set foreign_key_checks=1;
 
 drop table t2,t1;

Perhaps we should simply implement this check already in ha_innobase::create() when the #sql table is being created? That could let us to skip the dict_load_foreigns() call altogether on ALTER TABLE.

Also, please try to write a clear commit message (or include in the commit a test case) that explains exactly what was wrong and how we are improving it. There are two rename operations in ALTER TABLE…ALGORITHM=COPY, and the above code snippet does not say which one it is referring to.

Comment by Thirunarayanan Balathandayuthapani [ 2020-08-06 ]

Patch is in bb-10.2-MDEV-22934

Comment by Marko Mäkelä [ 2020-08-07 ]

The suggested fix affects not only ALTER TABLE but also RENAME TABLE. Also, it would seem to make part of a subsequent condition !old_is_tmp || trx->check_foreigns redundant and invalidate the immediately following comment:

diff --git a/storage/innobase/row/row0mysql.cc b/storage/innobase/row/row0mysql.cc
 
index 7c61ad9b45b..5203e26679a 100644
 
--- a/storage/innobase/row/row0mysql.cc
 
+++ b/storage/innobase/row/row0mysql.cc
 
@@ -4524,6 +4524,12 @@ row_rename_table_for_mysql(
 
 			innobase_rename_vc_templ(table);
 
 		}
 
 
+		/* Avoid renaming of foreign key constraints
 
+		if foreign_key_check is disabled */
 
+		if (!trx->check_foreigns) {
 
+			goto funct_exit;
 
+		}
 
+
 
 		/* We only want to switch off some of the type checking in
 
 		an ALTER TABLE...ALGORITHM=COPY, not in a RENAME. */
 
 		dict_names_t	fk_tables;

Instead of this change, could we improve the error handling of the dict_load_foreigns() call? That is, when foreign key checks are disabled, display warning messages instead of error messages, and do not attempt to roll back the operation? And maybe simply pass the parameter check_charsets=!old_is_tmp to that call?

Comment by Matthias Leich [ 2020-08-13 ]

The tree commit 935415a9cdecf87a66166921e3f7594bf80fce77 (HEAD, origin/bb-10.2-thiru)
containing fixes for MDEV-22934 and MDEV-23380 performed well during RQG testing.

Generated at Thu Feb 08 09:18:35 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.