Re: [Maria-developers] 74b2eba1ca6: MDEV-15458 Segfault in heap_scan() upon UPDATE after ADD SYSTEM VERSIONING

Hi, Sergei! On Wed, Mar 27, 2019 at 1:04 AM Sergei Golubchik wrote:

Hi, Aleksey!

On Mar 26, Aleksey Midenkov wrote:

...

revision-id: 74b2eba1ca6 (mariadb-10.3.12-115-g74b2eba1ca6) parent(s): 2c0901d808b author: Aleksey Midenkov committer: Sergei Golubchik timestamp: 2019-03-26 17:43:38 +0100 message:

MDEV-15458 Segfault in heap_scan() upon UPDATE after ADD SYSTEM VERSIONING

* HA_EXTRA_REMEMBER_POS, HA_EXTRA_RESTORE_POS for HEAP storage engine * Versioning tests support

diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc index abe4254db3e..1d51947cc86 100644 --- a/sql/sql_insert.cc +++ b/sql/sql_insert.cc @@ -1643,7 +1643,12 @@ int vers_insert_history_row(TABLE *table) if (row_start->cmp(row_start->ptr, row_end->ptr) >= 0) return 0;

- return table->file->ha_write_row(table->record[0]); + int res; + if ((res= table->file->extra(HA_EXTRA_REMEMBER_POS))) + return res; + if ((res= table->file->ha_write_row(table->record[0]))) + return res; + return table->file->extra(HA_EXTRA_RESTORE_POS); }

Frankly, I don't like it. I mean, this particular fix is fine, but here's the problem:

We now have (at least) three features where the server might need to do something (insert/update/search) during the index/rnd scan.

And three different solutions: you use HA_EXTRA_REMEMBER_POS in system versioning, Nikita simply sets delete_while_scanning=false in application time period tables, and Sachin uses handler::clone() to create a second handler that can be used to check for hash collisions without disrupting the scan on the primary handler.

I think it's getting somewhat out of hands. Can we please reduce the number of approaches to fix the same issue?

What solution do you propose to use?

Regards, Sergei Chief Architect MariaDB and security@mariadb.org

-- All the best, Aleksey Midenkov @midenok