MDEV-23547 InnoDB: Failing assertion: *len in row_upd_ext_fetch

This bug was originally repeated on 10.4 after defining a UNIQUE KEY
on a TEXT column, which is implemented by MDEV-371 by creating the
index on a hidden virtual column.

While row_vers_vc_matches_cluster() is executing in a purge thread
to find out if an index entry may be removed in a secondary index
that comprises a virtual column, another purge thread may process
the undo log record that this check is interested in, and write
a null BLOB pointer in that record. This would trip the assertion.

To prevent this from occurring, we must propagate the 'missing BLOB'
error up the call stack.

row_upd_ext_fetch(): Return NULL when the error occurs.

row_upd_index_replace_new_col_val(): Return whether the previous
version was built successfully.

row_upd_index_replace_new_col_vals_index_pos(): Check the error
result. Yes, we would intentionally crash on this error if it
occurs outside the purge thread.

row_upd_index_replace_new_col_vals(): Check for the error condition,
and simplify the logic.

trx_undo_prev_version_build(): Check for the error condition.

3 files changed, 78 insertions, 85 deletions

diff --git a/storage/innobase/include/row0upd.h b/storage/innobase/include/row0upd.h
index 04701042658..b60770b01fb 100644
--- a/storage/innobase/include/row0upd.h
+++ b/storage/innobase/include/row0upd.h
@@ -257,24 +257,18 @@ row_upd_index_replace_new_col_vals_index_pos(
 	mem_heap_t*	heap)	/*!< in: memory heap for allocating and
 				copying the new values */
 	MY_ATTRIBUTE((nonnull));
-/***********************************************************//**
-Replaces the new column values stored in the update vector to the index entry
-given. */
-void
-row_upd_index_replace_new_col_vals(
-/*===============================*/
-	dtuple_t*	entry,	/*!< in/out: index entry where replaced;
-				the clustered index record must be
-				covered by a lock or a page latch to
-				prevent deletion (rollback or purge) */
-	dict_index_t*	index,	/*!< in: index; NOTE that this may also be a
-				non-clustered index */
-	const upd_t*	update,	/*!< in: an update vector built for the
-				CLUSTERED index so that the field number in
-				an upd_field is the clustered index position */
-	mem_heap_t*	heap)	/*!< in: memory heap for allocating and
-				copying the new values */
-	MY_ATTRIBUTE((nonnull));
+/** Replace the new column values stored in the update vector,
+during trx_undo_prev_version_build().
+@param entry   clustered index tuple where the values are replaced
+               (the clustered index leaf page latch must be held)
+@param index   clustered index
+@param update  update vector for the clustered index
+@param heap    memory heap for allocating and copying values
+@return whether the previous version was built successfully */
+bool
+row_upd_index_replace_new_col_vals(dtuple_t *entry, const dict_index_t &index,
+                                   const upd_t *update, mem_heap_t *heap)
+  MY_ATTRIBUTE((nonnull, warn_unused_result));
 /***********************************************************//**
 Replaces the new column values stored in the update vector. */
 void
diff --git a/storage/innobase/row/row0upd.cc b/storage/innobase/row/row0upd.cc
index 800c7a6d1f3..6d210cfd6bb 100644
--- a/storage/innobase/row/row0upd.cc
+++ b/storage/innobase/row/row0upd.cc
@@ -1206,7 +1206,9 @@ containing also the reference to the external part
 @param[in,out]	len		input - length of prefix to
 fetch; output: fetched length of the prefix
 @param[in,out]	heap		heap where to allocate
