From 932ec586aada4bd78f613ee10750effc7f442327 Mon Sep 17 00:00:00 2001 From: Aleksey Midenkov Date: Tue, 22 Dec 2020 03:33:53 +0300 Subject: [PATCH] MDEV-23644 Assertion on evaluating foreign referential action for self-reference in system versioned table First part of the fix (row0mysql.cc) addresses external columns when adding history row on referential action. The full data must be retrieved before the row is inserted. Second part of the fix (the rest) avoids duplicate primary key error between the history row generated on referential action and the history row generated by SQL command. Both command and referential action can happen on same table since foreign key can be self-reference (parent and child tables are same). Moreover, the self-reference can refer multiple rows when the key is non-unique. In such case history is generated by referential action occured on first row but processed all rows by a matched key. The second round is when the next row is processed by a command but history already exists. In such case we check TRX_ID of existing history row and if it is the same we assume the above situation and skip adding one more history row or failing the command. --- mysql-test/suite/versioning/common.inc | 21 +++++++++++++++++++ mysql-test/suite/versioning/common_finish.inc | 2 ++ mysql-test/suite/versioning/r/delete.result | 8 +++---- mysql-test/suite/versioning/r/foreign.result | 16 ++++++++++++++ mysql-test/suite/versioning/r/trx_id.result | 8 +++++++ mysql-test/suite/versioning/t/delete.test | 4 ++-- mysql-test/suite/versioning/t/foreign.test | 18 ++++++++++++++++ mysql-test/suite/versioning/t/trx_id.test | 6 ++++++ sql/sql_delete.cc | 10 ++++++++- storage/innobase/data/data0data.cc | 2 +- storage/innobase/include/data0data.h | 10 +++++++++ storage/innobase/row/row0ins.cc | 12 +++++++++++ storage/innobase/row/row0mysql.cc | 21 ++++++++++++++++++- 13 files changed, 129 insertions(+), 9 deletions(-) diff --git a/mysql-test/suite/versioning/common.inc b/mysql-test/suite/versioning/common.inc index efb081a02e4..25adf15dd50 100644 --- a/mysql-test/suite/versioning/common.inc +++ b/mysql-test/suite/versioning/common.inc @@ -70,6 +70,11 @@ returns int deterministic return sys_trx_end = $sys_datatype_max; +eval create or replace function current_row_ts(sys_trx_end timestamp(6)) +returns int +deterministic + return convert_tz(sys_trx_end, '+00:00', @@time_zone) = TIMESTAMP'2038-01-19 03:14:07.999999'; + delimiter ~~; eval create or replace function check_row(row_start $sys_datatype_expl, row_end $sys_datatype_expl) returns varchar(255) @@ -86,4 +91,20 @@ begin end~~ delimiter ;~~ +delimiter ~~; +eval create or replace function check_row_ts(row_start timestamp(6), row_end timestamp(6)) +returns varchar(255) +deterministic +begin + if row_end < row_start then + return "ERROR: row_end < row_start"; + elseif row_end = row_start then + return "ERROR: row_end == row_start"; + elseif current_row_ts(row_end) then + return "CURRENT ROW"; + end if; + return "HISTORICAL ROW"; +end~~ +delimiter ;~~ + --enable_query_log diff --git a/mysql-test/suite/versioning/common_finish.inc b/mysql-test/suite/versioning/common_finish.inc index 61641c6c5ce..3c4e7b66ff3 100644 --- a/mysql-test/suite/versioning/common_finish.inc +++ b/mysql-test/suite/versioning/common_finish.inc @@ -4,5 +4,7 @@ drop procedure if exists verify_trt; drop procedure if exists verify_trt_dummy; drop function if exists current_row; drop function if exists check_row; +drop function if exists current_row_ts; +drop function if exists check_row_ts; --enable_warnings --enable_query_log diff --git a/mysql-test/suite/versioning/r/delete.result b/mysql-test/suite/versioning/r/delete.result index 4a0fec639b0..0f9e2c22130 100644 --- a/mysql-test/suite/versioning/r/delete.result +++ b/mysql-test/suite/versioning/r/delete.result @@ -138,13 +138,13 @@ f1 int, f2 text, f3 int, fulltext (f2), key(f1), key(f3), foreign key r (f3) references t1 (f1) on delete set null) with system versioning engine innodb; insert into t1 values (1, repeat('a', 8193), 1), (1, repeat('b', 8193), 1); -select f1, f3, check_row(row_start, row_end) from t1; -f1 f3 check_row(row_start, row_end) +select f1, f3, check_row_ts(row_start, row_end) from t1; +f1 f3 check_row_ts(row_start, row_end) 1 1 CURRENT ROW 1 1 CURRENT ROW delete from t1; -select f1, f3, check_row(row_start, row_end) from t1 for system_time all; -f1 f3 check_row(row_start, row_end) +select f1, f3, check_row_ts(row_start, row_end) from t1 for system_time all; +f1 f3 check_row_ts(row_start, row_end) 1 1 HISTORICAL ROW 1 NULL ERROR: row_end == row_start 1 1 HISTORICAL ROW diff --git a/mysql-test/suite/versioning/r/foreign.result b/mysql-test/suite/versioning/r/foreign.result index 32b5e5cf3d8..e54afdbc74e 100644 --- a/mysql-test/suite/versioning/r/foreign.result +++ b/mysql-test/suite/versioning/r/foreign.result @@ -398,6 +398,8 @@ Warning 1265 Data truncated for column 'f12' at row 7 SET timestamp = 9; REPLACE INTO t2 SELECT * FROM t2; DROP TABLE t1, t2; +set timestamp= default; +set time_zone='+00:00'; # # MDEV-16210 FK constraints on versioned tables use historical rows, which may cause constraint violation # @@ -427,3 +429,17 @@ insert into t2 values (1), (1); # DELETE from foreign table is allowed delete from t2; drop tables t2, t1; +# +# MDEV-23644 Assertion on evaluating foreign referential action for self-reference in system versioned table +# +create table t1 (pk int primary key, f1 int,f2 int, f3 text, +key(f1), fulltext(f3), key(f3(10)), +foreign key (f2) references t1 (f1) on delete set null +) engine=innodb with system versioning; +insert into t1 values (1, 8, 8, 'SHORT'), (2, 8, 8, repeat('LONG', 8071)); +delete from t1; +select pk, f1, f2, left(f3, 4), check_row_ts(row_start, row_end) from t1 for system_time all order by pk; +pk f1 f2 left(f3, 4) check_row_ts(row_start, row_end) +1 8 8 SHOR HISTORICAL ROW +2 8 8 LONG HISTORICAL ROW +drop table t1; diff --git a/mysql-test/suite/versioning/r/trx_id.result b/mysql-test/suite/versioning/r/trx_id.result index 5dd9e7aa188..58d02505fb6 100644 --- a/mysql-test/suite/versioning/r/trx_id.result +++ b/mysql-test/suite/versioning/r/trx_id.result @@ -5,7 +5,15 @@ sys_trx_start bigint(20) unsigned as row start invisible, sys_trx_end bigint(20) unsigned as row end invisible, period for system_time (sys_trx_start, sys_trx_end) ) with system versioning; +# No history inside the transaction +start transaction; insert into t1 (x) values (1); +update t1 set x= x + 1; +update t1 set x= x + 1; +commit; +select *, sys_trx_start > 1, sys_trx_end from t1 for system_time all; +x sys_trx_start > 1 sys_trx_end +3 1 18446744073709551615 # ALTER ADD SYSTEM VERSIONING should write to mysql.transaction_registry set @@system_versioning_alter_history=keep; create or replace table t1 (x int); diff --git a/mysql-test/suite/versioning/t/delete.test b/mysql-test/suite/versioning/t/delete.test index e2f7240303d..a5a0497fef2 100644 --- a/mysql-test/suite/versioning/t/delete.test +++ b/mysql-test/suite/versioning/t/delete.test @@ -102,9 +102,9 @@ create table t1 ( foreign key r (f3) references t1 (f1) on delete set null) with system versioning engine innodb; insert into t1 values (1, repeat('a', 8193), 1), (1, repeat('b', 8193), 1); -select f1, f3, check_row(row_start, row_end) from t1; +select f1, f3, check_row_ts(row_start, row_end) from t1; delete from t1; -select f1, f3, check_row(row_start, row_end) from t1 for system_time all; +select f1, f3, check_row_ts(row_start, row_end) from t1 for system_time all; # cleanup drop table t1; diff --git a/mysql-test/suite/versioning/t/foreign.test b/mysql-test/suite/versioning/t/foreign.test index 7493f99cba7..725f51f0660 100644 --- a/mysql-test/suite/versioning/t/foreign.test +++ b/mysql-test/suite/versioning/t/foreign.test @@ -421,6 +421,8 @@ REPLACE INTO t2 SELECT * FROM t2; # Cleanup DROP TABLE t1, t2; +set timestamp= default; +set time_zone='+00:00'; --let $datadir= `select @@datadir` --remove_file $datadir/test/t1.data --remove_file $datadir/test/t1.data.2 @@ -458,4 +460,20 @@ insert into t2 values (1), (1); delete from t2; drop tables t2, t1; +--echo # +--echo # MDEV-23644 Assertion on evaluating foreign referential action for self-reference in system versioned table +--echo # +create table t1 (pk int primary key, f1 int,f2 int, f3 text, + key(f1), fulltext(f3), key(f3(10)), + foreign key (f2) references t1 (f1) on delete set null +) engine=innodb with system versioning; + +insert into t1 values (1, 8, 8, 'SHORT'), (2, 8, 8, repeat('LONG', 8071)); + +delete from t1; +select pk, f1, f2, left(f3, 4), check_row_ts(row_start, row_end) from t1 for system_time all order by pk; + +# cleanup +drop table t1; + --source suite/versioning/common_finish.inc diff --git a/mysql-test/suite/versioning/t/trx_id.test b/mysql-test/suite/versioning/t/trx_id.test index 38724a47fd1..7dfc8acb080 100644 --- a/mysql-test/suite/versioning/t/trx_id.test +++ b/mysql-test/suite/versioning/t/trx_id.test @@ -14,7 +14,13 @@ create or replace table t1 ( period for system_time (sys_trx_start, sys_trx_end) ) with system versioning; +--echo # No history inside the transaction +start transaction; insert into t1 (x) values (1); +update t1 set x= x + 1; +update t1 set x= x + 1; +commit; +select *, sys_trx_start > 1, sys_trx_end from t1 for system_time all; --echo # ALTER ADD SYSTEM VERSIONING should write to mysql.transaction_registry set @@system_versioning_alter_history=keep; diff --git a/sql/sql_delete.cc b/sql/sql_delete.cc index 929455977e6..0857c4e5457 100644 --- a/sql/sql_delete.cc +++ b/sql/sql_delete.cc @@ -246,7 +246,15 @@ int TABLE::delete_row() store_record(this, record[1]); vers_update_end(); - return file->ha_update_row(record[1], record[0]); + int err= file->ha_update_row(record[1], record[0]); + /* + MDEV-23644: we get HA_ERR_FOREIGN_DUPLICATE_KEY iff we already got history + row with same trx_id which is the result of foreign key action, so we + don't need one more history row. + */ + if (err == HA_ERR_FOREIGN_DUPLICATE_KEY) + return file->ha_delete_row(record[0]); + return err; } diff --git a/storage/innobase/data/data0data.cc b/storage/innobase/data/data0data.cc index 47e58f1614a..3e23cd6f662 100644 --- a/storage/innobase/data/data0data.cc +++ b/storage/innobase/data/data0data.cc @@ -738,7 +738,7 @@ void dtuple_convert_back_big_rec( /*========================*/ dict_index_t* index MY_ATTRIBUTE((unused)), /*!< in: index */ - dtuple_t* entry, /*!< in: entry whose data was put to vector */ + dtuple_t* entry, /*!< in/out: entry whose data was put to vector */ big_rec_t* vector) /*!< in, own: big rec vector; it is freed in this function */ { diff --git a/storage/innobase/include/data0data.h b/storage/innobase/include/data0data.h index 11a7f2e516f..002332852b8 100644 --- a/storage/innobase/include/data0data.h +++ b/storage/innobase/include/data0data.h @@ -543,6 +543,16 @@ struct dtuple_t { inserted or updated. @param[in] index index possibly with instantly added columns */ void trim(const dict_index_t& index); + bool vers_history_row() const + { + for (ulint i = 0; i < n_fields; i++) { + const dfield_t* field = &fields[i]; + if (field->type.vers_sys_end()) { + return field->vers_history_row(); + } + } + return false; + } }; inline ulint dtuple_get_n_fields(const dtuple_t* tuple) diff --git a/storage/innobase/row/row0ins.cc b/storage/innobase/row/row0ins.cc index 88152841293..43f80adebbd 100644 --- a/storage/innobase/row/row0ins.cc +++ b/storage/innobase/row/row0ins.cc @@ -2385,6 +2385,18 @@ row_ins_duplicate_error_in_clust( duplicate: trx->error_info = cursor->index; err = DB_DUPLICATE_KEY; + if (cursor->index->table->versioned() + && entry->vers_history_row()) + { + ulint trx_id_len; + byte *trx_id = rec_get_nth_field( + rec, offsets, n_unique, + &trx_id_len); + ut_ad(trx_id_len == DATA_TRX_ID_LEN); + if (trx->id == trx_read_trx_id(trx_id)) { + err = DB_FOREIGN_DUPLICATE_KEY; + } + } goto func_exit; } } diff --git a/storage/innobase/row/row0mysql.cc b/storage/innobase/row/row0mysql.cc index 848f60c47d0..c643a907722 100644 --- a/storage/innobase/row/row0mysql.cc +++ b/storage/innobase/row/row0mysql.cc @@ -2125,6 +2125,7 @@ static dberr_t row_update_vers_insert(que_thr_t* thr, upd_node_t* node) dfield_t* row_end; char row_end_data[8]; dict_table_t* table = node->table; + page_size_t page_size= dict_table_page_size(table); ut_ad(table->versioned()); dtuple_t* row; @@ -2152,9 +2153,27 @@ static dberr_t row_update_vers_insert(que_thr_t* thr, upd_node_t* node) ut_ad(n_cols > DATA_N_SYS_COLS); // Exclude DB_ROW_ID, DB_TRX_ID, DB_ROLL_PTR for (ulint i = 0; i < n_cols - DATA_N_SYS_COLS; i++) { - dfield_t *dst= dtuple_get_nth_field(row, i); dfield_t *src= dtuple_get_nth_field(node->historical_row, i); + dfield_t *dst= dtuple_get_nth_field(row, i); dfield_copy(dst, src); + if (dfield_is_ext(src)) { + byte *field_data + = static_cast(dfield_get_data(src)); + ulint ext_len; + ulint field_len = dfield_get_len(src); + + ut_a(field_len >= BTR_EXTERN_FIELD_REF_SIZE); + + ut_a(memcmp(field_data + field_len + - BTR_EXTERN_FIELD_REF_SIZE, + field_ref_zero, + BTR_EXTERN_FIELD_REF_SIZE)); + + byte *data = btr_copy_externally_stored_field( + &ext_len, field_data, page_size, field_len, + node->historical_heap); + dfield_set_data(dst, data, ext_len); + } } for (ulint i = 0; i < n_v_cols; i++) {