From 7b9830480b75aa1d940635585807bdbc4d2dd2df Mon Sep 17 00:00:00 2001 From: Mayank Prasad Date: Mon, 5 Sep 2022 20:26:40 +0200 Subject: [PATCH] Bug#34558510 : undetectable problems after upgrade from 8.0.28, crash & corruption Background : When an upgraded table has INSTANT ADD columns all the new rows inserted will have all the instant default materialized and row version 0. When a row in above table is updated, if row is inserted before upgrade, we don't materialize instant columns. Reason for this is becuase earlier, INSTANT ADD column is allowed even if after adding that column, max size of a possible row may cross the row_size_limit. This creates an issue with the UPDATE/ROLLBACK. Let's say during UPDATE all INSTANT ADD COLS are materialized, updated row is fitting the row_size_limit. But if a rollback is done and INSTANT columns are kept materialised row may exceed row_size_limit. Thus ROLLBACK will fail. So for these rows, we keep the old behavior. NOTE [1] : Any new row inserted after upgrade will have all INSTANT columns materialized and have version=0. NOTE [2] : In new implementation an INSTANT ADD will fail if max possible size of a row crosses row_size_limit. So new rows with materialized INSTANT columns will always be witin row_size_limit. Issue : The check which decides if row is to be materialized or not wasn't sufficient and was considering NOTE [1] above wrongly to not to be materlized. NOTE : This issue is not caught (may be) because this "not considering a new row not to be materilized" impacts UPDATE only if last (trailing) INSTANT ADD column value is (or being changed to) it's instant default value Fix : Fix above check to consider NOTE [1]. Change-Id: Idd2580e16e4c62cdab33fde3ca70634a84ceab09 --- storage/innobase/btr/btr0cur.cc | 62 ++++++++++++++++++++++++------ storage/innobase/include/btr0cur.h | 6 +++ storage/innobase/lob/lob0lob.cc | 5 ++- storage/innobase/row/row0umod.cc | 4 +- 4 files changed, 64 insertions(+), 13 deletions(-) diff --git a/storage/innobase/btr/btr0cur.cc b/storage/innobase/btr/btr0cur.cc index 1e8e68cd1998..a69ce1b18d4a 100644 --- a/storage/innobase/btr/btr0cur.cc +++ b/storage/innobase/btr/btr0cur.cc @@ -3445,6 +3445,54 @@ dberr_t btr_cur_update_in_place(ulint flags, btr_cur_t *cursor, ulint *offsets, return (err); } +/** If default value of INSTANT ADD column is to be materialize in updated row. +@param[in] index record descriptor +@param[in] rec record +@return true if instant add column(s) to be materialized. */ +bool materialize_instant_default(const dict_index_t *index, const rec_t *rec) { +#ifdef UNIV_DEBUG + if (rec_new_is_versioned(rec)) { + uint8_t v = rec_get_instant_row_version_new(rec); + if (v == 0) { + ut_ad(index->has_instant_cols()); + } else { + ut_ad(index->has_row_versions()); + } + } +#endif + + /* For upgraded table with INSTANT ADD column done in previous implementaion, + don't materialize INSTANT columns for existing rows during update. The reason + is, earlier INSTANT ADD column is allowed even if after adding that column, + max size of a possible row may cross the row_size_limit. This creates an issue + with the UPDATE/ROLLBACK. Let's say during UPDATE all INSTANT ADD COLS are + materialized, updated row is fitting the row_size_limit. But if a rollback is + done and INSTANT columns are kept materialised row may exceed row_size_limit. + Thus ROLLBACK will fail. So for these rows, we keep the old behavior. + NOTE: Any new row inserted after upgrade will have all INSTANT columns + materialized and have version=0. + NOTE: In new implementation an INSTANT ADD will fail if max possible size of + a row crosses row_size_limit. So new rows with materialized INSTANT columns + will always be within row_size_limit. + Thus the above mentioned issue isn't there for rows inserted post upgrade. */ + + /* If the record is versioned, the old instant add defaults must already have + been materialized. */ + if (rec_new_is_versioned(rec)) { + return true; + } + + /* The record is not versioned. If the index has instant default from older + version, we must not materialize it. */ + if (index->has_instant_cols()) { + return false; + } + + /* The record is not versioned and the index doesn't have instant default from + older version. It is safe to materialize it. */ + return true; +} + dberr_t btr_cur_optimistic_update(ulint flags, btr_cur_t *cursor, ulint **offsets, mem_heap_t **heap, const upd_t *update, ulint cmpl_info, @@ -3541,17 +3589,13 @@ dberr_t btr_cur_optimistic_update(ulint flags, btr_cur_t *cursor, /* We checked above that there are no externally stored fields. */ ut_a(!new_entry->has_ext()); - /* For upgraded instant table, don't materialize instant default */ - bool materialize_instant_default = - !(index->has_instant_cols() && !index->has_row_versions()); - /* The page containing the clustered index record corresponding to new_entry is latched in mtr. Thus the following call is safe. */ row_upd_index_replace_new_col_vals_index_pos(new_entry, index, update, false, *heap); - if (!materialize_instant_default) { + if (!materialize_instant_default(index, rec)) { new_entry->ignore_trailing_default(index); } @@ -3806,10 +3850,6 @@ dberr_t btr_cur_pessimistic_update(ulint flags, btr_cur_t *cursor, rec = btr_cur_get_rec(cursor); - /* For upgraded instant table, don't materialize instant default */ - bool materialize_instant_default = - !(index->has_instant_cols() && !index->has_row_versions()); - *offsets = rec_get_offsets(rec, index, *offsets, ULINT_UNDEFINED, UT_LOCATION_HERE, offsets_heap); @@ -3825,7 +3865,7 @@ dberr_t btr_cur_pessimistic_update(ulint flags, btr_cur_t *cursor, row_upd_index_replace_new_col_vals_index_pos(new_entry, index, update, false, entry_heap); - if (!materialize_instant_default) { + if (!materialize_instant_default(index, rec)) { new_entry->ignore_trailing_default(index); } @@ -3891,7 +3931,7 @@ dberr_t btr_cur_pessimistic_update(ulint flags, btr_cur_t *cursor, /* During rollback of a record in table having INSTANT fields, it is possible to have an external field now which wasn't there before update */ - ut_ad(big_rec_vec == nullptr || index->table->has_row_versions()); + ut_ad(big_rec_vec == nullptr || materialize_instant_default(index, rec)); ut_ad(index->is_clustered()); ut_ad((flags & ~BTR_KEEP_POS_FLAG) == diff --git a/storage/innobase/include/btr0cur.h b/storage/innobase/include/btr0cur.h index 68483646161b..32dcb267689a 100644 --- a/storage/innobase/include/btr0cur.h +++ b/storage/innobase/include/btr0cur.h @@ -768,6 +768,12 @@ extern ulint btr_cur_n_sea_old; extern uint btr_cur_limit_optimistic_insert_debug; #endif /* UNIV_DEBUG */ +/** If default value of INSTANT ADD column is to be materialize in updated row. +@param[in] index record descriptor +@param[in] rec record +@return true if instant add column(s) to be materialized. */ +bool materialize_instant_default(const dict_index_t *index, const rec_t *rec); + #include "btr0cur.ic" #endif diff --git a/storage/innobase/lob/lob0lob.cc b/storage/innobase/lob/lob0lob.cc index 27224d81a8e4..4c71920eaca7 100644 --- a/storage/innobase/lob/lob0lob.cc +++ b/storage/innobase/lob/lob0lob.cc @@ -960,7 +960,10 @@ void BtrContext::free_updated_extern_fields(trx_id_t trx_id, undo_no_t undo_no, ulint i; ut_ad(rollback); - ut_ad(big_rec_vec == nullptr || m_index->has_row_versions()); +#ifdef UNIV_DEBUG + ut_ad(big_rec_vec == nullptr || materialize_instant_default(m_index, m_rec)); +#endif + ut_ad(rec_offs_validate()); ut_ad(mtr_is_page_fix(m_mtr, m_rec, MTR_MEMO_PAGE_X_FIX, m_index->table)); /* Assert that the cursor position and the record are matching. */ diff --git a/storage/innobase/row/row0umod.cc b/storage/innobase/row/row0umod.cc index 1747544a436d..169e6770b762 100644 --- a/storage/innobase/row/row0umod.cc +++ b/storage/innobase/row/row0umod.cc @@ -149,7 +149,9 @@ introduced where a call to log_free_check() is bypassed. */ btr_cur, offsets, offsets_heap, heap, &dummy_big_rec, node->update, node->cmpl_info, thr, thr_get_trx(thr)->id, node->undo_no, mtr, pcur); - ut_a(!dummy_big_rec || node->table->has_row_versions()); + const rec_t *rec = btr_cur_get_rec(btr_cur); + ut_a(!dummy_big_rec || materialize_instant_default(btr_cur->index, rec)); + if (dummy_big_rec) { ut_a(err == DB_SUCCESS);