Details
-
Bug
-
Status: Stalled (View Workflow)
-
Major
-
Resolution: Unresolved
-
10.2(EOL), 10.3(EOL), 10.4(EOL)
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
- duplicates
-
MDEV-32668 tables with UNIQUE blob columns cannot be alter_algorithm=INSTANT modified
-
- Open
-
- relates to
-
MDEV-5800 indexes on virtual (not materialized) columns
-
- Closed
-
-
MDEV-11369 Instant add column for InnoDB
-
- Closed
-
-
MDEV-11424 Instant ALTER TABLE of failure-free record format changes
-
- Closed
-
-
MDEV-14046 Allow ALGORITHM=INPLACE for 10.1 tables that contain virtual columns
-
- Closed
-
-
MDEV-14341 Allow LOCK=NONE in table-rebuilding ALTER when indexed virtual columns exist
-
- Open
-
-
MDEV-15476 Inplace algorithm doesn't support changing virtual column datatype
-
- Stalled
-
-
MDEV-15562 Instant DROP COLUMN or changing the order of columns
-
- Closed
-
-
MDEV-16332 Allow ALGORITHM=NOCOPY or INSTANT for changes of virtual column type
-
- Confirmed
-
-
MDEV-17035 Support ALGORITHM=NOCOPY for CHANGE virtual column expression
-
- Open
-
-
MDEV-19214 Virtual column type cannot be converted from one to another - unhelpful error message
-
- Open
-
-
MDEV-22363 Reimplement the InnoDB virtual column code
-
- Open
-
To make the situation worse, in
MDEV-15562I 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
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.