maria-developers team mailing list archive

[Aleksey Midenkov <midenok@xxxxxxxxx>]

Hi, Sergei!

I tried variations of second and third options. The
gcol.gcol_keys_innodb depends on the return status and error code
thrown. It turns out that simplest solution is to use
Counting_error_handler alone.

The updated patch is

cd9cab54aac56f0f0171fa661fedffdbb4915edf (bb-10.2-midenok)

On Thu, Apr 23, 2020 at 2:48 PM Sergei Golubchik <serg@xxxxxxxxxxx> wrote:
>
> Hi, Aleksey!
>
> I don't see what you've changed. We've discussed that fix and that one
> isn't supposed to swap Diagnostics_area's like that. And in your new
> patch you do exactly the same.
>
> Possible correct approaches:
>
> * don't return in_use->is_error(), return the return value of
>   vf->vcol_info->expr->walk() || vf->vcol_info->expr->save_in_field()
>   This means that Item_field::update_vcol_processor() should also
>   do the same, I suspect
>
> * Use thd->push_internal_handler() and Counting_error_handler.
>   Or, better, Turn_errors_to_warnings_handler with counting.
>   This is the simplest one.
>
> there's a third option:
>
> * always return 0, because, looking how it's used, I don't really see
>   how update_virtual_field() can ever get an error. But it's not a
>   particularly future-proof approach. And I just might be wrong about
>   errors.
>
> On Apr 23, Aleksey Midenkov wrote:
> > revision-id: 7d593466a22 (mariadb-10.2.28-4-g7d593466a22)
> > parent(s): 7bc26de591c
> > author: Aleksey Midenkov <midenok@xxxxxxxxx>
> > committer: Aleksey Midenkov <midenok@xxxxxxxxx>
> > timestamp: 2019-11-07 10:45:21 +0300
> > message:
> >
> > MDEV-20015 Assertion `!in_use->is_error()' failed in TABLE::update_virtual_field
> >
> > Preserve and restore statement DA.
> >
> > update_virtual_field() is called as part of index rebuild in
> > ha_myisam::repair() (MDEV-5800) which is done on bulk INSERT finish.
> >
> > Assertion in update_virtual_field() was put as part of MDEV-16222
> > because update_virtual_field() returns in_use->is_error(). The idea:
> > wrongly mixed semantics of error status before update_virtual_field()
> > and the status returned by update_virtual_field(). The former can
> > falsely influence the latter.
> >
> > Preserve global error status and run update_virtual_field() with clear
> > DA since no matter how SQL command is finished it must update the
> > index after bulk INSERT.
> >
> Regards,
> Sergei
> VP of MariaDB Server Engineering
> and security@xxxxxxxxxxx



--
All the best,

Aleksey Midenkov
@midenok