-@return BLOB prefix */
+@return BLOB prefix
+@retval NULL if the record is incomplete (should only happen
+in row_vers_vc_matches_cluster() executed concurrently with another purge) */
 static
 byte*
 row_upd_ext_fetch(
@@ -1221,10 +1223,7 @@ row_upd_ext_fetch(
 	*len = btr_copy_externally_stored_field_prefix(
 		buf, *len, page_size, data, local_len);
 
-	/* We should never update records containing a half-deleted BLOB. */
-	ut_a(*len);
-
-	return(buf);
+	return *len ? buf : NULL;
 }
 
 /** Replaces the new column value stored in the update vector in
@@ -1235,9 +1234,11 @@ the given index entry field.
 @param[in]	uf		update field
 @param[in,out]	heap		memory heap for allocating and copying
 the new value
-@param[in]	page_size	page size */
+@param[in]	page_size	page size
+@return whether the previous version was built successfully */
+MY_ATTRIBUTE((nonnull, warn_unused_result))
 static
-void
+bool
 row_upd_index_replace_new_col_val(
 	dfield_t*		dfield,
 	const dict_field_t*	field,
@@ -1252,7 +1253,7 @@ row_upd_index_replace_new_col_val(
 	dfield_copy_data(dfield, &uf->new_val);
 
 	if (dfield_is_null(dfield)) {
-		return;
+		return true;
 	}
 
 	len = dfield_get_len(dfield);
@@ -1270,6 +1271,9 @@ row_upd_index_replace_new_col_val(
 
 			data = row_upd_ext_fetch(data, l, page_size,
 						 &len, heap);
+			if (UNIV_UNLIKELY(!data)) {
+				return false;
+			}
 		}
 
 		len = dtype_get_at_most_n_mbchars(col->prtype,
@@ -1283,7 +1287,7 @@ row_upd_index_replace_new_col_val(
 			dfield_dup(dfield, heap);
 		}
 
-		return;
+		return true;
 	}
 
 	switch (uf->orig_len) {
@@ -1322,6 +1326,8 @@ row_upd_index_replace_new_col_val(
 		dfield_set_ext(dfield);
 		break;
 	}
+
+	return true;
 }
 
 /***********************************************************//**
@@ -1379,68 +1385,57 @@ row_upd_index_replace_new_col_vals_index_pos(
 				update, i, false);
 		}
 
-		if (uf) {
-			row_upd_index_replace_new_col_val(
-				dtuple_get_nth_field(entry, i),
-				field, col, uf, heap, page_size);
+		if (uf && UNIV_UNLIKELY(!row_upd_index_replace_new_col_val(
+						dtuple_get_nth_field(entry, i),
+						field, col, uf, heap,
+						page_size))) {
+			ut_error;
 		}
 	}
 }
 
-/***********************************************************//**
-Replaces the new column values stored in the update vector to the index entry
-given. */
-void
-row_upd_index_replace_new_col_vals(
-/*===============================*/
-	dtuple_t*	entry,	/*!< in/out: index entry where replaced;
-				the clustered index record must be
-				covered by a lock or a page latch to
-				prevent deletion (rollback or purge) */
-	dict_index_t*	index,	/*!< in: index; NOTE that this may also be a
-				non-clustered index */
-	const upd_t*	update,	/*!< in: an update vector built for the
-				CLUSTERED index so that the field number in
-				an upd_field is the clustered index position */
-	mem_heap_t*	heap)	/*!< in: memory heap for allocating and
-				copying the new values */
+/** Replace the new column values stored in the update vector,
+during trx_undo_prev_version_build().
+@param entry   clustered index tuple where the values are replaced
+               (the clustered index leaf page latch must be held)
+@param index   clustered index
+@param update  update vector for the clustered index
+@param heap    memory heap for allocating and copying values
+@return whether the previous version was built successfully */
+bool
+row_upd_index_replace_new_col_vals(dtuple_t *entry, const dict_index_t &index,
+                                   const upd_t *update, mem_heap_t *heap)
 {
-	ulint			i;
-	const dict_index_t*	clust_index
-		= dict_table_get_first_index(index->table);
-	const page_size_t&	page_size = dict_table_page_size(index->table);
-
-	ut_ad(!index->table->skip_alter_undo);
-
-	dtuple_set_info_bits(entry, update->info_bits);
-
-	for (i = 0; i < dict_index_get_n_fields(index); i++) {
-		const dict_field_t*	field;
-		const dict_col_t*	col;
-		const upd_field_t*	uf;
-
-		field = dict_index_get_nth_field(index, i);
-		col = dict_field_get_col(field);
-		if (dict_col_is_virtual(col)) {
-			const dict_v_col_t*	vcol = reinterpret_cast<
-							const dict_v_col_t*>(
-								col);
-
-			uf = upd_get_field_by_field_no(
-				update, vcol->v_pos, true);
-		} else {
-			uf = upd_get_field_by_field_no(
-				update,
-				dict_col_get_clust_pos(col, clust_index),
-				false);
-		}
-
-		if (uf) {
-			row_upd_index_replace_new_col_val(
-				dtuple_get_nth_field(entry, i),
-				field, col, uf, heap, page_size);
-		}
-	}
+  ut_ad(index.is_primary());
+  const page_size_t& page_size= dict_table_page_size(index.table);
+
+  ut_ad(!index.table->skip_alter_undo);
+  dtuple_set_info_bits(entry, update->info_bits);
+
+  for (ulint i= 0; i < index.n_fields; i++)
+  {
+   const dict_field_t *field= &index.fields[i];
+   const dict_col_t* col= dict_field_get_col(field);
+   const upd_field_t *uf;
+
+   if (col->is_virtual())
+   {
+     const dict_v_col_t *vcol= reinterpret_cast<const dict_v_col_t*>(col);
+     uf= upd_get_field_by_field_no(update, vcol->v_pos, true);
+   }
+   else
+     uf= upd_get_field_by_field_no(update, dict_col_get_clust_pos(col, &index),
+                                   false);
+
+   if (!uf)
+     continue;
+
+   if (!row_upd_index_replace_new_col_val(dtuple_get_nth_field(entry, i),
+                                          field, col, uf, heap, page_size))
+     return false;
+  }
+
+  return true;
 }
 
 /** Replaces the virtual column values stored in the update vector.
diff --git a/storage/innobase/trx/trx0rec.cc b/storage/innobase/trx/trx0rec.cc
index ee022b4f1fd..623d8990381 100644
--- a/storage/innobase/trx/trx0rec.cc
+++ b/storage/innobase/trx/trx0rec.cc
@@ -2446,7 +2446,11 @@ trx_undo_prev_version_build(
 		/* The page containing the clustered index record
 		corresponding to entry is latched in mtr.  Thus the
 		following call is safe. */
-		row_upd_index_replace_new_col_vals(entry, index, update, heap);
+		if (!row_upd_index_replace_new_col_vals(entry, *index, update,
+							heap)) {
+			ut_a(v_status & TRX_UNDO_PREV_IN_PURGE);
+			return false;
+		}
 
 		/* Get number of externally stored columns in updated record */
 		const ulint n_ext = dtuple_get_n_ext(entry);