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

INSERT IGNORE on constraints result in ERROR rather than warning

Details

    • Bug
    • Status: Closed (View Workflow)
    • Major
    • Resolution: Fixed
    • 10.4.31, 10.11.4
    • 11.4.1
    • Data Manipulation - Insert
    • MySQL version: 10.11.4-MariaDB-1:10.11.4+maria~ubu2204 through PHP extension MySQLi

    Description

      The following INSERT IGNORE emits an error instead of a warning:

      DROP TABLE IF EXISTS `my_table`;

      CREATE TABLE `my_table` (
      `first_col` char(63),
      `second_col` char(63),
      constraint `my_const` check (`first_col` != `second_col`)
      );

      INSERT IGNORE INTO `my_table` VALUES ('a', 'a');

      Attachments

        Issue Links

          relates to

          Bug - A problem which impairs or prevents the functions of the product. MDEV-31985 IGNORE in INSERT and LOAD are being ignored in InnoDB bulk insert

          • Major - Major loss of function.
          • Confirmed

          Activity

            Ascending order - Click to sort in descending order
            jkoop Joe Koop created issue -
            jkoop Joe Koop made changes -
            Field Original Value New Value
            Description The following INSERT IGNORE emits an error instead of a warning:

            {{DROP TABLE IF EXISTS `my_table`;

            CREATE TABLE `my_table` (
                `first_col` char(63),
                `second_col` char(63),
                constraint `my_const` check (`first_col` != `second_col`)
            );

            INSERT IGNORE INTO `my_table` VALUES ('a', 'a');}}             		The following INSERT IGNORE emits an error instead of a warning:

            {{
            DROP TABLE IF EXISTS `my_table`;

            CREATE TABLE `my_table` (
                `first_col` char(63),
                `second_col` char(63),
                constraint `my_const` check (`first_col` != `second_col`)
            );

            INSERT IGNORE INTO `my_table` VALUES ('a', 'a');
            }}
            jkoop Joe Koop made changes -
            Description The following INSERT IGNORE emits an error instead of a warning:

            {{
            DROP TABLE IF EXISTS `my_table`;

            CREATE TABLE `my_table` (
                `first_col` char(63),
                `second_col` char(63),
                constraint `my_const` check (`first_col` != `second_col`)
            );

            INSERT IGNORE INTO `my_table` VALUES ('a', 'a');
            }}             		The following INSERT IGNORE emits an error instead of a warning:

                DROP TABLE IF EXISTS `my_table`;
                
                CREATE TABLE `my_table` (
                    `first_col` char(63),
                    `second_col` char(63),
                    constraint `my_const` check (`first_col` != `second_col`)
                );
                
                INSERT IGNORE INTO `my_table` VALUES ('a', 'a');
            jkoop Joe Koop made changes -
            Description The following INSERT IGNORE emits an error instead of a warning:

                DROP TABLE IF EXISTS `my_table`;
                
                CREATE TABLE `my_table` (
                    `first_col` char(63),
                    `second_col` char(63),
                    constraint `my_const` check (`first_col` != `second_col`)
                );
                
                INSERT IGNORE INTO `my_table` VALUES ('a', 'a');             		The following {{INSERT IGNORE}} emits an error instead of a warning:

            {quote}
            DROP TABLE IF EXISTS `my_table`;

            CREATE TABLE `my_table` (
                `first_col` char(63),
                `second_col` char(63),
                constraint `my_const` check (`first_col` != `second_col`)
            );

            INSERT IGNORE INTO `my_table` VALUES ('a', 'a');
            {quote}
            danblack Daniel Black added a comment - - edited

            I checked some earlier versions https://dbfiddle.uk/H9aZj6Y9 and it seems it was always causing an error, so I don't think its a regression.

            It seems the KB document https://mariadb.com/kb/en/insert-ignore/ inconsistent as looking at documentation this only applies to this list of errors is ignored.

            error generated

            		 
            ERROR 4025 (23000): CONSTRAINT `my_const` failed for `test`.`my_table`

            It does seem rather inconsistent to not insert on a FK constraint, but error on a local constraint.

            I tested a fix:

            diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc
            		 
            index d5f9ba68ccd..df319c4fb68 100644
            		 
            --- a/sql/sql_insert.cc
            		 
            +++ b/sql/sql_insert.cc
            		 
            @@ -1129,10 +1129,7 @@ bool mysql_insert(THD *thd, TABLE_LIST *table_list,
            		 
                     }
            		 
                   }
            		 
             
            -      if ((res= table_list->view_check_option(thd,
            		 
            -                                              (values_list.elements == 1 ?
            		 
            -                                               0 :
            		 
            -                                               ignore))) ==
            		 
            +      if ((res= table_list->view_check_option(thd, ignore)) ==
            		 
                       VIEW_CHECK_SKIP)
            		 
                     continue;
            		 
                   else if (res == VIEW_CHECK_ERROR)

            The reason for this check was added in 2aad30394d33eef23545d717f4b92eb280c2b19d around views with WITH CHECK OPTION.

            mysql_load and mysql_updates (from LOAD DATA and UPDATE) just pass with lex.ignore to view_check_option.

            Sanja, why is the insert behaviour different?

            Why does it error rather than warn if one value provided, by two will ignore?

            danblack Daniel Black added a comment - - edited I checked some earlier versions https://dbfiddle.uk/H9aZj6Y9 and it seems it was always causing an error, so I don't think its a regression. It seems the KB document https://mariadb.com/kb/en/insert-ignore/ inconsistent as looking at documentation this only applies to this list of errors is ignored. error generated ERROR 4025 (23000): CONSTRAINT `my_const` failed for `test`.`my_table` It does seem rather inconsistent to not insert on a FK constraint, but error on a local constraint. I tested a fix: diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc index d5f9ba68ccd..df319c4fb68 100644 --- a/sql/sql_insert.cc +++ b/sql/sql_insert.cc @@ -1129,10 +1129,7 @@ bool mysql_insert(THD *thd, TABLE_LIST *table_list, } } - if ((res= table_list->view_check_option(thd, - (values_list.elements == 1 ? - 0 : - ignore ))) == + if ((res= table_list->view_check_option(thd, ignore )) == VIEW_CHECK_SKIP) continue ; else if (res == VIEW_CHECK_ERROR) The reason for this check was added in 2aad30394d33eef23545d717f4b92eb280c2b19d around views with WITH CHECK OPTION . mysql_load and mysql_updates (from LOAD DATA and UPDATE) just pass with lex.ignore to view_check_option. Sanja, why is the insert behaviour different? Why does it error rather than warn if one value provided, by two will ignore?
            danblack Daniel Black made changes -
            Assignee Oleksandr Byelkin [ sanja ]
            danblack Daniel Black made changes -
            Labels regression
            danblack Daniel Black made changes -
            Summary The IGNORE in INSERT IGNORE is ignored in v10.11.4 INSERT IGNORE on constraints result in ERROR rather than warning
            danblack Daniel Black made changes -
            Affects Version/s 10.4.31 [ 29010 ]
            marko Marko Mäkelä added a comment -

            My understanding is that INSERT IGNORE would always enforce CONSTRAINT…FOREIGN KEY (unless they were separately disabled by SET foreign_key_checks=0). Based on this, it could have made sense to not ignore any CONSTRAINT…CHECK violations either.

            I suppose that you could implement ignorable CHECK constraints by rewriting them as virtual column expressions. If I remember correctly, INSERT IGNORE would not only ignore unique key violations (omit one of the duplicate rows) but also ignore errors in evaluating non-indexed virtual columns.

            We may have some similar confusion around ALTER IGNORE TABLE.

            marko Marko Mäkelä added a comment - My understanding is that INSERT IGNORE would always enforce CONSTRAINT…FOREIGN KEY (unless they were separately disabled by SET foreign_key_checks=0 ). Based on this, it could have made sense to not ignore any CONSTRAINT…CHECK violations either. I suppose that you could implement ignorable CHECK constraints by rewriting them as virtual column expressions. If I remember correctly, INSERT IGNORE would not only ignore unique key violations (omit one of the duplicate rows) but also ignore errors in evaluating non-indexed virtual columns. We may have some similar confusion around ALTER IGNORE TABLE .
            danblack Daniel Black added a comment -

            So the issue as to why its an error based on testing and its really obvious by the above code change, is that a single row of values will error, more than one row will warn.

            https://dbfiddle.uk/9BPpIaew

            I asked monty about this and this was because a non-transactional engine could abort before it did anything one row, but more than one it had to continue. Not the most convincing of arguments, but has been this way, and now that you've reported it could be fixed.

            This special rule for INSERT IGNORE VALUES doesn't apply to UPDATE IGNORE or LOAD DATA IGNORE.

            marko FK checks - https://dbfiddle.uk/LuMuGDEg are ok, as the IGNORE is preventing a FK violating row from being inserted which is different from the disable checks and just insert it. Interesting enough, this is a warning.

            So for consistency, lets fix the limited special case and just do warnings always.

            danblack Daniel Black added a comment - So the issue as to why its an error based on testing and its really obvious by the above code change, is that a single row of values will error, more than one row will warn. https://dbfiddle.uk/9BPpIaew I asked monty about this and this was because a non-transactional engine could abort before it did anything one row, but more than one it had to continue. Not the most convincing of arguments, but has been this way, and now that you've reported it could be fixed. This special rule for INSERT IGNORE VALUES doesn't apply to UPDATE IGNORE or LOAD DATA IGNORE . marko FK checks - https://dbfiddle.uk/LuMuGDEg are ok, as the IGNORE is preventing a FK violating row from being inserted which is different from the disable checks and just insert it. Interesting enough, this is a warning. So for consistency, lets fix the limited special case and just do warnings always.
            danblack Daniel Black made changes -
            Assignee Oleksandr Byelkin [ sanja ] Daniel Black [ danblack ]
            danblack Daniel Black made changes -
            Fix Version/s 10.4 [ 22408 ]
            danblack Daniel Black made changes -
            Status Open [ 1 ] In Progress [ 3 ]
            danblack Daniel Black added a comment -

            for review: https://github.com/MariaDB/server/pull/2783

            danblack Daniel Black added a comment - for review: https://github.com/MariaDB/server/pull/2783
            danblack Daniel Black made changes -
            Status In Progress [ 3 ] In Review [ 10002 ]
            danblack Daniel Black made changes -
            Labels compat80
            serg Sergei Golubchik made changes -
            Assignee Daniel Black [ danblack ] Michael Widenius [ monty ]
            serg Sergei Golubchik made changes -
            Fix Version/s 10.5 [ 23123 ]
            Fix Version/s 10.6 [ 24028 ]
            Fix Version/s 10.10 [ 27530 ]
            Fix Version/s 10.11 [ 27614 ]
            Fix Version/s 11.0 [ 28320 ]
            Fix Version/s 11.1 [ 28549 ]
            Fix Version/s 11.2 [ 28603 ]
            danblack Daniel Black added a comment -

            So while this was intended behaviour at some point:

            1. Generating an error violates what IGNORE is meant to do.
            2. People who use IGNORE don't expect it to error
            3. This behaviour isn't documented
            4. MySQL doesn't generate an error
            5. The error depends on the insert mechanism, so is rather an odd case.

            If there's a MyISAM/Aria case that needs to be fixed, lets do that in the actual engine rather than the SQL layer.

            danblack Daniel Black added a comment - So while this was intended behaviour at some point: Generating an error violates what IGNORE is meant to do. People who use IGNORE don't expect it to error This behaviour isn't documented MySQL doesn't generate an error The error depends on the insert mechanism, so is rather an odd case. If there's a MyISAM/Aria case that needs to be fixed, lets do that in the actual engine rather than the SQL layer.
            danblack Daniel Black made changes -
            Assignee Michael Widenius [ monty ] Daniel Black [ danblack ]
            danblack Daniel Black made changes -
            Fix Version/s 11.4 [ 29301 ]
            Fix Version/s 10.4 [ 22408 ]
            Fix Version/s 10.5 [ 23123 ]
            Fix Version/s 10.6 [ 24028 ]
            Fix Version/s 10.10 [ 27530 ]
            Fix Version/s 10.11 [ 27614 ]
            Fix Version/s 11.0 [ 28320 ]
            Fix Version/s 11.1 [ 28549 ]
            Fix Version/s 11.2 [ 28603 ]
            danblack Daniel Black added a comment -

            Review by monty said target for non-GA version.

            danblack Daniel Black added a comment - Review by monty said target for non-GA version.
            danblack Daniel Black made changes -
            Status In Review [ 10002 ] Stalled [ 10000 ]
            danblack Daniel Black made changes -
            issue.field.resolutiondate 2023-11-14 05:29:03.0 2023-11-14 05:29:02.801
            danblack Daniel Black made changes -
            Component/s Data Manipulation - Insert [ 10101 ]
            Fix Version/s 11.4.1 [ 29523 ]
            Fix Version/s 11.4 [ 29301 ]
            Resolution Fixed [ 1 ]
            Status Stalled [ 10000 ] Closed [ 6 ]
            marko Marko Mäkelä added a comment -

            To be consistent, should MDEV-31985 be fixed as well (without introducing a performance regression like MDEV-31835)?

            marko Marko Mäkelä added a comment - To be consistent, should MDEV-31985 be fixed as well (without introducing a performance regression like MDEV-31835 )?
            marko Marko Mäkelä made changes -

            People

              danblack Daniel Black
              jkoop Joe Koop
              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.