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

MariaDB crashes with foreign_key_checks=0 when changing a column and adding a foreign key at the same time

Details

    Description

      docker run --rm -p 20433:3306 -e MARIADB_DATABASE=test -e MARIADB_ROOT_PASSWORD=root mariadb:11.1.2
      		 
      mysql -h 127.0.0.1 --port=20433 -u root -proot test < migrate.sql

      migrate.sql:

      CREATE TABLE items (
      		 
          id VARCHAR(20) NOT NULL,
      		 
          PRIMARY KEY (id)
      		 
      );
      		 
      CREATE TABLE sgs_data (
      		 
          id INT(11) NOT NULL AUTO_INCREMENT,
      		 
          Number varchar(15) NOT NULL DEFAULT '',
      		 
          PRIMARY KEY (id)
      		 
      );
      		 
      SET foreign_key_checks=0;
      		 
      ALTER TABLE sgs_data
      		 
          CHANGE COLUMN `Number` itemId VARCHAR(20) NOT NULL,
      		 
          ADD CONSTRAINT fk_sgs_data_itemId FOREIGN KEY (itemId) REFERENCES items (id);

      This causes the MariaDB server to crash (see attached log output).

      • Moving the CHANGE COLUMN to a separate ALTER statement avoids the crash
      • Removing `SET foreign_key_checks=0` avoids the crash
      • Removing `CREATE TABLE items` still results in a crash even though the foreign key is invalid. If you additionally remove the `SET foreign_key_checks=0` it will instead report that the foreign key is malformed and not crash.

      Verified on 10.6.15, 10.9.8 and 11.1.2 (all with docker).

      Attachments

        Issue Links

          is caused by

          Bug - A problem which impairs or prevents the functions of the product. MDEV-31086 MODIFY COLUMN can break FK constraints, and lead to unrestorable dumps

          • Critical - Crashes, loss of data, severe memory leak.
          • Closed

          Activity

            Ascending order - Click to sort in descending order
            marko Marko Mäkelä added a comment -

            This is something that was missed in MDEV-32527, MDEV-32337, MDEV-32060, MDEV-31869 and caused by the fix of MDEV-31086.

            We fail to check for a null pointer:

            diff --git a/storage/innobase/handler/handler0alter.cc b/storage/innobase/handler/handler0alter.cc
            		 
            index 092aa54737d..6da95e42c11 100644
            		 
            --- a/storage/innobase/handler/handler0alter.cc
            		 
            +++ b/storage/innobase/handler/handler0alter.cc
            		 
            @@ -7759,12 +7759,15 @@ bool check_col_is_in_fk_indexes(
            		 
             
               for (const auto &a : add_fk)
            		 
               {
            		 
            -    for (ulint i= 0; i < a->n_fields; i++)
            		 
            +    if (const dict_index_t* foreign_index= a->foreign_index)
            		 
                 {
            		 
            -      if (a->foreign_index->fields[i].col == col)
            		 
            +      for (ulint i= 0; i < a->n_fields; i++)
            		 
                   {
            		 
            -        fk_id= a->id;
            		 
            -        goto err_exit;
            		 
            +        if (foreign_index->fields[i].col == col)
            		 
            +        {
            		 
            +          fk_id= a->id;
            		 
            +          goto err_exit;
            		 
            +        }
            		 
                   }
            		 
                 }
            		 
               }

            marko Marko Mäkelä added a comment - This is something that was missed in MDEV-32527 , MDEV-32337 , MDEV-32060 , MDEV-31869 and caused by the fix of MDEV-31086 . We fail to check for a null pointer: diff --git a/storage/innobase/handler/handler0alter.cc b/storage/innobase/handler/handler0alter.cc index 092aa54737d..6da95e42c11 100644 --- a/storage/innobase/handler/handler0alter.cc +++ b/storage/innobase/handler/handler0alter.cc @@ -7759,12 +7759,15 @@ bool check_col_is_in_fk_indexes( for (const auto &a : add_fk) { - for (ulint i= 0; i < a->n_fields; i++) + if (const dict_index_t* foreign_index= a->foreign_index) { - if (a->foreign_index->fields[i].col == col) + for (ulint i= 0; i < a->n_fields; i++) { - fk_id= a->id; - goto err_exit; + if (foreign_index->fields[i].col == col) + { + fk_id= a->id; + goto err_exit; + } } } }
            thiru Thirunarayanan Balathandayuthapani added a comment -

            https://github.com/MariaDB/server/pull/2814

            thiru Thirunarayanan Balathandayuthapani added a comment - https://github.com/MariaDB/server/pull/2814
            marko Marko Mäkelä added a comment -

            OK to push.

            marko Marko Mäkelä added a comment - OK to push.

            People

              thiru Thirunarayanan Balathandayuthapani
              SystemParadox Simon Williams
              Votes:
              0 Vote for this issue
              Watchers:
              3 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.