From 3cd2c1e8b6fa8435e634360c2ff63f5d645b65dc Mon Sep 17 00:00:00 2001 From: Nikita Malyavin Date: Wed, 17 Aug 2022 18:46:04 +0300 Subject: MDEV-29299 SELECT from table with vcol index reports warning As of now innodb does not store trx_id for each record in secondary index. The idea behind is following: let us store only per-page max_trx_id, and delete-mark the records when they are deleted/updated. If the read starts, it rememders the lowest id of currently active transaction. Innodb refers to it as trx->read_view->m_up_limit_id. See also ReadView::open. When the page is fetched, its max_trx_id is compared to m_up_limit_id. If the value is lower, and the secondary index record is not delete-marked, then this page is just safe to read as is. Else, a clustered index could be needed ato access. See page_get_max_trx_id call in row_search_mvcc, and the corresponding switch (row_search_idx_cond_check(...)) below. Virtual columns are required to be updated in case if the record was delete-marked. The motivation behind it is documented in Row_sel_get_clust_rec_for_mysql::operator() near row_sel_sec_rec_is_for_clust_rec call. This was basically a description why virtual column computation can normally happen during SELECT, and, generally, a vcol index access. Sometimes stats tables are updated by innodb. This starts a new transaction, and it can happen that it didn't finish to the moment of SELECT execution, forcing virtual columns recomputation. If the result was a something that normally outputs a warning, like division by zero, then it could be outputted in a racy manner. The solution is to suppress the warnings when a column is computed for the described purpose. ignore_wrnings argument is added innobase_get_computed_value. Currently, it is only true for a call from row_sel_sec_rec_is_for_clust_rec. --- sql/table.cc | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) (limited to 'sql/table.cc') diff --git a/sql/table.cc b/sql/table.cc index 5d91026963a..65b429a1b5a 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -8155,12 +8155,22 @@ int TABLE::update_virtual_fields(handler *h, enum_vcol_update_mode update_mode) DBUG_RETURN(in_use->is_error()); } -int TABLE::update_virtual_field(Field *vf) +/* + Calculate the virtual field value for a specified field. + @param vf A field to calculate + @param ignore_warnings Ignore calculation warnings. This usually + means that a calculation is internal and is + not expected to fail. +*/ +int TABLE::update_virtual_field(Field *vf, bool ignore_warnings) { DBUG_ENTER("TABLE::update_virtual_field"); Query_arena backup_arena; Counting_error_handler count_errors; + Suppress_warnings_error_handler warning_handler; in_use->push_internal_handler(&count_errors); + if (ignore_warnings) + in_use->push_internal_handler(&warning_handler); /* TODO: this may impose memory leak until table flush. See comment in @@ -8172,6 +8182,8 @@ int TABLE::update_virtual_field(Field *vf) vf->vcol_info->expr->save_in_field(vf, 0); in_use->restore_active_arena(expr_arena, &backup_arena); in_use->pop_internal_handler(); + if (ignore_warnings) + in_use->pop_internal_handler(); DBUG_RETURN(count_errors.errors); } -- cgit v1.2.1 From 128356b4b1cf3c8289ac1ba7e69a44d42e06e40c Mon Sep 17 00:00:00 2001 From: Nikita Malyavin Date: Mon, 10 Oct 2022 19:41:09 +0300 Subject: MDEV-29753 An error is wrongly reported during INSERT with vcol index See also commits aa8a31da and 64678c for a Bug #22990029 fix. In this scenario INSERT chose to check if delete unmarking is available for a just deleted record. To build an update vector, it needed to calculate the vcols as well. Since this INSERT was not IGNORE-flagged, recalculation failed. Solutiuon: temporarily set abort_on_warning=true, while calculating the column for delete-unmarked insert. --- sql/table.cc | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) (limited to 'sql/table.cc') diff --git a/sql/table.cc b/sql/table.cc index 65b429a1b5a..1d0edf88fb7 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -8158,9 +8158,10 @@ int TABLE::update_virtual_fields(handler *h, enum_vcol_update_mode update_mode) /* Calculate the virtual field value for a specified field. @param vf A field to calculate - @param ignore_warnings Ignore calculation warnings. This usually - means that a calculation is internal and is - not expected to fail. + @param ignore_warnings Ignore the warnings and also make the + calculations permissive. This usually means + that a calculation is internal and is not + expected to fail. */ int TABLE::update_virtual_field(Field *vf, bool ignore_warnings) { @@ -8169,8 +8170,13 @@ int TABLE::update_virtual_field(Field *vf, bool ignore_warnings) Counting_error_handler count_errors; Suppress_warnings_error_handler warning_handler; in_use->push_internal_handler(&count_errors); + bool abort_on_warning; if (ignore_warnings) + { + abort_on_warning= in_use->abort_on_warning; + in_use->abort_on_warning= false; in_use->push_internal_handler(&warning_handler); + } /* TODO: this may impose memory leak until table flush. See comment in @@ -8183,7 +8189,12 @@ int TABLE::update_virtual_field(Field *vf, bool ignore_warnings) in_use->restore_active_arena(expr_arena, &backup_arena); in_use->pop_internal_handler(); if (ignore_warnings) + { + in_use->abort_on_warning= abort_on_warning; in_use->pop_internal_handler(); + // This is an internal calculation, we expect it to always succeed + DBUG_ASSERT(count_errors.errors == 0); + } DBUG_RETURN(count_errors.errors); } -- cgit v1.2.1