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

Foreign key regression from 10.11.9 to 10.11.10 (and 11.4.4)

Details

    Description

      After upgrading to v10.11.10 creation of a foreign key failed with no information in engine innodb status. The same keys are allowed in 10.11.9 and below.

      reproducer SQL:

      CREATE TABLE `a` (
        `a_id` int(10) unsigned NOT NULL AUTO_INCREMENT,
        `a_p_id` int(10) unsigned DEFAULT NULL,
        PRIMARY KEY (`a_id`),
        FOREIGN KEY (`a_p_id`) REFERENCES `a` (`a_id`)
      ) ENGINE=InnoDBa
       
       
      CREATE TABLE `b` (
        `b_id` int(10) unsigned NOT NULL AUTO_INCREMENT,
        `b_p_id` int(10) unsigned DEFAULT NULL,
        PRIMARY KEY (`b_id`),
        FOREIGN KEY (`b_p_id`) REFERENCES `b` (`b_id`)
      ) ENGINE=InnoDB
       
       
      CREATE TABLE `b_a` (
        `b_id` int(10) unsigned NOT NULL,
        `a_p_id` int(10) unsigned NOT NULL,
        `a_id` int(10) unsigned DEFAULT NULL,
        PRIMARY KEY (`b_id`,`a_p_id`),
        FOREIGN KEY (`b_id`) REFERENCES `b` (`b_id`) ON DELETE CASCADE ON UPDATE CASCADE,
        FOREIGN KEY (`a_p_id`, `a_id`) REFERENCES `a` (`a_p_id`, `a_id`) ON UPDATE CASCADE
      ) ENGINE=InnoDB
      

      Changing the table b_a latest foreign key to "on delete cascade" does allow the table to be created, but that is not what I want to archieve.

      Attachments

        Issue Links

          Activity

            The column a.a_id is defined as NOT NULL, while b_a.a_id allows NULL values. It has been a while since I looked at FOREIGN KEY constraints, but it seems to me that the ON UPDATE CASCADE this way around should be allowed. If it were the other way around (the parent table does not allow NULL values but the child table column does not), then it would seem to me that ON UPDATE CASCADE can potentially be incorrect (trying to update a NOT NULL column to the NULL value).

            thiru, can you please investigate this? Did MDEV-34392 get something wrong?

            marko Marko Mäkelä added a comment - The column a.a_id is defined as NOT NULL , while b_a.a_id allows NULL values. It has been a while since I looked at FOREIGN KEY constraints, but it seems to me that the ON UPDATE CASCADE this way around should be allowed. If it were the other way around (the parent table does not allow NULL values but the child table column does not), then it would seem to me that ON UPDATE CASCADE can potentially be incorrect (trying to update a NOT NULL column to the NULL value). thiru , can you please investigate this? Did MDEV-34392 get something wrong?

            CREATE TABLE `a` (
              `a_id` int(10) unsigned NOT NULL AUTO_INCREMENT,
              `a_p_id` int(10) unsigned DEFAULT NULL,
              PRIMARY KEY (`a_id`),
              FOREIGN KEY (`a_p_id`) REFERENCES `a` (`a_id`)
            ) ENGINE=InnoDB
             
             CREATE TABLE `b_a` (
              `b_id` int(10) unsigned NOT NULL,
              `a_p_id` int(10) unsigned NOT NULL,
              `a_id` int(10) unsigned DEFAULT NULL,
              PRIMARY KEY (`b_id`,`a_p_id`),
              FOREIGN KEY (`a_p_id`, `a_id`) REFERENCES `a` (`a_p_id`, `a_id`) ON UPDATE CASCADE
            ) ENGINE=InnoDB;
            

            Here a_p_id in b_a table should be non null, but a_p_id in a can be null. This is what MDEV-34392 addresses.
            Here the foreign key constraint is UPDATE CASCADE and referencing key column is NOT NULL , referenced column declared as NULL.

            thiru Thirunarayanan Balathandayuthapani added a comment - CREATE TABLE `a` ( `a_id` int(10) unsigned NOT NULL AUTO_INCREMENT, `a_p_id` int(10) unsigned DEFAULT NULL, PRIMARY KEY (`a_id`), FOREIGN KEY (`a_p_id`) REFERENCES `a` (`a_id`) ) ENGINE=InnoDB   CREATE TABLE `b_a` ( `b_id` int(10) unsigned NOT NULL, `a_p_id` int(10) unsigned NOT NULL, `a_id` int(10) unsigned DEFAULT NULL, PRIMARY KEY (`b_id`,`a_p_id`), FOREIGN KEY (`a_p_id`, `a_id`) REFERENCES `a` (`a_p_id`, `a_id`) ON UPDATE CASCADE ) ENGINE=InnoDB; Here a_p_id in b_a table should be non null, but a_p_id in a can be null. This is what MDEV-34392 addresses. Here the foreign key constraint is UPDATE CASCADE and referencing key column is NOT NULL , referenced column declared as NULL.

            The reasoning is that if a_id as an a_p_id in a (a parent) and the parent is changed, this is changed in b_a as well. The 2nd goal of this foreign key is that once b_id is chained to a_id with an a_p_id, both are disallowed to be deleted. It does exactly that in <= 10.11.9 but is a bit fishy.

            Adding ON DELETE CASCADE to the foreign key allows the key to be created but does this leave the semantics unchanged?

            dicode Tim Westervoorde added a comment - The reasoning is that if a_id as an a_p_id in a (a parent) and the parent is changed, this is changed in b_a as well. The 2nd goal of this foreign key is that once b_id is chained to a_id with an a_p_id, both are disallowed to be deleted. It does exactly that in <= 10.11.9 but is a bit fishy. Adding ON DELETE CASCADE to the foreign key allows the key to be created but does this leave the semantics unchanged?

            Below test case is executed in 10.11.9

            CREATE TABLE t1(
            f1 INT DEFAULT NULL,
            f2 INT NOT NULL AUTO_INCREMENT,
            PRIMARY KEY(f2),
            KEY(f1))ENGINE=InnoDB;
             
            CREATE TABLE t2(
            f1 INT NOT NULL,
            f2 INT DEFAULT NULL,
            FOREIGN KEY(f1, f2) REFERENCES t1(f1, f2) ON UPDATE CASCADE)ENGINE=InnoDB;
            INSERT INTO t1 VALUES(1, 1);
            INSERT INTO t2 VALUES(1, 1);
            UPDATE t1 SET f1 = NULL;
             
            mysqltest: At line 14: query 'UPDATE t1 SET f1 = NULL' failed: ER_ROW_IS_REFERENCED_2 (1451): Cannot delete or update a parent row: a foreign key constraint fails (`test`.`t2`, CONSTRAINT `t2_ibfk_1` FOREIGN KEY (`f1`, `f2`) REFERENCES `t1` (`f1`, `f2`) ON UPDATE CASCADE)
            

            This is what we're trying to avoid by disallowing in create table statement.

            thiru Thirunarayanan Balathandayuthapani added a comment - Below test case is executed in 10.11.9 CREATE TABLE t1( f1 INT DEFAULT NULL, f2 INT NOT NULL AUTO_INCREMENT, PRIMARY KEY(f2), KEY(f1))ENGINE=InnoDB;   CREATE TABLE t2( f1 INT NOT NULL, f2 INT DEFAULT NULL, FOREIGN KEY(f1, f2) REFERENCES t1(f1, f2) ON UPDATE CASCADE)ENGINE=InnoDB; INSERT INTO t1 VALUES(1, 1); INSERT INTO t2 VALUES(1, 1); UPDATE t1 SET f1 = NULL;   mysqltest: At line 14: query 'UPDATE t1 SET f1 = NULL' failed: ER_ROW_IS_REFERENCED_2 (1451): Cannot delete or update a parent row: a foreign key constraint fails (`test`.`t2`, CONSTRAINT `t2_ibfk_1` FOREIGN KEY (`f1`, `f2`) REFERENCES `t1` (`f1`, `f2`) ON UPDATE CASCADE) This is what we're trying to avoid by disallowing in create table statement.

            With ON DELETE CASCADE there is no problem. When a row that is being deleted from the parent table contains a NULL value in a referenced column, then there obviously can’t be any corresponding record in the child table whose referencing column is NOT NULL. In that case, we can simply delete the record from the parent table.

            The operations ON…SET NULL or ON UPDATE CASCADE would attempt to update the referencing column to NULL. That is obviously not possible when the referencing column is declared as NOT NULL. We want to prevent a DML time error by checking this already at DDL time.

            dicode, does this explanation make sense to you?

            marko Marko Mäkelä added a comment - With ON DELETE CASCADE there is no problem. When a row that is being deleted from the parent table contains a NULL value in a referenced column, then there obviously can’t be any corresponding record in the child table whose referencing column is NOT NULL . In that case, we can simply delete the record from the parent table. The operations ON…SET NULL or ON UPDATE CASCADE would attempt to update the referencing column to NULL . That is obviously not possible when the referencing column is declared as NOT NULL . We want to prevent a DML time error by checking this already at DDL time. dicode , does this explanation make sense to you?

            Yeah, I think it makes sense and this foreign key is somewhat fishy declared. I'll go back to the drawing table and see if I can define a proper foreign key which does what I want and also 10.11.10+ compatible. I'll try to get back later today with my findings.

            dicode Tim Westervoorde added a comment - Yeah, I think it makes sense and this foreign key is somewhat fishy declared. I'll go back to the drawing table and see if I can define a proper foreign key which does what I want and also 10.11.10+ compatible. I'll try to get back later today with my findings.

            marko I'm not sure how to explain what I want but I'll give it a try.

            CREATE TABLE `fg` (
              `fg_id` int(10) unsigned NOT NULL AUTO_INCREMENT,
              `fg_p_id` int(10) unsigned DEFAULT NULL,
              PRIMARY KEY (`fg_id`),
              FOREIGN KEY (`fg_p_id`) REFERENCES `fg` (`fg_id`)
            ) ENGINE=InnoDB;
             
            insert into fg values (1,null),(2,1),(3,null),(4,3),(5,1);
            

            This table is holding a 2-level tree. There are parents and children. For example, 1 and 3 are parents, 2, 4 and 5 are children.

            CREATE TABLE `f` (
              `f_id` int(10) unsigned NOT NULL AUTO_INCREMENT,
              PRIMARY KEY (`f_id`)
            ) ENGINE=InnoDB;
             
            insert into f values (1);
            

            This table holds items, in this case just a single one, 1. Now I want to categorize these items with the parent/childs in fg. So i create a table as follows

            CREATE TABLE `f_fg` (
              `f_id` int(10) unsigned NOT NULL,
              `fg_p_id` int(10) unsigned NOT NULL,
              `fg_id` int(10) unsigned DEFAULT NULL,
              `fg` int(10) unsigned NOT NULL DEFAULT '0',
              PRIMARY KEY (`f_id`,`fg_p_id`,`fg`),
              FOREIGN KEY (`f_id`) REFERENCES `f` (`f_id`) ON DELETE CASCADE ON UPDATE CASCADE,
              FOREIGN KEY (`fg_p_id`, `fg_id`) REFERENCES `fg` (`fg_p_id`, `fg_id`) ON UPDATE CASCADE
            ) ENGINE=InnoDB;
             
            insert into f_fg values (1,1,2,1),(1,1,5,5),(1,3,4,4),(1,3,null,0);
            

            That is, each f_id is linked to a parent-child relation from table fg. As soon as a child from table fg is used the foreign key prevents promoting this item to a parent. There should also be a foreign key that forces fg_p_id in f_fg to be a valid f_id in table f but for simplicity it's excluded.

            <= 10.11.9 allows this to be created and prevents updating fg_p_id to null as soon as it is referenced in f_fg - a DML time error;
            10.11.10 doesn't allow this. But I can't seem to figure out how to do this properly. Am I trying to fix something with foreign keys which should be solved with foreign keys? Or am I missing something obviously here.

            dicode Tim Westervoorde added a comment - marko I'm not sure how to explain what I want but I'll give it a try. CREATE TABLE `fg` ( `fg_id` int(10) unsigned NOT NULL AUTO_INCREMENT, `fg_p_id` int(10) unsigned DEFAULT NULL, PRIMARY KEY (`fg_id`), FOREIGN KEY (`fg_p_id`) REFERENCES `fg` (`fg_id`) ) ENGINE=InnoDB;   insert into fg values (1,null),(2,1),(3,null),(4,3),(5,1); This table is holding a 2-level tree. There are parents and children. For example, 1 and 3 are parents, 2, 4 and 5 are children. CREATE TABLE `f` ( `f_id` int(10) unsigned NOT NULL AUTO_INCREMENT, PRIMARY KEY (`f_id`) ) ENGINE=InnoDB;   insert into f values (1); This table holds items, in this case just a single one, 1. Now I want to categorize these items with the parent/childs in fg. So i create a table as follows CREATE TABLE `f_fg` ( `f_id` int(10) unsigned NOT NULL, `fg_p_id` int(10) unsigned NOT NULL, `fg_id` int(10) unsigned DEFAULT NULL, `fg` int(10) unsigned NOT NULL DEFAULT '0', PRIMARY KEY (`f_id`,`fg_p_id`,`fg`), FOREIGN KEY (`f_id`) REFERENCES `f` (`f_id`) ON DELETE CASCADE ON UPDATE CASCADE, FOREIGN KEY (`fg_p_id`, `fg_id`) REFERENCES `fg` (`fg_p_id`, `fg_id`) ON UPDATE CASCADE ) ENGINE=InnoDB;   insert into f_fg values (1,1,2,1),(1,1,5,5),(1,3,4,4),(1,3,null,0); That is, each f_id is linked to a parent-child relation from table fg. As soon as a child from table fg is used the foreign key prevents promoting this item to a parent. There should also be a foreign key that forces fg_p_id in f_fg to be a valid f_id in table f but for simplicity it's excluded. <= 10.11.9 allows this to be created and prevents updating fg_p_id to null as soon as it is referenced in f_fg - a DML time error; 10.11.10 doesn't allow this. But I can't seem to figure out how to do this properly. Am I trying to fix something with foreign keys which should be solved with foreign keys? Or am I missing something obviously here.

            dicode, if I understood this correctly, you are intentionally using such a schema and indeed prefer to have a DML time error instead of a DDL time error. I agree that we should revise this.

            One possible fix could be that if sql_mode does not include STRICT_ALL_TABLES or STRICT_TRANS_TABLES, we would ignore any ON UPDATE inconsistency that is related to the referencing column being declared as NOT NULL.

            When it comes to ON UPDATE SET NULL, I can imagine that someone might want to (ab)use them as a kind of a stricter FOREIGN KEY constraint, not allowing any change in the parent table as long as a referencing record in a child table (with NOT NULL on that column) exists.

            I do not see any reason to allow ON DELETE SET NULL when the referencing column is declared NOT NULL. The same effect (prohibiting DELETE of the referenced row if a referencing row exists) would be achieved without defining any ON DELETE SET NULL.

            I tested the following variant of your example:

            --source include/have_innodb.inc
             
            CREATE TABLE fg (
              fg_id int unsigned AUTO_INCREMENT PRIMARY KEY,
              fg_p_id int unsigned REFERENCES fg (fg_id)
            ) ENGINE=InnoDB;
             
            insert into fg values (1,null),(2,1),(3,null),(4,3),(5,1);
             
            CREATE TABLE f (f_id int unsigned AUTO_INCREMENT PRIMARY KEY) ENGINE=InnoDB;
             
            insert into f values (1);
             
            CREATE TABLE f_fg (
              f_id int unsigned NOT NULL
              REFERENCES f(f_id) ON DELETE CASCADE ON UPDATE CASCADE,
              fg_p_id int unsigned,
              fg_id int unsigned,
              fg int unsigned NOT NULL,
              FOREIGN KEY (fg_p_id, fg_id) REFERENCES fg (fg_p_id, fg_id) ON UPDATE CASCADE
            ) ENGINE=InnoDB;
             
            insert into f_fg values (1,1,2,1),(1,1,5,5),(1,3,4,4),(1,3,null,0);
             
            update fg set fg_p_id=null where fg_id=4;
            select * from f_fg;
            drop table f_fg,f,fg;
            

            As far as I understand, you would want the original DDL to go through and the UPDATE statement to fail due to a foreign key constraint violation, because the record (1,3,4,4) cannot be updated to (1,NULL,4,4), since f_fg.fg_p_id is declared NOT NULL.

            Above, I "fixed" the DDL by removing the NOT NULL and by omitting a PRIMARY KEY in f_fg (all PRIMARY KEY columns are implicitly NOT NULL). Note: Omitting a PRIMARY KEY is a bad idea if you are using any replication; see MDEV-21181. InnoDB would add a hidden internal 48-bit column DB_ROW_ID that acts as the primary key, but it would not be visible outside InnoDB. It would be better to add an AUTO_INCREMENT PRIMARY KEY if one wanted to go this route.

            marko Marko Mäkelä added a comment - dicode , if I understood this correctly, you are intentionally using such a schema and indeed prefer to have a DML time error instead of a DDL time error. I agree that we should revise this. One possible fix could be that if sql_mode does not include STRICT_ALL_TABLES or STRICT_TRANS_TABLES , we would ignore any ON UPDATE inconsistency that is related to the referencing column being declared as NOT NULL . When it comes to ON UPDATE SET NULL , I can imagine that someone might want to (ab)use them as a kind of a stricter FOREIGN KEY constraint, not allowing any change in the parent table as long as a referencing record in a child table (with NOT NULL on that column) exists. I do not see any reason to allow ON DELETE SET NULL when the referencing column is declared NOT NULL . The same effect (prohibiting DELETE of the referenced row if a referencing row exists) would be achieved without defining any ON DELETE SET NULL . I tested the following variant of your example: --source include/have_innodb.inc   CREATE TABLE fg ( fg_id int unsigned AUTO_INCREMENT PRIMARY KEY , fg_p_id int unsigned REFERENCES fg (fg_id) ) ENGINE=InnoDB;   insert into fg values (1, null ),(2,1),(3, null ),(4,3),(5,1);   CREATE TABLE f (f_id int unsigned AUTO_INCREMENT PRIMARY KEY ) ENGINE=InnoDB;   insert into f values (1);   CREATE TABLE f_fg ( f_id int unsigned NOT NULL REFERENCES f(f_id) ON DELETE CASCADE ON UPDATE CASCADE , fg_p_id int unsigned, fg_id int unsigned, fg int unsigned NOT NULL , FOREIGN KEY (fg_p_id, fg_id) REFERENCES fg (fg_p_id, fg_id) ON UPDATE CASCADE ) ENGINE=InnoDB;   insert into f_fg values (1,1,2,1),(1,1,5,5),(1,3,4,4),(1,3, null ,0);   update fg set fg_p_id= null where fg_id=4; select * from f_fg; drop table f_fg,f,fg; As far as I understand, you would want the original DDL to go through and the UPDATE statement to fail due to a foreign key constraint violation, because the record (1,3,4,4) cannot be updated to (1,NULL,4,4), since f_fg.fg_p_id is declared NOT NULL . Above, I "fixed" the DDL by removing the NOT NULL and by omitting a PRIMARY KEY in f_fg (all PRIMARY KEY columns are implicitly NOT NULL ). Note: Omitting a PRIMARY KEY is a bad idea if you are using any replication; see MDEV-21181 . InnoDB would add a hidden internal 48-bit column DB_ROW_ID that acts as the primary key, but it would not be visible outside InnoDB. It would be better to add an AUTO_INCREMENT PRIMARY KEY if one wanted to go this route.

            I am wondering why there is no reference to the SQL standard about what is the prescribed behaviour there. Or is this not covered by the standard? How does it work in other databases?

            I do agree that to the extend that this concerns regressions in GA releases, this needs to be taken into account as well as the standard, but it still seems necessary to have a good understanding of what the standard says (if anything), and how things work in other databases.

            Hope this helps,

            - Kristian.

            knielsen Kristian Nielsen added a comment - I am wondering why there is no reference to the SQL standard about what is the prescribed behaviour there. Or is this not covered by the standard? How does it work in other databases? I do agree that to the extend that this concerns regressions in GA releases, this needs to be taken into account as well as the standard, but it still seems necessary to have a good understanding of what the standard says (if anything), and how things work in other databases. Hope this helps, - Kristian.

            markus you are right about my intentions, we explicitly use this kind of foreign key to protect the data against modifications which would result in a broken tree. Your mention about the primary key is why we use a "shadow" column `fg` which hasn't got a foreign key and mirrors fg_id to allow binding a f to a parent (fg_p_id) without selecting a child yet (fgid). I can live with the solution of allowing the DDL only when sql_mode does not include STRICT_ALL_TABLES or STRICT_TRANS_TABLES.

            dicode Tim Westervoorde added a comment - markus you are right about my intentions, we explicitly use this kind of foreign key to protect the data against modifications which would result in a broken tree. Your mention about the primary key is why we use a "shadow" column `fg` which hasn't got a foreign key and mirrors fg_id to allow binding a f to a parent (fg_p_id) without selecting a child yet (fgid). I can live with the solution of allowing the DDL only when sql_mode does not include STRICT_ALL_TABLES or STRICT_TRANS_TABLES.

            knielsen, I don’t have access to the standard. The services https://dbfiddle.dev and https://dbfiddle.uk could be used for checking how some other RDBMS would behave.

            marko Marko Mäkelä added a comment - knielsen , I don’t have access to the standard. The services https://dbfiddle.dev and https://dbfiddle.uk could be used for checking how some other RDBMS would behave.

            If I understood the test case correctly, the fix in its current form would not consistently allow ON UPDATE…SET NULL in non-strict sql_mode when the referencing column is declared NOT NULL. I’m glad to see that the addition to our regression test suite demonstrates that all the DML time checks are already in place.

            marko Marko Mäkelä added a comment - If I understood the test case correctly, the fix in its current form would not consistently allow ON UPDATE…SET NULL in non-strict sql_mode when the referencing column is declared NOT NULL . I’m glad to see that the addition to our regression test suite demonstrates that all the DML time checks are already in place.

            People

              thiru Thirunarayanan Balathandayuthapani
              dicode Tim Westervoorde
              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.