Bug#37602657 virtual index corruption

Problem:
In some scenarios fields in secondary indexes based on virtual columns
become corrupted with random or invalid values.

Cause:
When an inplace update happens to a secondary index keyed on an virtual
column, the format of the update vector is confused - the field_no
member contains the actual field number, but some code wants it
to be the virtual column number.

The alternative meaning of field_no makes sense for clustered index,
which does not have fields for virtual columns. When for various
reasons we need the update vector to contain information about updates
to virtual column values for the row, we create "virtual field updates"
which do not actually update any fields.

Fix:
"Virtual field updates" with alternative semantics for field_no should
only be used for clustered index update vector. For updates to
materialized virtual column values in secondary indexes, we treat them
as regular field updates.

Change-Id: I6574d5df9ad0f6c527a8e020cb1eb44a87f88f2f

Author: Andrzej Jarzabek

storage/innobase/btr/btr0cur.cc

@@ -3137,6 +3137,7 @@ void btr_cur_update_in_place_log(ulint flags, const rec_t *rec,
   ut_d(const page_t *page = page_align(rec));
   ut_ad(flags < 256);
   ut_ad(page_is_comp(page) == dict_table_is_comp(index->table));
+  ut_d(update->validate_for_index(index));
 
   const bool opened = mlog_open_and_write_index(
       mtr, rec, index, MLOG_REC_UPDATE_IN_PLACE,
@@ -3220,7 +3221,7 @@ byte *btr_cur_parse_update_in_place(
 
   heap = mem_heap_create(256, UT_LOCATION_HERE);
 
-  ptr = row_upd_index_parse(ptr, end_ptr, heap, &update);
+  ptr = row_upd_index_parse(ptr, end_ptr, heap, &update, index);
 
   if (!ptr || !page) {
     goto func_exit;
@@ -3527,6 +3528,8 @@ dberr_t btr_cur_optimistic_update(ulint flags, btr_cur_t *cursor,
         thr_get_trx(thr)->id == trx_id);
   ut_ad(fil_page_index_page_check(page));
   ut_ad(btr_page_get_index_id(page) == index->id);
+  ut_ad(update);
+  ut_d(update->validate_for_index(index));
 
   DBUG_EXECUTE_IF("DB_ZIP_OVERFLOW_on_btr_cur_optimistic_update",
                   return (DB_ZIP_OVERFLOW););
@@ -3724,7 +3727,8 @@ dberr_t btr_cur_optimistic_update(ulint flags, btr_cur_t *cursor,
     btr_cur_prefetch_siblings(block);
   }
 
-  return (err);
+  ut_d(update->validate_for_index(index));
+  return err;
 }
 
 /** If, in a split, a new supremum record was created as the predecessor of the
@@ -3816,6 +3820,7 @@ dberr_t btr_cur_pessimistic_update(ulint flags, btr_cur_t *cursor,
             (BTR_NO_UNDO_LOG_FLAG | BTR_NO_LOCKING_FLAG | BTR_CREATE_FLAG |
              BTR_KEEP_SYS_FLAG) ||
         thr_get_trx(thr)->id == trx_id);
+  ut_d(update->validate_for_index(index));
 
   err = optim_err = btr_cur_optimistic_update(
       flags | BTR_KEEP_IBUF_BITMAP, cursor, offsets, offsets_heap, update,
@@ -4161,6 +4166,7 @@ dberr_t btr_cur_pessimistic_update(ulint flags, btr_cur_t *cursor,
 
   *big_rec = big_rec_vec;
 
+  ut_d(update->validate_for_index(index));
   return err;
 }
 

storage/innobase/data/data0data.cc

@@ -779,6 +779,32 @@ std::ostream &big_rec_t::print(std::ostream &out) const {
   out << "]";
   return (out);
 }
+
+void dtuple_t::validate() const {
+  for (uint16_t i = 0; i < n_fields; i++) {
+    const dfield_t &field = fields[i];
+    /* All fields in the fields vector represent actual fields in the
+    record, so their DATA_VIRTUAL flag is cleared regardless of
+    whether they are defined for virtual columns. */
+    ut_a((field.type.prtype & DATA_VIRTUAL) == 0U);
+  }
+  for (uint16_t i = 0; i < n_v_fields; i++) {
+    const dfield_t &field = v_fields[i];
+    /* All fields in the v_fields vector represent fields not present
+    in the record, and as such have their DATA_VIRTUAL flag set. */
+    ut_a((field.type.prtype & DATA_VIRTUAL) == DATA_VIRTUAL);
+  }
+}
+
+void dtuple_t::validate_for_index(const dict_index_t *index) const {
+  validate();
+  if (!index->is_clustered()) {
+    /* Only tuples for clustered index may have virtual fields.
+    Secondary index fields based on virtual columns are not
+    considered virtual. */
+    ut_a(n_v_fields == 0);
+  }
+}
 #endif /* UNIV_DEBUG */
 
 trx_id_t dtuple_t::get_trx_id() const {

storage/innobase/dict/dict0dict.cc

@@ -2893,7 +2893,10 @@ void dict_index_copy_types(dtuple_t *tuple, const dict_index_t *index,
 
     ifield = index->get_field(i);
     dfield_type = dfield_get_type(dtuple_get_nth_field(tuple, i));
+    ut_ad(!ifield->col->is_virtual() || !index->is_clustered());
     ifield->col->copy_type(dfield_type);
+    /* Field for materialized virtual column is not itself virtual. */
+    dfield_type->prtype &= ~DATA_VIRTUAL;
     if (dict_index_is_spatial(index) &&
         DATA_GEOMETRY_MTYPE(dfield_type->mtype)) {
       dfield_type->prtype |= DATA_GIS_MBR;
@@ -5296,7 +5299,7 @@ upd_t *DDTableBuffer::update_set_metadata(const dtuple_t *entry,
   dfield_copy(&upd_field->new_val, metadata_dfield);
   upd_field_set_field_no(upd_field, METADATA_FIELD_NO, m_index);
 
-  ut_ad(update->validate());
+  ut_d(update->validate());
 
   return (update);
 }

storage/innobase/handler/ha_innodb.cc

@@ -9315,10 +9315,11 @@ static void innobase_get_multi_value_and_diff(
 }
 
 /** Checks which fields have changed in a row and stores information
- of them to an update vector.
+ of them to an update vector for the table's clustered index.
  @return DB_SUCCESS or error code */
 static dberr_t calc_row_difference(
-    upd_t *uvect,             /*!< in/out: update vector */
+    upd_t *uvect,             /*!< in/out: update vector for the
+                              clustered index */
     const uchar *old_row,     /*!< in: old row in MySQL format */
     uchar *new_row,           /*!< in: new row in MySQL format */
     TABLE *table,             /*!< in: table in MySQL data
@@ -9701,7 +9702,7 @@ static dberr_t calc_row_difference(
       if (changes_fts_column && !changes_fts_doc_col) {
         ib::warn(ER_IB_MSG_559) << "A new Doc ID must be supplied"
                                    " while updating FTS indexed columns.";
-        return (DB_FTS_INVALID_DOCID);
+        return DB_FTS_INVALID_DOCID;
       }
 
       /* Doc ID must monotonically increase */
@@ -9711,7 +9712,7 @@ static dberr_t calc_row_difference(
                                 << innodb_table->fts->cache->next_doc_id - 1
                                 << " for table " << innodb_table->name;
 
-        return (DB_FTS_INVALID_DOCID);
+        return DB_FTS_INVALID_DOCID;
       } else if ((doc_id - prebuilt->table->fts->cache->next_doc_id) >=
                  FTS_DOC_ID_MAX_STEP) {
         ib::warn(ER_IB_MSG_561)
@@ -9750,8 +9751,8 @@ static dberr_t calc_row_difference(
 
   ut_a(buf <= (byte *)original_upd_buff + buff_len);
 
-  ut_ad(uvect->validate());
-  return (DB_SUCCESS);
+  ut_d(uvect->validate_for_index(clust_index));
+  return DB_SUCCESS;
 }
 
 /**

storage/innobase/ibuf/ibuf0ibuf.cc

@@ -3505,6 +3505,7 @@ static void ibuf_insert_to_index_page(
   ut_ad(!dict_index_is_online_ddl(index));  // this is an ibuf_dummy index
   ut_ad(ibuf_inside(mtr));
   ut_ad(dtuple_check_typed(entry));
+  ut_d(entry->validate_for_index(index));
   /* A change buffer merge must occur before users are granted
   any access to the page. No adaptive hash index entries may
   point to a freshly read page. */

storage/innobase/include/data0data.h

@@ -793,6 +793,11 @@ struct dtuple_t {
     }
     return false;
   }
+
+#ifdef UNIV_DEBUG
+  void validate() const;
+  void validate_for_index(const dict_index_t *index) const;
+#endif /* UNIV_DEBUG */
 };
 
 /** A slot for a field in a big rec vector */

storage/innobase/include/dict0mem.h

@@ -776,7 +776,7 @@ struct dict_col_t {
     ut_ad(type);
 
     ut_ad(mtype == type->mtype);
-    ut_ad(prtype == type->prtype);
+    ut_ad((prtype | DATA_VIRTUAL) == (type->prtype | DATA_VIRTUAL));
     // ut_ad(col->len == type->len);
 #ifndef UNIV_HOTBACKUP
     ut_ad(mbminmaxlen == type->mbminmaxlen);

storage/innobase/include/row0upd.h

@@ -298,11 +298,11 @@ static inline bool row_upd_changes_ord_field_binary(
 }
 
 /** Checks if an FTS indexed column is affected by an UPDATE.
+ @param[in]  table      table
+ @param[in]  upd_field  clustered index field update
  @return offset within fts_t::indexes if FTS indexed column updated else
  ULINT_UNDEFINED */
-ulint row_upd_changes_fts_column(
-    dict_table_t *table,     /*!< in: table */
-    upd_field_t *upd_field); /*!< in: field to check */
+ulint row_upd_changes_fts_column(dict_table_t *table, upd_field_t *upd_field);
 /** Checks if an FTS Doc ID column is affected by an UPDATE.
  @return whether Doc ID column is affected */
 [[nodiscard]] bool row_upd_changes_doc_id(
@@ -350,12 +350,15 @@ void row_upd_rec_sys_fields_in_recovery(rec_t *rec, page_zip_des_t *page_zip,
                                         trx_id_t trx_id, roll_ptr_t roll_ptr);
 
 /** Parses the log data written by row_upd_index_write_log.
+ @param[in]   ptr         buffer
+ @param[in]   end_ptr     buffer end
+ @param[in]   heap        memory heap where update vector is built
+ @param[out]  update_out  update vector
+ @param[in]   index       the index corresponding to the update
  @return log data end or NULL */
-byte *row_upd_index_parse(const byte *ptr,     /*!< in: buffer */
-                          const byte *end_ptr, /*!< in: buffer end */
-                          mem_heap_t *heap,    /*!< in: memory heap where update
-                                               vector is    built */
-                          upd_t **update_out); /*!< out: update vector */
+byte *row_upd_index_parse(const byte *ptr, const byte *end_ptr,
+                          mem_heap_t *heap, upd_t **update_out,
+                          dict_index_t *index);
 
 /** Get the new autoinc counter from the update vector when there is
 an autoinc field defined in this table.
@@ -554,11 +557,6 @@ inline std::ostream &operator<<(std::ostream &out, const upd_field_t &obj) {
   return (obj.print(out));
 }
 
-/* check whether an update field is on virtual column */
-inline bool upd_fld_is_virtual_col(const upd_field_t *upd_fld) {
-  return upd_fld->is_virtual();
-}
-
 /* check whether an update field is on multi-value virtual column */
 inline bool upd_fld_is_multi_value_col(const upd_field_t *upd_fld) {
   return dfield_is_multi_value(&upd_fld->new_val);
@@ -626,14 +624,26 @@ struct upd_t {
   }
 
 #ifdef UNIV_DEBUG
-  bool validate() const {
+  void validate() const {
     for (ulint i = 0; i < n_fields; ++i) {
       dfield_t *field = &fields[i].new_val;
       if (dfield_is_ext(field)) {
         ut_ad(dfield_get_len(field) >= BTR_EXTERN_FIELD_REF_SIZE);
       }
     }
-    return (true);
+  }
+
+  void validate_for_index(const dict_index_t *index) const {
+    validate();
+    for (ulint i = 0; i < n_fields; ++i) {
+      const upd_field_t &field = fields[i];
+      ut_a(index->is_clustered() || !field.is_virtual());
+      if (!field.is_virtual()) {
+        ut_a(field.field_no < dict_index_get_n_fields(index));
+      } else {
+        ut_a(field.field_no < index->table->n_v_cols);
+      }
+    }
   }
 #endif  // UNIV_DEBUG
 

storage/innobase/include/row0upd.ic

@@ -93,8 +93,6 @@ inline void upd_field_set_field_no(upd_field_t *upd_field, ulint field_no,
         << index->n_fields << " fields";
     ut_d(ut_error);
   }
-
-  index->get_col(field_no)->copy_type(dfield_get_type(&upd_field->new_val));
 }
 
 inline void upd_field_set_v_field_no(upd_field_t *upd_field, ulint field_no,
@@ -121,7 +119,7 @@ inline const upd_field_t *upd_get_field_by_field_no(const upd_t *update,
     const upd_field_t *uf = upd_get_nth_field(update, i);
 
     /* matches only if the field matches that of is_virtual */
-    if ((!is_virtual) != (!upd_fld_is_virtual_col(uf))) {
+    if ((!is_virtual) != (!uf->is_virtual())) {
       continue;
     }
 

storage/innobase/pars/pars0pars.cc

@@ -1047,12 +1047,13 @@ static void pars_process_assign_list(upd_node_t *node) /*!< in: update node */
 
     col_sym = assign_node->col;
 
-    upd_field_set_field_no(upd_field, clust_index->get_col_pos(col_sym->col_no),
+    const dict_col_t *const col = node->table->get_col(col_sym->col_no);
+    upd_field_set_field_no(upd_field, dict_col_get_clust_pos(col, clust_index),
                            clust_index);
     upd_field->exp = assign_node->val;
+    col->copy_type(dfield_get_type(&upd_field->new_val));
 
-    if (!clust_index->get_col(upd_field->field_no)
-             ->get_fixed_size(dict_table_is_comp(node->table))) {
+    if (!col->get_fixed_size(dict_table_is_comp(node->table))) {
       changes_field_size = 0;
     }