From 1562b2c20b5b76ee305942238e3e1e66f9952e29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Wed, 5 Oct 2022 09:30:33 +0300 Subject: [PATCH] MDEV-29666 InnoDB fails to purge secondary index records when indexed virtual columns exist row_purge_get_partial(): Replaces trx_undo_rec_get_partial_row(). Also copy the purge_node_t::ref to the purge_node_t::row. In this way, the clustered index key fields will always be available, even if thanks to commit d384ead0f024995787b1f29bc672c33b0d3d40a8 (MDEV-14799) they would no longer be repeated in the remaining part of the undo log record. --- .../gcol/r/innodb_virtual_debug_purge.result | 3 - .../gcol/t/innodb_virtual_debug_purge.test | 3 - .../suite/innodb/r/purge_secondary.result | 15 ++ .../suite/innodb/t/purge_secondary.test | 32 ++++ storage/innobase/include/trx0rec.h | 26 +-- storage/innobase/row/row0purge.cc | 135 +++++++++++++- storage/innobase/trx/trx0rec.cc | 176 ------------------ 7 files changed, 178 insertions(+), 212 deletions(-) diff --git a/mysql-test/suite/gcol/r/innodb_virtual_debug_purge.result b/mysql-test/suite/gcol/r/innodb_virtual_debug_purge.result index 3ef3679db75..5767aaf63ca 100644 --- a/mysql-test/suite/gcol/r/innodb_virtual_debug_purge.result +++ b/mysql-test/suite/gcol/r/innodb_virtual_debug_purge.result @@ -148,10 +148,7 @@ lock table t write; connection prevent_purge; commit; connection default; -InnoDB 0 transactions not purged disconnect lock_table; -start transaction with consistent snapshot; -commit; InnoDB 0 transactions not purged set global debug_dbug=@old_dbug; drop table t; diff --git a/mysql-test/suite/gcol/t/innodb_virtual_debug_purge.test b/mysql-test/suite/gcol/t/innodb_virtual_debug_purge.test index da203f118a8..20db2cd5533 100644 --- a/mysql-test/suite/gcol/t/innodb_virtual_debug_purge.test +++ b/mysql-test/suite/gcol/t/innodb_virtual_debug_purge.test @@ -189,10 +189,7 @@ lock table t write; connection prevent_purge; commit; connection default; ---source ../../innodb/include/wait_all_purged.inc disconnect lock_table; -start transaction with consistent snapshot; -commit; --source ../../innodb/include/wait_all_purged.inc set global debug_dbug=@old_dbug; drop table t; diff --git a/mysql-test/suite/innodb/r/purge_secondary.result b/mysql-test/suite/innodb/r/purge_secondary.result index 8f20f5baacb..66aaeb9e1a1 100644 --- a/mysql-test/suite/innodb/r/purge_secondary.result +++ b/mysql-test/suite/innodb/r/purge_secondary.result @@ -166,4 +166,19 @@ buffer_LRU_batch_evict_total_pages buffer # FLUSH TABLES t1 FOR EXPORT; # UNLOCK TABLES; DROP TABLE t1; +# +# MDEV-29666 InnoDB fails to purge secondary index records +# when indexed virtual columns exist +# +CREATE TABLE t1 (a INT, b INT, a1 INT AS(a) VIRTUAL, +INDEX(a1),INDEX(b)) ENGINE=InnoDB; +INSERT INTO t1 SET a=1, b=1; +UPDATE t1 SET a=2, b=3; +InnoDB 0 transactions not purged +FLUSH TABLE t1 FOR EXPORT; +page 4: N_RECS=0x0001 +page 5: N_RECS=0x0001 +UNLOCK TABLES; +DROP TABLE t1; +# End of 10.3 tests SET GLOBAL innodb_purge_rseg_truncate_frequency = @saved_frequency; diff --git a/mysql-test/suite/innodb/t/purge_secondary.test b/mysql-test/suite/innodb/t/purge_secondary.test index bf702b6b737..530da2c33d9 100644 --- a/mysql-test/suite/innodb/t/purge_secondary.test +++ b/mysql-test/suite/innodb/t/purge_secondary.test @@ -149,4 +149,36 @@ WHERE NAME="buffer_LRU_batch_evict_total_pages" AND COUNT > 0; DROP TABLE t1; +--echo # +--echo # MDEV-29666 InnoDB fails to purge secondary index records +--echo # when indexed virtual columns exist +--echo # + +CREATE TABLE t1 (a INT, b INT, a1 INT AS(a) VIRTUAL, +INDEX(a1),INDEX(b)) ENGINE=InnoDB; +INSERT INTO t1 SET a=1, b=1; + +UPDATE t1 SET a=2, b=3; +let DATADIR=`select @@datadir`; +let PAGE_SIZE=`select @@innodb_page_size`; + +source include/wait_all_purged.inc; +FLUSH TABLE t1 FOR EXPORT; + +perl; +my $ps = $ENV{PAGE_SIZE}; +my $file = "$ENV{DATADIR}/test/t1.ibd"; +open(FILE, "<", $file) or die "Unable to open $file\n"; +die "Unable to read $file\n" unless + sysread(FILE, $_, 6*$ps) == 6*$ps; +close(FILE); +print "page 4: N_RECS=0x", unpack("H*", substr($_, 4 * $ps + 54, 2)), "\n"; +print "page 5: N_RECS=0x", unpack("H*", substr($_, 5 * $ps + 54, 2)), "\n"; +EOF + +UNLOCK TABLES; +DROP TABLE t1; + +--echo # End of 10.3 tests + SET GLOBAL innodb_purge_rseg_truncate_frequency = @saved_frequency; diff --git a/storage/innobase/include/trx0rec.h b/storage/innobase/include/trx0rec.h index fa084ff274c..992e7645bc7 100644 --- a/storage/innobase/include/trx0rec.h +++ b/storage/innobase/include/trx0rec.h @@ -1,7 +1,7 @@ /***************************************************************************** Copyright (c) 1996, 2016, Oracle and/or its affiliates. All Rights Reserved. -Copyright (c) 2017, 2019, MariaDB Corporation. +Copyright (c) 2017, 2022, MariaDB Corporation. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software @@ -136,30 +136,6 @@ trx_undo_update_rec_get_update( mem_heap_t* heap, /*!< in: memory heap from which the memory needed is allocated */ upd_t** upd); /*!< out, own: update vector */ -/*******************************************************************//** -Builds a partial row from an update undo log record, for purge. -It contains the columns which occur as ordering in any index of the table. -Any missing columns are indicated by col->mtype == DATA_MISSING. -@return pointer to remaining part of undo record */ -byte* -trx_undo_rec_get_partial_row( -/*=========================*/ - const byte* ptr, /*!< in: remaining part in update undo log - record of a suitable type, at the start of - the stored index columns; - NOTE that this copy of the undo log record must - be preserved as long as the partial row is - used, as we do NOT copy the data in the - record! */ - dict_index_t* index, /*!< in: clustered index */ - const upd_t* update, /*!< in: updated columns */ - dtuple_t** row, /*!< out, own: partial row */ - ibool ignore_prefix, /*!< in: flag to indicate if we - expect blob prefixes in undo. Used - only in the assertion. */ - mem_heap_t* heap) /*!< in: memory heap from which the memory - needed is allocated */ - MY_ATTRIBUTE((nonnull, warn_unused_result)); /** Report a RENAME TABLE operation. @param[in,out] trx transaction @param[in] table table that is being renamed diff --git a/storage/innobase/row/row0purge.cc b/storage/innobase/row/row0purge.cc index e46d515e8c2..77e772786d5 100644 --- a/storage/innobase/row/row0purge.cc +++ b/storage/innobase/row/row0purge.cc @@ -1,7 +1,7 @@ /***************************************************************************** Copyright (c) 1997, 2017, Oracle and/or its affiliates. All Rights Reserved. -Copyright (c) 2017, 2021, MariaDB Corporation. +Copyright (c) 2017, 2022, MariaDB Corporation. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software @@ -1004,6 +1004,134 @@ skip_secondaries: row_purge_upd_exist_or_extern_func(node,undo_rec) #endif /* UNIV_DEBUG */ +/** Build a partial row from an update undo log record for purge. +Any columns which occur as ordering in any index of the table are present. +Any missing columns are indicated by col->mtype == DATA_MISSING. + +@param ptr remaining part of the undo log record +@param index clustered index +@param node purge node +@return pointer to remaining part of undo record */ +static byte *row_purge_get_partial(const byte *ptr, const dict_index_t &index, + purge_node_t *node) +{ + bool first_v_col= true; + bool is_undo_log= true; + + ut_ad(index.is_primary()); + ut_ad(index.n_uniq == node->ref->n_fields); + + node->row= dtuple_create_with_vcol(node->heap, index.table->n_cols, + index.table->n_v_cols); + + /* Mark all columns in the row uninitialized, so that + we can distinguish missing fields from fields that are SQL NULL. */ + for (ulint i= 0; i < index.table->n_cols; i++) + node->row->fields[i].type.mtype= DATA_MISSING; + + dtuple_init_v_fld(node->row); + + for (const upd_field_t *uf= node->update->fields, *const ue= + node->update->fields + node->update->n_fields; uf != ue; uf++) + if (!uf->old_v_val) + node->row->fields[dict_index_get_nth_col(&index, uf->field_no)->ind]= + uf->new_val; + + const byte *end_ptr= ptr + mach_read_from_2(ptr); + ptr+= 2; + + while (ptr != end_ptr) + { + dfield_t *dfield; + const byte *field; + const dict_col_t *col; + ulint len; + ulint orig_len; + + ulint field_no= mach_read_next_compressed(&ptr); + + if (field_no >= REC_MAX_N_FIELDS) + { + ptr= trx_undo_read_v_idx(index.table, ptr, first_v_col, &is_undo_log, + &field_no); + first_v_col= false; + + ptr= trx_undo_rec_get_col_val(ptr, &field, &len, &orig_len); + + if (field_no == ULINT_UNDEFINED) + continue; /* there no longer is an index on the virtual column */ + + dict_v_col_t *vcol= dict_table_get_nth_v_col(index.table, field_no); + col =&vcol->m_col; + dfield= dtuple_get_nth_v_field(node->row, vcol->v_pos); + dict_col_copy_type(&vcol->m_col, &dfield->type); + } + else + { + ptr= trx_undo_rec_get_col_val(ptr, &field, &len, &orig_len); + col= dict_index_get_nth_col(&index, field_no); + dfield= dtuple_get_nth_field(node->row, col->ind); + ut_ad(dfield->type.mtype == DATA_MISSING || + dict_col_type_assert_equal(col, &dfield->type)); + ut_ad(dfield->type.mtype == DATA_MISSING || + dfield->len == len || + (len != UNIV_SQL_NULL && len >= UNIV_EXTERN_STORAGE_FIELD)); + dict_col_copy_type(dict_table_get_nth_col(index.table, col->ind), + &dfield->type); + } + + dfield_set_data(dfield, field, len); + + if (len == UNIV_SQL_NULL || len < UNIV_EXTERN_STORAGE_FIELD) + continue; + + spatial_status_t spatial_status= static_cast + ((len & SPATIAL_STATUS_MASK) >> SPATIAL_STATUS_SHIFT); + len&= ~SPATIAL_STATUS_MASK; + + /* Keep compatible with 5.7.9 format. */ + if (spatial_status == SPATIAL_UNKNOWN) + spatial_status= dict_col_get_spatial_status(col); + + switch (UNIV_EXPECT(spatial_status, SPATIAL_NONE)) { + case SPATIAL_ONLY: + ut_ad(len - UNIV_EXTERN_STORAGE_FIELD == DATA_MBR_LEN); + dfield_set_len(dfield, len - UNIV_EXTERN_STORAGE_FIELD); + break; + + case SPATIAL_MIXED: + dfield_set_len(dfield, len - UNIV_EXTERN_STORAGE_FIELD - DATA_MBR_LEN); + break; + + default: + dfield_set_len(dfield, len - UNIV_EXTERN_STORAGE_FIELD); + break; + } + + dfield_set_ext(dfield); + dfield_set_spatial_status(dfield, spatial_status); + + if (!col->ord_part || spatial_status == SPATIAL_ONLY || + node->rec_type == TRX_UNDO_UPD_DEL_REC) + continue; + /* If the prefix of this BLOB column is indexed, ensure that enough + prefix is stored in the undo log record. */ + ut_a(dfield_get_len(dfield) >= BTR_EXTERN_FIELD_REF_SIZE); + ut_a(dict_table_has_atomic_blobs(index.table) || + dfield_get_len(dfield) >= + REC_ANTELOPE_MAX_INDEX_COL_LEN + BTR_EXTERN_FIELD_REF_SIZE); + } + + for (ulint i= 0; i < index.n_uniq; i++) + { + dfield_t &field= node->row->fields[index.fields[i].col->ind]; + if (field.type.mtype == DATA_MISSING) + field= node->ref->fields[i]; + } + + return const_cast(ptr); +} + /***********************************************************//** Parses the row reference and other info in a modify undo log record. @return true if purge operation required */ @@ -1153,10 +1281,7 @@ err_exit: if (!(node->cmpl_info & UPD_NODE_NO_ORD_CHANGE)) { ut_ad(!(node->update->info_bits & REC_INFO_MIN_REC_FLAG)); - ptr = trx_undo_rec_get_partial_row( - ptr, clust_index, node->update, &node->row, - type == TRX_UNDO_UPD_DEL_REC, - node->heap); + ptr = row_purge_get_partial(ptr, *clust_index, node); } else if (node->update->info_bits & REC_INFO_MIN_REC_FLAG) { node->ref = &trx_undo_metadata; } diff --git a/storage/innobase/trx/trx0rec.cc b/storage/innobase/trx/trx0rec.cc index d7ec2a38f19..e25ebc3e22a 100644 --- a/storage/innobase/trx/trx0rec.cc +++ b/storage/innobase/trx/trx0rec.cc @@ -1655,182 +1655,6 @@ trx_undo_update_rec_get_update( return(const_cast(ptr)); } -/*******************************************************************//** -Builds a partial row from an update undo log record, for purge. -It contains the columns which occur as ordering in any index of the table. -Any missing columns are indicated by col->mtype == DATA_MISSING. -@return pointer to remaining part of undo record */ -byte* -trx_undo_rec_get_partial_row( -/*=========================*/ - const byte* ptr, /*!< in: remaining part in update undo log - record of a suitable type, at the start of - the stored index columns; - NOTE that this copy of the undo log record must - be preserved as long as the partial row is - used, as we do NOT copy the data in the - record! */ - dict_index_t* index, /*!< in: clustered index */ - const upd_t* update, /*!< in: updated columns */ - dtuple_t** row, /*!< out, own: partial row */ - ibool ignore_prefix, /*!< in: flag to indicate if we - expect blob prefixes in undo. Used - only in the assertion. */ - mem_heap_t* heap) /*!< in: memory heap from which the memory - needed is allocated */ -{ - const byte* end_ptr; - bool first_v_col = true; - bool is_undo_log = true; - - ut_ad(index->is_primary()); - - *row = dtuple_create_with_vcol( - heap, dict_table_get_n_cols(index->table), - dict_table_get_n_v_cols(index->table)); - - /* Mark all columns in the row uninitialized, so that - we can distinguish missing fields from fields that are SQL NULL. */ - for (ulint i = 0; i < dict_table_get_n_cols(index->table); i++) { - dfield_get_type(dtuple_get_nth_field(*row, i)) - ->mtype = DATA_MISSING; - } - - dtuple_init_v_fld(*row); - - for (const upd_field_t* uf = update->fields, * const ue - = update->fields + update->n_fields; - uf != ue; uf++) { - if (uf->old_v_val) { - continue; - } - ulint c = dict_index_get_nth_col(index, uf->field_no)->ind; - *dtuple_get_nth_field(*row, c) = uf->new_val; - } - - end_ptr = ptr + mach_read_from_2(ptr); - ptr += 2; - - while (ptr != end_ptr) { - dfield_t* dfield; - const byte* field; - ulint field_no; - const dict_col_t* col; - ulint col_no; - ulint len; - ulint orig_len; - bool is_virtual; - - field_no = mach_read_next_compressed(&ptr); - - is_virtual = (field_no >= REC_MAX_N_FIELDS); - - if (is_virtual) { - ptr = trx_undo_read_v_idx( - index->table, ptr, first_v_col, &is_undo_log, - &field_no); - first_v_col = false; - } - - ptr = trx_undo_rec_get_col_val(ptr, &field, &len, &orig_len); - - /* This column could be dropped or no longer indexed */ - if (field_no == ULINT_UNDEFINED) { - ut_ad(is_virtual); - continue; - } - - if (is_virtual) { - dict_v_col_t* vcol = dict_table_get_nth_v_col( - index->table, field_no); - col = &vcol->m_col; - col_no = dict_col_get_no(col); - dfield = dtuple_get_nth_v_field(*row, vcol->v_pos); - dict_col_copy_type( - &vcol->m_col, - dfield_get_type(dfield)); - } else { - col = dict_index_get_nth_col(index, field_no); - col_no = dict_col_get_no(col); - dfield = dtuple_get_nth_field(*row, col_no); - ut_ad(dfield->type.mtype == DATA_MISSING - || dict_col_type_assert_equal(col, - &dfield->type)); - ut_ad(dfield->type.mtype == DATA_MISSING - || dfield->len == len - || (len != UNIV_SQL_NULL - && len >= UNIV_EXTERN_STORAGE_FIELD)); - dict_col_copy_type( - dict_table_get_nth_col(index->table, col_no), - dfield_get_type(dfield)); - } - - dfield_set_data(dfield, field, len); - - if (len != UNIV_SQL_NULL - && len >= UNIV_EXTERN_STORAGE_FIELD) { - spatial_status_t spatial_status; - - /* Decode spatial status. */ - spatial_status = static_cast( - (len & SPATIAL_STATUS_MASK) - >> SPATIAL_STATUS_SHIFT); - len &= ~SPATIAL_STATUS_MASK; - - /* Keep compatible with 5.7.9 format. */ - if (spatial_status == SPATIAL_UNKNOWN) { - spatial_status = - dict_col_get_spatial_status(col); - } - - switch (spatial_status) { - case SPATIAL_ONLY: - ut_ad(len - UNIV_EXTERN_STORAGE_FIELD - == DATA_MBR_LEN); - dfield_set_len( - dfield, - len - UNIV_EXTERN_STORAGE_FIELD); - break; - - case SPATIAL_MIXED: - dfield_set_len( - dfield, - len - UNIV_EXTERN_STORAGE_FIELD - - DATA_MBR_LEN); - break; - - case SPATIAL_NONE: - dfield_set_len( - dfield, - len - UNIV_EXTERN_STORAGE_FIELD); - break; - - case SPATIAL_UNKNOWN: - ut_ad(0); - break; - } - - dfield_set_ext(dfield); - dfield_set_spatial_status(dfield, spatial_status); - - /* If the prefix of this column is indexed, - ensure that enough prefix is stored in the - undo log record. */ - if (!ignore_prefix && col->ord_part - && spatial_status != SPATIAL_ONLY) { - ut_a(dfield_get_len(dfield) - >= BTR_EXTERN_FIELD_REF_SIZE); - ut_a(dict_table_has_atomic_blobs(index->table) - || dfield_get_len(dfield) - >= REC_ANTELOPE_MAX_INDEX_COL_LEN - + BTR_EXTERN_FIELD_REF_SIZE); - } - } - } - - return(const_cast(ptr)); -} - /** Erase the unused undo log page end. @param[in,out] undo_page undo log page @return whether the page contained something */