Details
-
Bug
-
Status: Closed (View Workflow)
-
Critical
-
Resolution: Fixed
-
N/A
-
None
Description
--source include/have_debug_sync.inc
|
--source include/have_innodb.inc
|
|
create table t1 (a int primary key) engine=InnoDB; |
insert into t1 values (1),(2),(3); |
create table t2 (b int, foreign key (b) references t1 (a) on update cascade) engine=InnoDB; |
insert into t2 values (1),(2),(3); |
--send
|
set debug_sync= 'now wait_for downgraded'; |
|
--connect (con_alter,localhost,root,,test)
|
set debug_sync= 'alter_table_online_downgraded signal downgraded wait_for goforit'; |
--send
|
alter table t2 add c int, algorithm=copy, lock=none; |
|
--connection default
|
--reap
|
update t1 set a = 22 where a = 2; |
set debug_sync= 'now signal goforit'; |
|
--connection con_alter
|
--reap
|
select * from t2; |
|
# Cleanup
|
drop table t2, t1; |
set debug_sync= reset; |
bb-10.10-MDEV-16329 49ad87590 |
connection default; |
update t1 set a = 22 where a = 2; |
set debug_sync= 'now signal goforit'; |
connection con_alter; |
select * from t2; |
b c
|
1 NULL |
2 NULL |
3 NULL |
Expected 22 instead of 2, via update cascade. 2 is now an orphan row.
Attachments
Issue Links
- is caused by
-
MDEV-16329 Engine-independent online ALTER TABLE
-
- Closed
-
- relates to
-
MDEV-20640 [ERROR] InnoDB: tried to purge non-delete-marked record and Assertion `0' failed in row_purge_remove_sec_if_poss_leaf
-
- Closed
-
-
MDEV-29182 Assertion `fld->field_no < table->n_v_def' failed on cascade FK update of table with vcol index in parent
-
- Closed
-
-
MDEV-28808 Test MDEV-16329 (ALTER ONLINE TABLE) - Core server part
-
- Closed
-
Thank you. Here is some quick feedback before studying the changes in more detail. This must not only pass my review, but also correctness testing by mleich and performance testing by axel, once we are that far.
Unlike THD, InnoDB always knew about the internals of TABLE and TABLE_SHARE. So, please do not add sql_get_maria_table_record().
Frequently invoking current_thd or innodb_find_table_for_vc(current_thd, table) is something that should be avoided due to potential performance regression. (In the purge of committed transaction history we do it, but that is due to bad design, which will hopefully be addressed by MDEV-17598.)
Can we just ensure that table->vc_templ->mysql_table is always initialized when necessary? There is a mismatch here, because dict_table_t actually corresponds to TABLE_SHARE while row_prebuilt_t corresponds to TABLE. I believe that this dict_table_t::vc_templ is some dummy shared handle that is mainly used for metadata. I did not check how virtual column values are evaluated; hopefully no concurrent calls use the same TABLE::record buffer in table->vc_templ->mysql_table->record.
In report_row_update(), there is a conversion of a record followed by computing the offsets to the start of each field, followed by converting to the TABLE::record format. Can we please just convert directly from dtuple_t to TABLE::record?
Please test this also for versioned tables, for both UPDATE and DELETE.
The Boolean data member upd_node_t::in_mysql_interface could be replaced with a 3-valued enum, using a third value instead of your proposed upd_node_t::sql_is_online_alter. The change to row_upd() cleanup needs to be explained in a source code comment or the commit message.