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

Avoid table rebuild on operations on generated columns

Details

    Description

      When MySQL 5.7 introduced indexed virtual columns in InnoDB, it added many limitations around ALTER TABLE…ALGORITHM=INPLACE. One of them is that altering or adding virtual columns is not supported when the same ALTER TABLE statement is also adding, dropping or reordering stored columns. MariaDB 10.2 inherited these limitations in MDEV-5800 and related work.

      In MDEV-15562 the right thing would be to allow any combination of ADD/DROP/reorder of virtual and stored columns. (Only the ADD of virtual persistent aka generated stored columns cannot possibly be instantaneous.)

      I ran into trouble with the following in the test gcol.innodb_virtual_basic, around line 541:

      CREATE TABLE t (a INT, b INT, c INT GENERATED ALWAYS AS(a+b), h VARCHAR(10), j INT, m INT  GENERATED ALWAYS AS(b + j), n VARCHAR(10), p VARCHAR(20) GENERATED ALWAYS AS(CONCAT(n, h)), INDEX idx1(c), INDEX idx2 (m), INDEX idx3(p));
       
      INSERT INTO t VALUES(11, 22, DEFAULT, "AAA", 8, DEFAULT, "XXX", DEFAULT);
      INSERT INTO t VALUES(1, 2, DEFAULT, "uuu", 9, DEFAULT, "uu", DEFAULT);
      INSERT INTO t VALUES(3, 4, DEFAULT, "uooo", 1, DEFAULT, "umm", DEFAULT);
       
      alter table t add  x int, add xx int generated ALWAYS AS(x);
       
      DROP TABLE t;
      

      The problem is that instant ADD of persistent columns (MDEV-11369) would modify the data dictionary cache already before commit, while adding virtual columns uses a different approach, deferring modifications of the table to the commit time.

      In this case, we should be able to avoid a table rebuild, and instantly add both columns, then generate the values for the stored column.

      Furthermore, there is the function check_v_col_in_order() whose sole purpose is to refuse operations that would change the order of virtual columns. This function should be removed and the code be fixed.

      Last but not least, the function ha_innobase::commit_inplace_alter_table() is evicting and reloading the table definition if any virtual columns were added or dropped. This code does not appear to work for partitioned tables (or it is only operating on the first partition or subpartition of the table). It is causing an expensive operation of emptying the adaptive hash index. The purpose of the exercise seems to be to invoke dict_load_column_low(), which is the only place where num_base for virtual columns is assigned to nonzero. It would be better to update this field directly in the data dictionary cache.

      Attachments

        Issue Links

          Activity

            To make the situation worse, in MDEV-15562 I had to disable instant ADD/DROP/reorder of stored columns if indexed virtual columns exist.
            The reason is that I occasionally got 2 distinct crashes for the following portion of the test gcol.innodb_virtual_basic:

            --source include/have_innodb.inc
            create table t (
              a int,b int,c text,d int,e int,f int,g int,
              h text generated always as ('1') virtual,
              i int,j int,k int,l int,m int,
              primary key (c(1)),unique key (c(1)),
              key (i),key (h(1))
            ) engine=innodb default charset latin1;
             
            replace into t(c) values ('');
            replace into t(c) values ('');
            alter table t drop column d ;
             
            drop table t;
            

            Before MDEV-15562, the DROP COLUMN would rebuild the table. After (and without the added work-around) it would be an instantaneous operation.

            Occasionally, there would be a crash in purge, if purge gets a chance to run before the drop table. Adding --sleep 1 or wait_all_purged.inc did not improve the chances for me. You will have to repeat the test many times and run multiple instances of it in parallel, to get a crash. It never crashed for me on 10.2 or 10.3.

            I saw two types of crashes:

            bb-10.4-mdev-15562 83afd52ed01819c048ed7962c5dcec4c9bb95c6d

            #2  0x0000563b279cbf2e in handle_fatal_signal (sig=11) at /mariadb/10.4m/sql/signal_handler.cc:305
            #3  <signal handler called>
            #4  __memmove_avx_unaligned_erms () at ../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S:274
            #5  0x0000563b27bf7ee5 in mem_heap_dup (heap=0x7f1e0c00ddd0, data=0xf168000000007f1e, len=1) at /mariadb/10.4m/storage/innobase/include/mem0mem.h:306
            #6  0x0000563b27bccea9 in dfield_dup (field=0x7f1e0c00e090, heap=0x7f1e0c00ddd0) at /mariadb/10.4m/storage/innobase/include/data0data.ic:287
            #7  0x0000563b27bf4803 in innobase_get_computed_value (row=0x7f1e0c00def8, col=0x7f1df42a72d0, index=0x7f1df41eda58, local_heap=0x7f1e2ce11560, heap=0x7f1e0c00ddd0, ifield=0x0, thd=0x7f1e0c000cf8, mysql_table=0x7f1df42af168, mysql_rec=0x7f1e0c00e958 "\023", '\063' <repeats 15 times>, "\027 \347\003", old_table=0x0, parent_update=0x0, foreign=0x0) at /mariadb/10.4m/storage/innobase/handler/ha_innodb.cc:20834
            #8  0x0000563b27d923aa in row_vers_build_clust_v_col (row=0x7f1e0c00def8, clust_index=0x7f1df41eda58, index=0x7f1df4237888, heap=0x7f1e0c00ddd0, vcol_info=0x563b2a35aae8) at /mariadb/10.4m/storage/innobase/row/row0vers.cc:483
            #9  0x0000563b27d93473 in row_vers_old_has_index_entry (also_curr=true, rec=0x7f1e41d5c081 "", mtr=0x7f1e2ce119e0, index=0x7f1df4237888, ientry=0x7f1e0c00d268, roll_ptr=27021597796210643, trx_id=16957, vcol_info=0x563b2a35aae8) at /mariadb/10.4m/storage/innobase/row/row0vers.cc:958
            #10 0x0000563b27d5a1b8 in row_purge_poss_sec (node=0x563b2a35a948, index=0x7f1df4237888, entry=0x7f1e0c00d268, sec_pcur=0x7f1e2ce11f80, sec_mtr=0x7f1e2ce12220, is_tree=false) at /mariadb/10.4m/storage/innobase/row/row0purge.cc:342
            #11 0x0000563b27d5aef8 in row_purge_remove_sec_if_poss_leaf (node=0x563b2a35a948, index=0x7f1df4237888, entry=0x7f1e0c00d268) at /mariadb/10.4m/storage/innobase/row/row0purge.cc:604
            #12 0x0000563b27d5b3e9 in row_purge_remove_sec_if_poss (node=0x563b2a35a948, index=0x7f1df4237888, entry=0x7f1e0c00d268) at /mariadb/10.4m/storage/innobase/row/row0purge.cc:717
            #13 0x0000563b27d5b609 in row_purge_del_mark (node=0x563b2a35a948) at /mariadb/10.4m/storage/innobase/row/row0purge.cc:791
            #14 0x0000563b27d5cc26 in row_purge_record_func (node=0x563b2a35a948, undo_rec=0x563b2a35aef8 "", thr=0x563b2a35a778, updated_extern=false) at /mariadb/10.4m/storage/innobase/row/row0purge.cc:1191
            

            and another:

            bb-10.4-mdev-15562 83afd52ed01819c048ed7962c5dcec4c9bb95c6d

            #7  0x00007fbb9e2e0b02 in __GI___assert_fail (assertion=0x55e715b65681 "0", file=0x55e715b65df0 "/mariadb/10.4m/storage/innobase/row/row0purge.cc", line=621, function=0x55e715b67420 <row_purge_remove_sec_if_poss_leaf(purge_node_t*, dict_index_t*, dtuple_t const*)::__PRETTY_FUNCTION__> "bool row_purge_remove_sec_if_poss_leaf(purge_node_t*, dict_index_t*, const dtuple_t*)") at assert.c:101
            #8  0x000055e71545407b in row_purge_remove_sec_if_poss_leaf (node=0x55e718747948, index=0x7fbb4c242a88, entry=0x7fbb7801cea8) at /mariadb/10.4m/storage/innobase/row/row0purge.cc:621
            #9  0x000055e7154543e9 in row_purge_remove_sec_if_poss (node=0x55e718747948, index=0x7fbb4c242a88, entry=0x7fbb7801cea8) at /mariadb/10.4m/storage/innobase/row/row0purge.cc:717
            #10 0x000055e715454609 in row_purge_del_mark (node=0x55e718747948) at /mariadb/10.4m/storage/innobase/row/row0purge.cc:791
            #11 0x000055e715455c26 in row_purge_record_func (node=0x55e718747948, undo_rec=0x7fbb7800ce78 "", thr=0x55e718747778, updated_extern=false) at /mariadb/10.4m/storage/innobase/row/row0purge.cc:1191
            

            ASAN did not flag anything. I tried both with and without it.

            Due to this, until this bug has been fixed, 10.4 will have a regression compared to 10.3: instant ADD COLUMN (at the end of the column list) will not be supported if indexed virtual columns exist.

            marko Marko Mäkelä added a comment - To make the situation worse, in MDEV-15562 I had to disable instant ADD / DROP /reorder of stored columns if indexed virtual columns exist. The reason is that I occasionally got 2 distinct crashes for the following portion of the test gcol.innodb_virtual_basic : --source include/have_innodb.inc create table t ( a int ,b int ,c text,d int ,e int ,f int ,g int , h text generated always as ( '1' ) virtual, i int ,j int ,k int ,l int ,m int , primary key (c(1)), unique key (c(1)), key (i), key (h(1)) ) engine=innodb default charset latin1;   replace into t(c) values ( '' ); replace into t(c) values ( '' ); alter table t drop column d ;   drop table t; Before MDEV-15562 , the DROP COLUMN would rebuild the table. After (and without the added work-around) it would be an instantaneous operation. Occasionally, there would be a crash in purge, if purge gets a chance to run before the drop table . Adding --sleep 1 or wait_all_purged.inc did not improve the chances for me. You will have to repeat the test many times and run multiple instances of it in parallel, to get a crash. It never crashed for me on 10.2 or 10.3. I saw two types of crashes: bb-10.4-mdev-15562 83afd52ed01819c048ed7962c5dcec4c9bb95c6d #2 0x0000563b279cbf2e in handle_fatal_signal (sig=11) at /mariadb/10.4m/sql/signal_handler.cc:305 #3 <signal handler called> #4 __memmove_avx_unaligned_erms () at ../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S:274 #5 0x0000563b27bf7ee5 in mem_heap_dup (heap=0x7f1e0c00ddd0, data=0xf168000000007f1e, len=1) at /mariadb/10.4m/storage/innobase/include/mem0mem.h:306 #6 0x0000563b27bccea9 in dfield_dup (field=0x7f1e0c00e090, heap=0x7f1e0c00ddd0) at /mariadb/10.4m/storage/innobase/include/data0data.ic:287 #7 0x0000563b27bf4803 in innobase_get_computed_value (row=0x7f1e0c00def8, col=0x7f1df42a72d0, index=0x7f1df41eda58, local_heap=0x7f1e2ce11560, heap=0x7f1e0c00ddd0, ifield=0x0, thd=0x7f1e0c000cf8, mysql_table=0x7f1df42af168, mysql_rec=0x7f1e0c00e958 "\023", '\063' <repeats 15 times>, "\027 \347\003", old_table=0x0, parent_update=0x0, foreign=0x0) at /mariadb/10.4m/storage/innobase/handler/ha_innodb.cc:20834 #8 0x0000563b27d923aa in row_vers_build_clust_v_col (row=0x7f1e0c00def8, clust_index=0x7f1df41eda58, index=0x7f1df4237888, heap=0x7f1e0c00ddd0, vcol_info=0x563b2a35aae8) at /mariadb/10.4m/storage/innobase/row/row0vers.cc:483 #9 0x0000563b27d93473 in row_vers_old_has_index_entry (also_curr=true, rec=0x7f1e41d5c081 "", mtr=0x7f1e2ce119e0, index=0x7f1df4237888, ientry=0x7f1e0c00d268, roll_ptr=27021597796210643, trx_id=16957, vcol_info=0x563b2a35aae8) at /mariadb/10.4m/storage/innobase/row/row0vers.cc:958 #10 0x0000563b27d5a1b8 in row_purge_poss_sec (node=0x563b2a35a948, index=0x7f1df4237888, entry=0x7f1e0c00d268, sec_pcur=0x7f1e2ce11f80, sec_mtr=0x7f1e2ce12220, is_tree=false) at /mariadb/10.4m/storage/innobase/row/row0purge.cc:342 #11 0x0000563b27d5aef8 in row_purge_remove_sec_if_poss_leaf (node=0x563b2a35a948, index=0x7f1df4237888, entry=0x7f1e0c00d268) at /mariadb/10.4m/storage/innobase/row/row0purge.cc:604 #12 0x0000563b27d5b3e9 in row_purge_remove_sec_if_poss (node=0x563b2a35a948, index=0x7f1df4237888, entry=0x7f1e0c00d268) at /mariadb/10.4m/storage/innobase/row/row0purge.cc:717 #13 0x0000563b27d5b609 in row_purge_del_mark (node=0x563b2a35a948) at /mariadb/10.4m/storage/innobase/row/row0purge.cc:791 #14 0x0000563b27d5cc26 in row_purge_record_func (node=0x563b2a35a948, undo_rec=0x563b2a35aef8 "", thr=0x563b2a35a778, updated_extern=false) at /mariadb/10.4m/storage/innobase/row/row0purge.cc:1191 and another: bb-10.4-mdev-15562 83afd52ed01819c048ed7962c5dcec4c9bb95c6d #7 0x00007fbb9e2e0b02 in __GI___assert_fail (assertion=0x55e715b65681 "0", file=0x55e715b65df0 "/mariadb/10.4m/storage/innobase/row/row0purge.cc", line=621, function=0x55e715b67420 <row_purge_remove_sec_if_poss_leaf(purge_node_t*, dict_index_t*, dtuple_t const*)::__PRETTY_FUNCTION__> "bool row_purge_remove_sec_if_poss_leaf(purge_node_t*, dict_index_t*, const dtuple_t*)") at assert.c:101 #8 0x000055e71545407b in row_purge_remove_sec_if_poss_leaf (node=0x55e718747948, index=0x7fbb4c242a88, entry=0x7fbb7801cea8) at /mariadb/10.4m/storage/innobase/row/row0purge.cc:621 #9 0x000055e7154543e9 in row_purge_remove_sec_if_poss (node=0x55e718747948, index=0x7fbb4c242a88, entry=0x7fbb7801cea8) at /mariadb/10.4m/storage/innobase/row/row0purge.cc:717 #10 0x000055e715454609 in row_purge_del_mark (node=0x55e718747948) at /mariadb/10.4m/storage/innobase/row/row0purge.cc:791 #11 0x000055e715455c26 in row_purge_record_func (node=0x55e718747948, undo_rec=0x7fbb7800ce78 "", thr=0x55e718747778, updated_extern=false) at /mariadb/10.4m/storage/innobase/row/row0purge.cc:1191 ASAN did not flag anything. I tried both with and without it. Due to this, until this bug has been fixed, 10.4 will have a regression compared to 10.3: instant ADD COLUMN (at the end of the column list) will not be supported if indexed virtual columns exist.

            The second one looks much like MDEV-16222.

            elenst Elena Stepanova added a comment - The second one looks much like MDEV-16222 .

            People

              thiru Thirunarayanan Balathandayuthapani
              marko Marko Mäkelä
              Votes:
              1 Vote for this issue
              Watchers:
              7 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.