diff options
author | Nikita Malyavin <nikitamalyavin@gmail.com> | 2022-08-17 18:46:04 +0300 |
---|---|---|
committer | Nikita Malyavin <nikitamalyavin@gmail.com> | 2022-10-12 20:49:45 +0300 |
commit | 3cd2c1e8b6fa8435e634360c2ff63f5d645b65dc (patch) | |
tree | 9cb684504929ed314b124e1d1c4ddb0f90bbe818 | |
parent | 4fec99a2ba6034592d273d918402540d3d4fe772 (diff) | |
download | mariadb-git-3cd2c1e8b6fa8435e634360c2ff63f5d645b65dc.tar.gz |
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.
-rw-r--r-- | mysql-test/suite/gcol/r/innodb_virtual_index.result | 31 | ||||
-rw-r--r-- | mysql-test/suite/gcol/t/innodb_virtual_index.test | 29 | ||||
-rw-r--r-- | sql/sql_class.h | 15 | ||||
-rw-r--r-- | sql/table.cc | 14 | ||||
-rw-r--r-- | sql/table.h | 2 | ||||
-rw-r--r-- | storage/innobase/handler/ha_innodb.cc | 7 | ||||
-rw-r--r-- | storage/innobase/include/row0mysql.h | 9 | ||||
-rw-r--r-- | storage/innobase/row/row0sel.cc | 3 | ||||
-rw-r--r-- | storage/mroonga/mrn_mysql_compat.h | 2 | ||||
-rw-r--r-- | storage/myisam/ha_myisam.cc | 2 |
10 files changed, 104 insertions, 10 deletions
diff --git a/mysql-test/suite/gcol/r/innodb_virtual_index.result b/mysql-test/suite/gcol/r/innodb_virtual_index.result index e63d47391c9..d5ad08cd4bf 100644 --- a/mysql-test/suite/gcol/r/innodb_virtual_index.result +++ b/mysql-test/suite/gcol/r/innodb_virtual_index.result @@ -1,3 +1,4 @@ +SET default_storage_engine= innodb; SET @saved_frequency = @@GLOBAL.innodb_purge_rseg_truncate_frequency; SET GLOBAL innodb_purge_rseg_truncate_frequency = 1; # @@ -310,3 +311,33 @@ ALTER TABLE t1 ADD KEY (b), ALGORITHM=INPLACE; # Cleanup DROP TABLE t1; # End of 10.2 tests +# +# MDEV-29299 SELECT from table with vcol index reports warning +# +CREATE TABLE t(fld1 INT NOT NULL, +fld2 INT AS (100/fld1) VIRTUAL, +KEY(fld1), KEY(fld2)); +CREATE TABLE t_odd(id int); +INSERT INTO t(fld1) VALUES(1), (2); +connect stop_purge,localhost,root; +START TRANSACTION WITH CONSISTENT SNAPSHOT; +INSERT INTO t_odd VALUES(10000); +connection default; +UPDATE IGNORE t SET fld1= 3 WHERE fld1= 2; +UPDATE IGNORE t SET fld1= 4 WHERE fld1= 3; +UPDATE IGNORE t SET fld1= 0 WHERE fld1= 4; +Warnings: +Warning 1365 Division by 0 +SELECT fld2 FROM t FORCE INDEX(fld2); +fld2 +NULL +100 +SELECT fld2 FROM t FORCE INDEX(fld1); +fld2 +100 +NULL +Warnings: +Warning 1365 Division by 0 +disconnect stop_purge; +DROP TABLE t, t_odd; +# End of 10.3 tests diff --git a/mysql-test/suite/gcol/t/innodb_virtual_index.test b/mysql-test/suite/gcol/t/innodb_virtual_index.test index 46ffadcdd8c..10a7ac51562 100644 --- a/mysql-test/suite/gcol/t/innodb_virtual_index.test +++ b/mysql-test/suite/gcol/t/innodb_virtual_index.test @@ -1,6 +1,8 @@ --source include/have_innodb.inc --source include/have_sequence.inc +SET default_storage_engine= innodb; + # Ensure that the history list length will actually be decremented by purge. SET @saved_frequency = @@GLOBAL.innodb_purge_rseg_truncate_frequency; SET GLOBAL innodb_purge_rseg_truncate_frequency = 1; @@ -334,3 +336,30 @@ DROP TABLE t1; --echo # End of 10.2 tests +--echo # +--echo # MDEV-29299 SELECT from table with vcol index reports warning +--echo # + +CREATE TABLE t(fld1 INT NOT NULL, + fld2 INT AS (100/fld1) VIRTUAL, + KEY(fld1), KEY(fld2)); +CREATE TABLE t_odd(id int); +INSERT INTO t(fld1) VALUES(1), (2); + +--connect stop_purge,localhost,root +# This prevents purge for records in t +START TRANSACTION WITH CONSISTENT SNAPSHOT; +INSERT INTO t_odd VALUES(10000); + +--connection default +UPDATE IGNORE t SET fld1= 3 WHERE fld1= 2; +UPDATE IGNORE t SET fld1= 4 WHERE fld1= 3; +UPDATE IGNORE t SET fld1= 0 WHERE fld1= 4; +SELECT fld2 FROM t FORCE INDEX(fld2); +SELECT fld2 FROM t FORCE INDEX(fld1); + +--disconnect stop_purge +DROP TABLE t, t_odd; + + +--echo # End of 10.3 tests diff --git a/sql/sql_class.h b/sql/sql_class.h index 7e06f0b0903..4d1f5b40db5 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -1856,6 +1856,21 @@ private: }; +struct Suppress_warnings_error_handler : public Internal_error_handler +{ + bool handle_condition(THD *thd, + uint sql_errno, + const char *sqlstate, + Sql_condition::enum_warning_level *level, + const char *msg, + Sql_condition **cond_hdl) + { + return *level == Sql_condition::WARN_LEVEL_WARN; + } +}; + + + /** Tables that were locked with LOCK TABLES statement. 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); } diff --git a/sql/table.h b/sql/table.h index 1eb09f119ed..577c11c37ee 100644 --- a/sql/table.h +++ b/sql/table.h @@ -1610,7 +1610,7 @@ public: uint actual_n_key_parts(KEY *keyinfo); ulong actual_key_flags(KEY *keyinfo); - int update_virtual_field(Field *vf); + int update_virtual_field(Field *vf, bool ignore_warnings); int update_virtual_fields(handler *h, enum_vcol_update_mode update_mode); int update_default_fields(bool ignore_errors); void evaluate_update_default_function(); diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index 9ce1915b2cd..0fe60ccabe6 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -21138,7 +21138,8 @@ innobase_get_computed_value( TABLE* mysql_table, byte* mysql_rec, const dict_table_t* old_table, - const upd_t* update) + const upd_t* update, + bool ignore_warnings) { byte rec_buf2[REC_VERSION_56_MAX_INDEX_COL_LEN]; byte* buf; @@ -21236,7 +21237,9 @@ innobase_get_computed_value( MY_BITMAP *old_write_set = dbug_tmp_use_all_columns(mysql_table, &mysql_table->write_set); MY_BITMAP *old_read_set = dbug_tmp_use_all_columns(mysql_table, &mysql_table->read_set); - ret = mysql_table->update_virtual_field(mysql_table->field[col->m_col.ind]); + ret = mysql_table->update_virtual_field( + mysql_table->field[col->m_col.ind], + ignore_warnings); dbug_tmp_restore_column_map(&mysql_table->read_set, old_read_set); dbug_tmp_restore_column_map(&mysql_table->write_set, old_write_set); diff --git a/storage/innobase/include/row0mysql.h b/storage/innobase/include/row0mysql.h index 3e5a7a77333..07f0f56093c 100644 --- a/storage/innobase/include/row0mysql.h +++ b/storage/innobase/include/row0mysql.h @@ -916,7 +916,9 @@ void innobase_report_computed_value_failed(dtuple_t *row); @param[in] old_table during ALTER TABLE, this is the old table or NULL. @param[in] update update vector for the parent row -@param[in] foreign foreign key information +@param[in] ignore_warnings ignore warnings during calculation. Usually + means that a calculation is internal and + should have no side effects. @return the field filled with computed value */ dfield_t* innobase_get_computed_value( @@ -929,8 +931,9 @@ innobase_get_computed_value( THD* thd, TABLE* mysql_table, byte* mysql_rec, - const dict_table_t* old_table, - const upd_t* update); + const dict_table_t* old_table=NULL, + const upd_t* update=NULL, + bool ignore_warnings=false); /** Get the computed value by supplying the base column values. @param[in,out] table the table whose virtual column diff --git a/storage/innobase/row/row0sel.cc b/storage/innobase/row/row0sel.cc index d9b517309c6..ff0f9d47892 100644 --- a/storage/innobase/row/row0sel.cc +++ b/storage/innobase/row/row0sel.cc @@ -329,7 +329,8 @@ row_sel_sec_rec_is_for_clust_rec( &heap, NULL, NULL, thr_get_trx(thr)->mysql_thd, thr->prebuilt->m_mysql_table, - record, NULL, NULL); + record, NULL, NULL, + true); if (vfield == NULL) { innobase_report_computed_value_failed(row); diff --git a/storage/mroonga/mrn_mysql_compat.h b/storage/mroonga/mrn_mysql_compat.h index bdb15637e31..ae1feffb6b8 100644 --- a/storage/mroonga/mrn_mysql_compat.h +++ b/storage/mroonga/mrn_mysql_compat.h @@ -481,7 +481,7 @@ #ifdef MRN_MARIADB_P # if (MYSQL_VERSION_ID >= 100203) # define MRN_GENERATED_COLUMNS_UPDATE_VIRTUAL_FIELD(table, field) \ - (table->update_virtual_field(field)) + (table->update_virtual_field(field,false)) # else # define MRN_GENERATED_COLUMNS_UPDATE_VIRTUAL_FIELD(table, field) \ (field->vcol_info->expr_item->save_in_field(field, 0)) diff --git a/storage/myisam/ha_myisam.cc b/storage/myisam/ha_myisam.cc index d43ba828957..dfb2cc86ec1 100644 --- a/storage/myisam/ha_myisam.cc +++ b/storage/myisam/ha_myisam.cc @@ -681,7 +681,7 @@ static int compute_vcols(MI_INFO *info, uchar *record, int keynum) { Field *f= table->field[kp->fieldnr - 1]; if (f->vcol_info) - table->update_virtual_field(f); + table->update_virtual_field(f, false); } return 0; } |