maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

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