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

          Activity

            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().

            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.