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

ALTER .. ADD PARTITION uses wrong partition-level option values

Details

    • Bug
    • Status: Closed (View Workflow)
    • Critical
    • Resolution: Fixed
    • 10.8(EOL)
    • 10.8.3
    • Partitioning
    • None

    Description

      When a partition with partition-level options is added by ALTER, it shows the correct definition in SHOW CREATE, but the actual values get inherited from the previous partition.
      At least that's my guess of what's happening with the flag, see details below.
      MTR test case is at the end of the description.

      The test will be using PAGE_COMPRESSED as an example. I have no information whether the problem affects other options.

      Since I don't have the flag decoder handy, let's create a dictionary first.

      drop database if exists db;
      create database db;
      use db;
      create table t_not_compressed (a int) engine=InnoDB page_compressed=0;
      # Default level is 6
      create table t_compressed (a int) engine=InnoDB page_compressed=1;
      create table t_compressed_level_3 (a int) engine=InnoDB page_compressed=1 page_compression_level=3;
       
      select name, flag from information_schema.innodb_sys_tablespaces where name like 'db%';
      

      +-------------------------+------------+
      | name                    | flag       |
      +-------------------------+------------+
      | db/t_not_compressed     |         21 |
      | db/t_compressed         | 1610612789 |
      | db/t_compressed_level_3 |  805306421 |
      +-------------------------+------------+
      3 rows in set (0.001 sec)
      

      So, these are the expected flag values.
      Let's check that they are the same for partitions, just in case.

      create table t_parts (a int) engine=InnoDB partition by hash(a) (
        partition p_not_compressed page_compressed=0,
        partition p_compressed page_compressed=1,
        partition p_compressed_level_3 page_compressed=1 page_compression_level=3
      );
      select name, flag from information_schema.innodb_sys_tablespaces where name like 'db/t_parts%';
      

      +-----------------------------------+------------+
      | name                              | flag       |
      +-----------------------------------+------------+
      | db/t_parts#P#p_not_compressed     |         21 |
      | db/t_parts#P#p_compressed         | 1610612789 |
      | db/t_parts#P#p_compressed_level_3 |  805306421 |
      +-----------------------------------+------------+
      

      So far so good.

      Now let's create the same structure but in steps.

      create table t_alter (a int) engine=InnoDB partition by hash(a) (partition p_not_compressed page_compressed=0);
      select name, flag from information_schema.innodb_sys_tablespaces where name like 'db/t_alter%';
       
      alter table t_alter add partition (partition p_compressed page_compressed=1);
      select name, flag from information_schema.innodb_sys_tablespaces where name like 'db/t_alter%';
       
      alter table t_alter add partition (partition p_compressed_level_3 page_compressed=1 page_compression_level=3);
      select name, flag from information_schema.innodb_sys_tablespaces where name like 'db/t_alter%';
      

      10.8 050508672

      MariaDB [db]> select name, flag from information_schema.innodb_sys_tablespaces where name like 'db/t_alter%';
      +-------------------------------+------+
      | name                          | flag |
      +-------------------------------+------+
      | db/t_alter#P#p_not_compressed |   21 |
      +-------------------------------+------+
      1 row in set (0.001 sec)
      

      MariaDB [db]> alter table t_alter add partition (partition p_compressed page_compressed=1);
      Query OK, 0 rows affected (0.111 sec)
      Records: 0  Duplicates: 0  Warnings: 0
       
      MariaDB [db]> select name, flag from information_schema.innodb_sys_tablespaces where name like 'db/t_alter%';
      +-------------------------------+------+
      | name                          | flag |
      +-------------------------------+------+
      | db/t_alter#P#p_not_compressed |   21 |
      | db/t_alter#P#p_compressed     |   21 |
      +-------------------------------+------+
      2 rows in set (0.001 sec)
      

      MariaDB [db]> alter table t_alter add partition (partition p_compressed_level_3 page_compressed=1 page_compression_level=3);
      Query OK, 0 rows affected (0.269 sec)
      Records: 0  Duplicates: 0  Warnings: 0
       
      MariaDB [db]> select name, flag from information_schema.innodb_sys_tablespaces where name like 'db/t_alter%';
      +-----------------------------------+------------+
      | name                              | flag       |
      +-----------------------------------+------------+
      | db/t_alter#P#p_not_compressed     |         21 |
      | db/t_alter#P#p_compressed         | 1610612789 |
      | db/t_alter#P#p_compressed_level_3 | 1610612789 |
      +-----------------------------------+------------+
      3 rows in set (0.001 sec)
      

      Not so good.
      The initial partition was created all right.
      The "compressed" partition is actually added with the non-compressed flag – that is, the flag from the previous partition.

      When the "compressed-level-3" partition is added, the previous "compressed" partition is recalculated/rebuilt to use its correct flag, but the new one is added with a wrong one – again, with the one from the previous partition.

      SHOW CREATE TABLE shows the intended values though.

      MariaDB [db]> show create table t_alter \G
      *************************** 1. row ***************************
             Table: t_alter
      Create Table: CREATE TABLE `t_alter` (
        `a` int(11) DEFAULT NULL
      ) ENGINE=InnoDB DEFAULT CHARSET=latin1
       PARTITION BY HASH (`a`)
      (PARTITION `p_not_compressed` ENGINE = InnoDB page_compressed = 0,
       PARTITION `p_compressed` ENGINE = InnoDB page_compressed = 1,
       PARTITION `p_compressed_level_3` ENGINE = InnoDB page_compressed = 1 page_compression_level = 3)
      1 row in set (0.000 sec)
      

      MTR

      --source include/have_innodb.inc
      --source include/have_partition.inc
       
      --let $restart_parameters= --innodb-sys-tablespaces
      --source include/restart_mysqld.inc 
       
      create table t (a int) engine=InnoDB partition by hash(a) (partition p_not_compressed page_compressed=0);
      alter table t add partition (partition p_compressed page_compressed=1);
      select name, flag from information_schema.innodb_sys_tablespaces where name like 'test/t%';
       
      # Cleanup
      drop table t;
      

      Attachments

        Issue Links

          Activity

            I confirmed the issue. Thank you very much for your detailed description. elenst

            nayuta-yanagisawa Nayuta Yanagisawa (Inactive) added a comment - - edited I confirmed the issue. Thank you very much for your detailed description. elenst

            From the viewpoint of Spider, this is a serious bug because this could result in data corruption. Thus, I raised the priority.

            nayuta-yanagisawa Nayuta Yanagisawa (Inactive) added a comment - From the viewpoint of Spider, this is a serious bug because this could result in data corruption. Thus, I raised the priority.

            parse_engine_part_options() gets partition options from TABLE_SHARE. In the case of ALTER TABLEs, TABLE_SHARE seems to hold the partition information which is based on the table schema before ALTERs. This seems to be (at least part of) the cause of the bug.

            nayuta-yanagisawa Nayuta Yanagisawa (Inactive) added a comment - parse_engine_part_options() gets partition options from TABLE_SHARE. In the case of ALTER TABLEs, TABLE_SHARE seems to hold the partition information which is based on the table schema before ALTERs. This seems to be (at least part of) the cause of the bug.

            The complete part_info, which represents the partition structure after ALTER, is built by prep_alter_part_table() and is set to thd->work_part_info. So, we need to parse thd->work_part_info after the call of the function.

            nayuta-yanagisawa Nayuta Yanagisawa (Inactive) added a comment - - edited The complete part_info, which represents the partition structure after ALTER, is built by prep_alter_part_table() and is set to thd->work_part_info . So, we need to parse thd->work_part_info after the call of the function.

            It seems to be good to call parse_engine_part_options() in partition_info::fix_parser_data() according to the comments on the function.

            /**
              Fix partition data from parser.
             
              @details The parser generates generic data structures, we need to set them
              up as the rest of the code expects to find them. This is in reality part
              of the syntax check of the parser code.
             
              It is necessary to call this function in the case of a CREATE TABLE
              statement, in this case we do it early in the check_partition_info
              function.
             
              It is necessary to call this function for ALTER TABLE where we
              assign a completely new partition structure, in this case we do it
              in prep_alter_part_table after discovering that the partition
              structure is entirely redefined.
             
              It's necessary to call this method also for ALTER TABLE ADD/REORGANIZE
              of partitions, in this we call it in prep_alter_part_table after
              making some initial checks but before going deep to check the partition
              info, we also assign the column_list variable before calling this function
              here.
             
              Finally we also call it immediately after returning from parsing the
              partitioning text found in the frm file.
             
              This function mainly fixes the VALUES parts, these are handled differently
              whether or not we use column list partitioning. Since the parser doesn't
              know which we are using we need to set-up the old data structures after
              the parser is complete when we know if what type of partitioning the
              base table is using.
             
              For column lists we will handle this in the fix_column_value_function.
              For column lists it is sufficient to verify that the number of columns
              and number of elements are in synch with each other. So only partitioning
              using functions need to be set-up to their data structures.
             
              @param thd  Thread object
             
              @return Operation status
                @retval TRUE   Failure
                @retval FALSE  Success
            */
             
            bool partition_info::fix_parser_data(THD *thd)
            {
                ...
            }
            

            nayuta-yanagisawa Nayuta Yanagisawa (Inactive) added a comment - It seems to be good to call parse_engine_part_options() in partition_info::fix_parser_data() according to the comments on the function. /** Fix partition data from parser.   @details The parser generates generic data structures, we need to set them up as the rest of the code expects to find them. This is in reality part of the syntax check of the parser code.   It is necessary to call this function in the case of a CREATE TABLE statement, in this case we do it early in the check_partition_info function.   It is necessary to call this function for ALTER TABLE where we assign a completely new partition structure, in this case we do it in prep_alter_part_table after discovering that the partition structure is entirely redefined.   It's necessary to call this method also for ALTER TABLE ADD/REORGANIZE of partitions, in this we call it in prep_alter_part_table after making some initial checks but before going deep to check the partition info, we also assign the column_list variable before calling this function here.   Finally we also call it immediately after returning from parsing the partitioning text found in the frm file.   This function mainly fixes the VALUES parts, these are handled differently whether or not we use column list partitioning. Since the parser doesn't know which we are using we need to set-up the old data structures after the parser is complete when we know if what type of partitioning the base table is using.   For column lists we will handle this in the fix_column_value_function. For column lists it is sufficient to verify that the number of columns and number of elements are in synch with each other. So only partitioning using functions need to be set-up to their data structures.   @param thd Thread object   @return Operation status @retval TRUE Failure @retval FALSE Success */   bool partition_info::fix_parser_data(THD *thd) { ... }

            An important point that I was not really aware of is that partition options are only used for storage engines. So, we need to parse the options only when storage engines can access to the options.

            nayuta-yanagisawa Nayuta Yanagisawa (Inactive) added a comment - An important point that I was not really aware of is that partition options are only used for storage engines. So, we need to parse the options only when storage engines can access to the options.
            nayuta-yanagisawa Nayuta Yanagisawa (Inactive) added a comment - holyfoot please review: https://github.com/MariaDB/server/commit/001f54effdb535d468e4a7062e97e155c0915c09

            ok to push

            holyfoot Alexey Botchkov added a comment - ok to push

            People

              nayuta-yanagisawa Nayuta Yanagisawa (Inactive)
              elenst Elena Stepanova
              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.