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

mariadb-check does not return warning for incorrect sequence with engine InnoDB

Details

    Description

      mariadb-check does not return warning for incorrect sequence (has two recods) with engine InnoDB

      Create table with engine InnoDB and convert it into sequence:

      CREATE TABLE `s1` (
        `next_not_cached_value` bigint(21) NOT NULL,
        `minimum_value` bigint(21) NOT NULL,
        `maximum_value` bigint(21) NOT NULL,
        `start_value` bigint(21) NOT NULL COMMENT 'start value when sequences is created or value if RESTART is used',
        `increment` bigint(21) NOT NULL COMMENT 'increment value',
        `cache_size` bigint(21) unsigned NOT NULL,
        `cycle_option` tinyint(1) unsigned NOT NULL COMMENT '0 if no cycles are allowed, 1 if the sequence should begin a new cycle when maximum_value is passed',
        `cycle_count` bigint(21) NOT NULL COMMENT 'How many cycles have been done'
      ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_uca1400_ai_ci;
       
      insert into s1 values (1,1,9223372036854775806,1,1,1000,0,0);
      insert into s1 values (2,1,9223372036854775806,1,1,1000,0,0);
       
      alter table s1 sequence=1;
      

      MariaDB [test]> select * from s1;
      +-----------------------+---------------+---------------------+-------------+-----------+------------+--------------+-------------+
      | next_not_cached_value | minimum_value | maximum_value       | start_value | increment | cache_size | cycle_option | cycle_count |
      +-----------------------+---------------+---------------------+-------------+-----------+------------+--------------+-------------+
      |                     1 |             1 | 9223372036854775806 |           1 |         1 |       1000 |            0 |           0 |
      +-----------------------+---------------+---------------------+-------------+-----------+------------+--------------+-------------+
      1 row in set (0,002 sec)
      

      mariadb-check does not show any warnings

      mariadb-check --databases test 
      test.s1                                            OK
      

      If you change the type from sequence to table, you can see that the table still has two records:

      MariaDB [test]> alter table s1 sequence=0;
      Query OK, 0 rows affected (0,003 sec)
      Records: 0  Duplicates: 0  Warnings: 0
       
      MariaDB [test]> select * from s1;
      +-----------------------+---------------+---------------------+-------------+-----------+------------+--------------+-------------+
      | next_not_cached_value | minimum_value | maximum_value       | start_value | increment | cache_size | cycle_option | cycle_count |
      +-----------------------+---------------+---------------------+-------------+-----------+------------+--------------+-------------+
      |                     1 |             1 | 9223372036854775806 |           1 |         1 |       1000 |            0 |           0 |
      |                     2 |             1 | 9223372036854775806 |           1 |         1 |       1000 |            0 |           0 |
      +-----------------------+---------------+---------------------+-------------+-----------+------------+--------------+-------------+
      2 rows in set (0,003 sec)
      

      In a similar situation for Aria and MyISAM warning is shown:

      MariaDB [test]> select TABLE_NAME, TABLE_TYPE , ENGINE  from information_schema.tables where table_schema='test';
      +------------+------------+--------+
      | TABLE_NAME | TABLE_TYPE | ENGINE |
      +------------+------------+--------+
      | s1         | SEQUENCE   | InnoDB |
      | s2         | SEQUENCE   | MyISAM |
      +------------+------------+--------+
      

      mariadb-check --databases test 
      test.s1                                            OK
      test.s2
      Warning  : More than one row in the table
      status   : OK
      

      Attachments

        Issue Links

          Activity

            Like I wrote on 2025-01-21, a valid InnoDB sequence must consist of a single record in a single index page, with no transaction metadata. If it is anything else, then various bad things can happen, as it has already been demonstrated in MDEV-36032. Because that level of detail is not visible across the storage engine handler interface, the checks will need to be implemented at the InnoDB level.

            Thorough stress testing should find problems related to the scenarios that I have in mind. But, it does not seem to be feasible to conduct thorough testing before all currently known ways to create corrupted sequences (MDEV-36038 and MDEV-36032) have been fixed.

            marko Marko Mäkelä added a comment - Like I wrote on 2025-01-21, a valid InnoDB sequence must consist of a single record in a single index page, with no transaction metadata. If it is anything else, then various bad things can happen, as it has already been demonstrated in MDEV-36032 . Because that level of detail is not visible across the storage engine handler interface, the checks will need to be implemented at the InnoDB level. Thorough stress testing should find problems related to the scenarios that I have in mind. But, it does not seem to be feasible to conduct thorough testing before all currently known ways to create corrupted sequences ( MDEV-36038 and MDEV-36032 ) have been fixed.
            ycp Yuchen Pei added a comment -

            Sorry, I need to rework this.

            The patch I sent for review no longer works when rebased on the MDEV-36038 patch - see the regression testcase in my comment in that issue. The proposed fix MDEV-36032 causes the only way to create sequence with multiple rows, i.e. with ALTER TABLE ... SEQUENCE=1, to fail. So it will no longer be possible to test the CHECK coverage on this. In fact there will be no need to CHECK number of rows. Therefore a MDEV-36032 fix will make the present issue obsolete. I have a wip draft patch for that issue - will post there.

            ycp Yuchen Pei added a comment - Sorry, I need to rework this. The patch I sent for review no longer works when rebased on the MDEV-36038 patch - see the regression testcase in my comment in that issue. The proposed fix MDEV-36032 causes the only way to create sequence with multiple rows, i.e. with ALTER TABLE ... SEQUENCE=1, to fail. So it will no longer be possible to test the CHECK coverage on this. In fact there will be no need to CHECK number of rows. Therefore a MDEV-36032 fix will make the present issue obsolete. I have a wip draft patch for that issue - will post there.
            ycp Yuchen Pei added a comment - - edited

            I could construct testcases, the same way I did as in the comment https://jira.mariadb.org/browse/MDEV-36032?focusedCommentId=303346&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-303346. So I have updated the patch which is ready for review. Note however that changes in MDEV-36487 may affect the test coverage here, as the check method of the underlying engine is called before the more generic checks implemented here.

            Hi sanja, ptal thanks (based on MDEV-36032 commits):

            de9c76fd570 upstream/bb-12.0-mdev-35866 MDEV-35866 CHECK TABLE get number of rows without HA_STATS_RECORDS_IS_EXACT
            

            ycp Yuchen Pei added a comment - - edited I could construct testcases, the same way I did as in the comment https://jira.mariadb.org/browse/MDEV-36032?focusedCommentId=303346&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-303346 . So I have updated the patch which is ready for review. Note however that changes in MDEV-36487 may affect the test coverage here, as the check method of the underlying engine is called before the more generic checks implemented here. Hi sanja , ptal thanks (based on MDEV-36032 commits): de9c76fd570 upstream/bb-12.0-mdev-35866 MDEV-35866 CHECK TABLE get number of rows without HA_STATS_RECORDS_IS_EXACT
            sanja Oleksandr Byelkin added a comment -

            Looks good but use ha_rnd_init_with_error() instead of ha_rnd_init, you will not need error printing code in this case, it looks like operation is so frequent so we have such function. after fixing it is ok to push.

            sanja Oleksandr Byelkin added a comment - Looks good but use ha_rnd_init_with_error() instead of ha_rnd_init, you will not need error printing code in this case, it looks like operation is so frequent so we have such function. after fixing it is ok to push.
            ycp Yuchen Pei added a comment - - edited

            Thanks for the review. I have pushed to amended commit as bb-12.0-mdev-35866 e2eec78973d523098a71f9d9cb9740f5fd10bd29.

            sanja: we can't "push" this one yet - what remains is MDEV-36032. Once that is reviewed and approved, together with this one MDEV-22491 will be unblocked.

            ycp Yuchen Pei added a comment - - edited Thanks for the review. I have pushed to amended commit as bb-12.0-mdev-35866 e2eec78973d523098a71f9d9cb9740f5fd10bd29. sanja : we can't "push" this one yet - what remains is MDEV-36032 . Once that is reviewed and approved, together with this one MDEV-22491 will be unblocked.

            People

              ycp Yuchen Pei
              lstartseva Lena Startseva
              Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

                Created:
                Updated:

                Git Integration

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