Uploaded image for project: 'MariaDB Server'
  1. MariaDB Server
  2. MDEV-22934

Table disappear after two alter table command

Details

    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.

      Attachments

        Activity

          Thanks for the report. Reproducible as described.

          elenst Elena Stepanova added a comment - Thanks for the report. Reproducible as described.

          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.

          elenst Elena Stepanova added a comment - 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.

          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

          thiru Thirunarayanan Balathandayuthapani added a comment - - edited 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

          Patch is in bb-10.1-thiru

          thiru Thirunarayanan Balathandayuthapani added a comment - Patch is in bb-10.1-thiru

          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.

          marko Marko Mäkelä added a comment - 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.

          Patch is in bb-10.2-MDEV-22934

          thiru Thirunarayanan Balathandayuthapani added a comment - Patch is in bb-10.2- MDEV-22934

          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?

          marko Marko Mäkelä added a comment - 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?

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

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

          People

            thiru Thirunarayanan Balathandayuthapani
            LaySoft Lay András
            Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Git Integration

                Error rendering 'com.xiplink.jira.git.jira_git_plugin:git-issue-webpanel'. Please contact your Jira administrators.