From c83f87ece4a48f3ae38c2962532dc228c857bfb7 Mon Sep 17 00:00:00 2001 From: Nikita Malyavin Date: Fri, 20 Jul 2018 00:24:40 +1000 Subject: [PATCH] MDEV-15990: REPLACE on a precise-versioned table returns duplicate key error (ER_DUP_ENTRY) * handle zero value in `row_start` `trx_id` * refuse to do more than one versioned insert on the same transaction and same row * generalize versioned cases with and without triggers [closes tempesta-tech/mariadb#517] [closes tempesta-tech/mariadb#520] --- .../suite/versioning/r/optimized.result | 25 +++++++- mysql-test/suite/versioning/r/replace.result | 29 ++++++++-- mysql-test/suite/versioning/t/optimized.test | 28 ++++++++- mysql-test/suite/versioning/t/replace.test | 20 ++++++- sql/sql_insert.cc | 57 ++++++++----------- storage/innobase/handler/ha_innodb.cc | 18 +++++- 6 files changed, 135 insertions(+), 42 deletions(-) diff --git a/mysql-test/suite/versioning/r/optimized.result b/mysql-test/suite/versioning/r/optimized.result index 2385a43d0a4eb..707483ae89c47 100644 --- a/mysql-test/suite/versioning/r/optimized.result +++ b/mysql-test/suite/versioning/r/optimized.result @@ -61,6 +61,26 @@ a b 3 4 select * from t for system_time as of timestamp now(6) where b is NULL; a b +# Replace on unversioned field should create history record +create or replace table t( +id int primary key, +x int without system versioning, +row_start SYS_DATATYPE as row start invisible, +row_end SYS_DATATYPE as row end invisible, +period for system_time(row_start, row_end) +) with system versioning; +insert t values (1, 2); +replace t values (1, 3); +select *, check_row(row_start, row_end), row_start != 0 as start_nonzero +from t for system_time all order by x; +id x check_row(row_start, row_end) start_nonzero +1 2 HISTORICAL ROW 1 +1 3 CURRENT ROW 1 +# ensure that row_start is not duplicated +select count(row_start) as empty from t for system_time all +group by row_start having count(row_start) > 1; +empty +drop table t; create or replace table t (x int with system versioning, y int); select column_name, extra from information_schema.columns where table_name='t'; column_name extra @@ -71,5 +91,6 @@ Table Create Table t CREATE TABLE `t` ( `x` int(11) DEFAULT NULL, `y` int(11) DEFAULT NULL WITHOUT SYSTEM VERSIONING -) ENGINE=MyISAM DEFAULT CHARSET=latin1 WITH SYSTEM VERSIONING -drop table t; +) ENGINE=DEFAULT_ENGINE DEFAULT CHARSET=latin1 WITH SYSTEM VERSIONING +drop database test; +create database test; diff --git a/mysql-test/suite/versioning/r/replace.result b/mysql-test/suite/versioning/r/replace.result index 1711be5212e30..09b9bfea1d2a2 100644 --- a/mysql-test/suite/versioning/r/replace.result +++ b/mysql-test/suite/versioning/r/replace.result @@ -7,10 +7,31 @@ period for system_time (row_start, row_end) ) with system versioning; insert t values (1, 2); replace t values (1, 3); -select *, current_row(row_end) as current from t for system_time all order by x; -id x current -1 2 0 -1 3 1 +replace t values (1, 3); +# MDEV-15990: REPLACE on a precise-versioned table +# returns duplicate key error (ER_DUP_ENTRY) +replace t values (1, 3), (1, 30), (1, 40); +create table log(s varchar(3)); +create trigger tr1del before delete on t +for each row insert into log values('DEL'); +replace t values (1, 4), (1, 40), (1, 400); +select *, check_row(row_start, row_end), row_start != 0 as start_nonzero +from t for system_time all order by x, row_end; +id x check_row(row_start, row_end) start_nonzero +1 2 HISTORICAL ROW 1 +1 3 HISTORICAL ROW 1 +1 3 HISTORICAL ROW 1 +1 40 HISTORICAL ROW 1 +1 400 CURRENT ROW 1 +select * from log; +s +DEL +DEL +DEL +# ensure that row_start is not duplicated +select count(row_start) as empty from t for system_time all +group by row_start having count(row_start) > 1; +empty drop table t; create table t ( id int unique, diff --git a/mysql-test/suite/versioning/t/optimized.test b/mysql-test/suite/versioning/t/optimized.test index 054c1d32559c7..0d59c41881149 100644 --- a/mysql-test/suite/versioning/t/optimized.test +++ b/mysql-test/suite/versioning/t/optimized.test @@ -1,3 +1,6 @@ +--source suite/versioning/common.inc +--source suite/versioning/engines.inc + create table t ( a int, b int without system versioning @@ -30,11 +33,34 @@ insert into t values (1, 2), (3, 4); select * from t for system_time as of timestamp now(6); select * from t for system_time as of timestamp now(6) where b is NULL; +--echo # Replace on unversioned field should create history record +--replace_result $sys_datatype_expl SYS_DATATYPE +eval create or replace table t( + id int primary key, + x int without system versioning, + row_start $sys_datatype_expl as row start invisible, + row_end $sys_datatype_expl as row end invisible, + period for system_time(row_start, row_end) +) with system versioning; + +insert t values (1, 2); +replace t values (1, 3); +select *, check_row(row_start, row_end), row_start != 0 as start_nonzero + from t for system_time all order by x; + +--echo # ensure that row_start is not duplicated +select count(row_start) as empty from t for system_time all + group by row_start having count(row_start) > 1; + +drop table t; + # # MDEV-15062 Information Schema COLUMNS Table does not show system versioning information # create or replace table t (x int with system versioning, y int); select column_name, extra from information_schema.columns where table_name='t'; +--replace_result $default_engine DEFAULT_ENGINE $sys_datatype_expl SYS_DATATYPE show create table t; -drop table t; +drop database test; +create database test; diff --git a/mysql-test/suite/versioning/t/replace.test b/mysql-test/suite/versioning/t/replace.test index bc7e084758db7..9f3b6df62392f 100644 --- a/mysql-test/suite/versioning/t/replace.test +++ b/mysql-test/suite/versioning/t/replace.test @@ -12,7 +12,25 @@ eval create table t ( insert t values (1, 2); replace t values (1, 3); -select *, current_row(row_end) as current from t for system_time all order by x; +replace t values (1, 3); + +--echo # MDEV-15990: REPLACE on a precise-versioned table +--echo # returns duplicate key error (ER_DUP_ENTRY) +replace t values (1, 3), (1, 30), (1, 40); + +create table log(s varchar(3)); +create trigger tr1del before delete on t + for each row insert into log values('DEL'); +replace t values (1, 4), (1, 40), (1, 400); + +select *, check_row(row_start, row_end), row_start != 0 as start_nonzero + from t for system_time all order by x, row_end; +select * from log; + +--echo # ensure that row_start is not duplicated +select count(row_start) as empty from t for system_time all + group by row_start having count(row_start) > 1; + drop table t; --replace_result $sys_datatype_expl SYS_DATATYPE diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc index dda469a1426d5..cd2690128de67 100644 --- a/sql/sql_insert.cc +++ b/sql/sql_insert.cc @@ -1928,33 +1928,16 @@ int write_record(THD *thd, TABLE *table,COPY_INFO *info) For system versioning wa also use path through delete since we would save nothing through this cheating. */ - if (last_uniq_key(table,key_nr) && + if (last_uniq_key(table,key_nr) && !table->versioned() && !table->file->referenced_by_foreign_key() && (!table->triggers || !table->triggers->has_delete_triggers())) { - if (table->versioned(VERS_TRX_ID) && table->vers_write) - { - bitmap_set_bit(table->write_set, table->vers_start_field()->field_index); - table->vers_start_field()->store(0, false); - } - if (unlikely(error= table->file->ha_update_row(table->record[1], - table->record[0])) && - error != HA_ERR_RECORD_IS_THE_SAME) + error= table->file->ha_update_row(table->record[1], table->record[0]); + if (unlikely(error && error != HA_ERR_RECORD_IS_THE_SAME)) goto err; if (likely(!error)) - { info->deleted++; - if (table->versioned(VERS_TIMESTAMP) && table->vers_write) - { - store_record(table, record[2]); - error= vers_insert_history_row(table); - restore_record(table, record[2]); - if (unlikely(error)) - goto err; - } - } - else - error= 0; // error was HA_ERR_RECORD_IS_THE_SAME + error= 0; // error was HA_ERR_RECORD_IS_THE_SAME thd->record_first_successful_insert_id_in_cur_stmt(table->file->insert_id_for_cur_row); /* Since we pretend that we have done insert we should call @@ -1969,23 +1952,22 @@ int write_record(THD *thd, TABLE *table,COPY_INFO *info) TRG_ACTION_BEFORE, TRUE)) goto before_trg_err; - if (!table->versioned(VERS_TIMESTAMP)) + if (table->versioned(VERS_TRX_ID) && table->vers_write) + { + bitmap_set_bit(table->write_set, table->vers_start_field()->field_index); + table->vers_start_field()->store(0, false); + } + + if (!table->versioned()) error= table->file->ha_delete_row(table->record[1]); else - { - store_record(table, record[2]); - restore_record(table, record[1]); - table->vers_update_end(); error= table->file->ha_update_row(table->record[1], table->record[0]); - restore_record(table, record[2]); - } - if (unlikely(error)) + if (unlikely(error && error != HA_ERR_RECORD_IS_THE_SAME)) goto err; - if (!table->versioned(VERS_TIMESTAMP)) + if (likely(!error)) info->deleted++; - else - info->updated++; + error= 0; if (!table->file->has_transactions()) thd->transaction.stmt.modified_non_trans_table= TRUE; if (table->triggers && @@ -1995,6 +1977,17 @@ int write_record(THD *thd, TABLE *table,COPY_INFO *info) trg_error= 1; goto ok_or_after_trg_err; } + if (table->versioned(VERS_TIMESTAMP) && table->vers_write) + { + store_record(table, record[2]); + error = vers_insert_history_row(table); + restore_record(table, record[2]); + if (unlikely(error)) + goto err; + } + if (table->versioned()) + goto after_trg_n_copied_inc; + /* Let us attempt do write_row() once more */ } } diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index d0398bfe0e1b5..15693eebf5dfa 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -8800,6 +8800,19 @@ ha_innobase::update_row( upd_t* uvect = row_get_prebuilt_update_vector(m_prebuilt); ib_uint64_t autoinc; + bool force_insert= table->versioned_write(VERS_TRX_ID) + && table->vers_start_id() == 0; + + if (force_insert) { + table->vers_start_field()->store(trx->id); + + Field *start= table->vers_start_field(); + trx_id_t old_trx_id= uint8korr(start->ptr_in_record(old_row)); + + /* avoid double versioned insert on the same transaction and same row */ + force_insert= (old_trx_id != trx->id); + } + /* Build an update vector from the modified fields in the rows (uses m_upd_buf of the handle) */ @@ -8819,7 +8832,7 @@ ha_innobase::update_row( DBUG_RETURN(HA_ERR_RECORD_IS_THE_SAME); } else { const bool vers_set_fields = m_prebuilt->versioned_write - && m_prebuilt->upd_node->update->affects_versioned(); + && (m_prebuilt->upd_node->update->affects_versioned() || force_insert); const bool vers_ins_row = vers_set_fields && thd_sql_command(m_user_thd) != SQLCOM_ALTER_TABLE; @@ -8838,7 +8851,8 @@ ha_innobase::update_row( if (error == DB_SUCCESS && vers_ins_row /* Multiple UPDATE of same rows in single transaction create historical rows only once. */ - && trx->id != table->vers_start_id()) { + && (trx->id != table->vers_start_id() || force_insert) + && trx->id != table->vers_end_id()){ error = row_insert_for_mysql((byte*) old_row, m_prebuilt, ROW_INS_HISTORICAL);