Re: [Maria-developers] 7d593466a22: MDEV-20015 Assertion `!in_use->is_error()' failed in TABLE::update_virtual_field
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@gmail.com> committer: Aleksey Midenkov <midenok@gmail.com> 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@mariadb.org
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@mariadb.org> 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@gmail.com> committer: Aleksey Midenkov <midenok@gmail.com> 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@mariadb.org
-- All the best, Aleksey Midenkov @midenok
participants (2)
-
Aleksey Midenkov -
Sergei Golubchik