Hi, Nikita, Sorry, this comes very late. But the bug is still present in 10.10, and the filter sorts by the date precisely to highlight these old bugs. So, here you are. First comment, please, port it to the latest 10.3. I wasn't able to find the repository with the fix, but the patch itself is avaliable from the pull request as https://github.com/MariaDB/server/commit/c83f87ece4a.patch so it can be salvaged. More comments - see below. On Oct 15, Nikita Malyavin wrote:

From: Nikita Malyavin Date: Fri, 20 Jul 2018 00:24:40 +1000 Subject: [PATCH] MDEV-15990: REPLACE on a precise-versioned table returns * 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

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,32 +1928,15 @@ 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() &&

ok!

!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 thd->record_first_successful_insert_id_in_cur_stmt(table->file->insert_id_for_cur_row); /* @@ -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);

this is conceptually wrong, you cannot set table->write_set in the middle of statement execution. that is you can and it might even work for some engines, but it breaks the API.

+ 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();

why? is the row_end updated somewhere else?

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);

if you do that, you shouldn't have been doing ha_update_row() above, as far as I can see. What is that update above for?

+ 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 */ } }

Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org