← Back to team overview

maria-developers team mailing list archive

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

 

I personally like this one more then mine. BTW I also clone the handler to
do something similar in WITHOUT OVERLAPS. Maybe I should also consider to
rewrite it to HA_EXTRA_REMEMBER_POS

On Wed, 27 Mar 2019 at 20:37, Aleksey Midenkov <midenok@xxxxxxxxx> wrote:

> Hi, Sergei!
>
> On Wed, Mar 27, 2019 at 1:04 AM Sergei Golubchik <serg@xxxxxxxxxxx> wrote:
>
>> Hi, Aleksey!
>>
>> On Mar 26, Aleksey Midenkov wrote:
>> > revision-id: 74b2eba1ca6 (mariadb-10.3.12-115-g74b2eba1ca6)
>> > parent(s): 2c0901d808b
>> > author: Aleksey Midenkov <midenok@xxxxxxxxx>
>> > committer: Sergei Golubchik <serg@xxxxxxxxxxx>
>> > 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@xxxxxxxxxxx
>>
>
>
> --
> All the best,
>
> Aleksey Midenkov
> @midenok
>


-- 
Yours truly,
Nikita Malyavin

References