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

ALTER TABLE…DROP PRIMARY KEY, ADD PRIMARY KEY fails when VIRTUAL columns exist

Details

    Description

      I got a suspicion that there might be something wrong with DROP PRIMARY KEY, ADD PRIMARY KEY with ROW_FORMAT=REDUNDANT tables.
      The reason for this is the call in row_log_table_low_redundant():

      		old_pk_size = rec_get_converted_size_temp(
      		 
      			new_index, old_pk->fields, old_pk->n_fields,
      		 
      			ventry, &old_pk_extra_size);

      The logic is duplicated differently in row_log_table_low(), where ventry is replaced with NULL:

      		old_pk_size = rec_get_converted_size_temp(
      		 
      			new_index, old_pk->fields, old_pk->n_fields,
      		 
      			NULL, &old_pk_extra_size);

      This turns out to be a bigger issue. I can repeat corruption and a crash with a small modification to an existing test:

      diff --git a/mysql-test/suite/innodb/t/innodb-table-online.test b/mysql-test/suite/innodb/t/innodb-table-online.test
      		 
      index a1977fa06a4..3ed206d0f44 100644
      		 
      --- a/mysql-test/suite/innodb/t/innodb-table-online.test
      		 
      +++ b/mysql-test/suite/innodb/t/innodb-table-online.test
      		 
      @@ -241,6 +241,12 @@ reap;
      		 
       # when the above error was noticed.
      		 
       eval $innodb_metrics_select;
      		 
       
      +SET @saved_max = @@GLOBAL.innodb_online_alter_log_max_size;
      		 
      +SET GLOBAL innodb_online_alter_log_max_size = 131072;
      		 
      +ALTER TABLE t1 ROW_FORMAT=REDUNDANT;
      		 
      +ALTER TABLE t1 ADD COLUMN v1 INT AS (c1) VIRTUAL, LOCK=NONE;
      		 
      +#ALTER TABLE t1 ADD INDEX(v1), LOCK=NONE;
      		 
      +
      		 
       # Accumulate and apply some modification log.
      		 
       SET DEBUG_SYNC = 'row_log_table_apply1_before SIGNAL rebuilt3 WAIT_FOR dml3_done';
      		 
       --error ER_MULTIPLE_PRI_KEY
      		 
      @@ -258,7 +264,7 @@ SET DEBUG_SYNC = 'now WAIT_FOR rebuilt3';
      		 
       # Generate some log (delete-mark, delete-unmark, insert etc.)
      		 
       eval $innodb_metrics_select;
      		 
       BEGIN;
      		 
      -INSERT INTO t1 SELECT 320 + c1, c2, c3 FROM t1 WHERE c1 > 240;
      		 
      +INSERT INTO t1 (c1,c2,c3) SELECT 320 + c1, c2, c3 FROM t1 WHERE c1 > 240;
      		 
       DELETE FROM t1 WHERE c1 > 320;
      		 
       ROLLBACK;
      		 
       BEGIN;
      		 
      @@ -273,8 +279,11 @@ SET DEBUG_SYNC = 'now SIGNAL dml3_done';
      		 
       connection con1;
      		 
       reap;
      		 
       eval $innodb_metrics_select;
      		 
      +SET GLOBAL innodb_online_alter_log_max_size = @saved_max;
      		 
      +
      		 
       SELECT COUNT(c22f) FROM t1;
      		 
       CHECK TABLE t1;
      		 
      +ALTER TABLE t1 DROP COLUMN v1;
      		 
       
       # Create a column prefix index.
      		 
       --error ER_DUP_ENTRY

      I did request this kind of tests when I reviewed WL#8149 while I was employed at Oracle.

      In the above patch, there are a couple of ‘parameters’:

      1. the ROW_FORMAT of the table (ROW_FORMAT=REDUNDANT is logged differently)
      2. whether an index exists on the virtual column

      Actually, virtual columns should not matter at all for the logging of table-rebuilding ALTER TABLE operations. No matter if the columns are indexed, all that we need the values of the PRIMARY KEY and the base columns.

      Creating secondary indexes is a different matter. There currently exist unnecessary limitations around that (for example, adding a virtual column and adding an index is not supported in a single ALTER TABLE statement).

      Attachments

        Issue Links

          blocks

          Bug - A problem which impairs or prevents the functions of the product. MDEV-31087 Merge new release of InnoDB 5.7.42 to 10.3

          • Blocker - Blocks development and/or testing work, production could not run.
          • Closed
          causes

          Bug - A problem which impairs or prevents the functions of the product. MDEV-14341 Allow LOCK=NONE in table-rebuilding ALTER when indexed virtual columns exist

          • Major - Major loss of function.
          • Open
          is part of

          Bug - A problem which impairs or prevents the functions of the product. MDEV-14332 Corruption during online table-rebuilding ALTER when VIRTUAL columns exist, causes assertion failure

          • Major - Major loss of function.
          • Closed
          relates to

          Task - A task that needs to be done. MDEV-11371 Big column compressed

          • Major - Major loss of function.
          • Closed

          Bug - A problem which impairs or prevents the functions of the product. MDEV-13359 Enable online ALTER TABLE for compressed columns

          • Major - Major loss of function.
          • Closed

          Task - A task that needs to be done. MDEV-16329 Engine-independent online ALTER TABLE

          • Blocker - Blocks development and/or testing work, production could not run.
          • Closed

          Task - A task that needs to be done. MDEV-5800 indexes on virtual (not materialized) columns

          • Blocker - Blocks development and/or testing work, production could not run.
          • Closed

          Task - A task that needs to be done. MDEV-13625 Merge InnoDB test cases from MySQL 5.6

          • Major - Major loss of function.
          • Closed

          Bug - A problem which impairs or prevents the functions of the product. MDEV-14332 Corruption during online table-rebuilding ALTER when VIRTUAL columns exist, causes assertion failure

          • Major - Major loss of function.
          • Closed

          Activity

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

            Thinking deeper, this may need more effort to fix properly.
            It is true that the values of virtual columns are redundant information for a table-rebuilding online ALTER when no indexes on virtual columns exist. And in this case, we should not expect a crash like this:

            mysqld: /mariadb/10.2/storage/innobase/trx/trx0rec.cc:2426: void trx_undo_read_v_cols(const dict_table_t *, const byte *, const dtuple_t *, bool, const ulint *): Assertion `ptr == end_ptr' failed.

            But, if there exist indexed virtual columns after the ALTER TABLE, the values of those columns must be available to row_log_table_apply(), either by evaluating the virtual column values (which could collide with MDEV-11371 compressed columns; see MDEV-13359) or by retrieving the values from the online_log. Currently, if the ADD INDEX in the above test patch is enabled, the test will fail in a more graceful way:

            CURRENT_TEST: innodb.innodb-table-online
            		 
            mysqltest: At line 280: query 'reap' failed: 1712: Index PRIMARY is corrupted

            I remember another problematic scenario in MySQL 5.7 that was filed as an Oracle internal bug and probably remains open. It was something like ALTER TABLE…ADD INDEX(virtual_column) concurrently with an UPDATE that would affect the value of the virtual_column. Because the column was not indexed at the time when the UPDATE was logged, a garbage value of virtual_column would have been provided to row_log_online_op().

            marko Marko Mäkelä added a comment - Thinking deeper, this may need more effort to fix properly. It is true that the values of virtual columns are redundant information for a table-rebuilding online ALTER when no indexes on virtual columns exist. And in this case, we should not expect a crash like this: mysqld: /mariadb/10.2/storage/innobase/trx/trx0rec.cc:2426: void trx_undo_read_v_cols(const dict_table_t *, const byte *, const dtuple_t *, bool, const ulint *): Assertion `ptr == end_ptr' failed. But, if there exist indexed virtual columns after the ALTER TABLE, the values of those columns must be available to row_log_table_apply(), either by evaluating the virtual column values (which could collide with MDEV-11371 compressed columns; see MDEV-13359 ) or by retrieving the values from the online_log. Currently, if the ADD INDEX in the above test patch is enabled, the test will fail in a more graceful way: CURRENT_TEST: innodb.innodb-table-online mysqltest: At line 280: query 'reap' failed: 1712: Index PRIMARY is corrupted I remember another problematic scenario in MySQL 5.7 that was filed as an Oracle internal bug and probably remains open. It was something like ALTER TABLE…ADD INDEX(virtual_column) concurrently with an UPDATE that would affect the value of the virtual_column. Because the column was not indexed at the time when the UPDATE was logged, a garbage value of virtual_column would have been provided to row_log_online_op().
            marko Marko Mäkelä added a comment -

            One more crash, indicating `online_log` corruption:

            mysqld: /mariadb/10.2/storage/innobase/row/row0log.cc:2393: const mrec_t *row_log_table_apply_op(que_thr_t *, ulint, ulint, row_merge_dup_t *, dberr_t *, mem_heap_t *, mem_heap_t *, const mrec_t *, const mrec_t *, ulint *): Assertion `0' failed.
            		 
            170913 15:20:54 [ERROR] mysqld got signal 6 ;


            	switch (*mrec++) {
            		 
            	default:
            		 
            		ut_ad(0);
            		 
            		*error = DB_CORRUPTION;
            		 
            		return(NULL);

            marko Marko Mäkelä added a comment - One more crash, indicating `online_log` corruption: mysqld: /mariadb/10.2/storage/innobase/row/row0log.cc:2393: const mrec_t *row_log_table_apply_op(que_thr_t *, ulint, ulint, row_merge_dup_t *, dberr_t *, mem_heap_t *, mem_heap_t *, const mrec_t *, const mrec_t *, ulint *): Assertion `0' failed. 170913 15:20:54 [ERROR] mysqld got signal 6 ; switch (*mrec++) { default : ut_ad(0); *error = DB_CORRUPTION; return (NULL);

            People

              marko Marko Mäkelä
              marko Marko Mäkelä
              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.