List:Commits« Previous MessageNext Message »
From:marko.makela Date:June 12 2012 9:50am
Subject:bzr push into mysql-trunk-wl6255 branch (marko.makela:3963 to 3966) WL#6255
View as plain text  
 3966 Marko Mäkelä	2012-06-12
      WL#6255 Portability fix. Use ut_snprintf instead of my_snprintf(),
      so that UINT64PF will work as desired on Windows platforms.

    modified:
      storage/innobase/handler/handler0alter.cc
 3965 Marko Mäkelä	2012-06-12
      WL#6255 BLOB bug fix (part 1 of 2).
      
      When rebuilding a table online, check if the creating transaction of the
      record was rolled back, while holding an S-latch on the original clustered
      index tree of the table. Holding the tree latch will prevent other threads
      from freeing any off-page columns.
      
      TODO: When creating a secondary index online, prevent purge from freeing
      externally stored columns, and keep track on rolled-back inserts.
      
      row_log_table_apply_convert_mrec(): Add the parameter trx_id for performing
      the row_log_table_is_rollback() check. Return NULL and *error=DB_SUCCESS
      if the record has been rolled back. If the record contains off-page columns,
      S-latch the clustered index of the original table.
      
      row_log_table_apply_insert(), row_log_table_apply_update(): Add the
      parameter trx_id for performing the row_log_table_is_rollback() check.
      Allow row_log_table_apply_convert_mrec() to return NULL when there is
      no error, but the transaction has been rolled back.
      
      row_log_table_apply_op(): Remove the row_log_table_is_rollback() calls.
      
      row_merge_insert_index_tuples(): When the record contains off-page columns,
      protect the BLOB fetch by clustered index S-latch when rebuilding table,
      and by re-checking if the transaction was rolled back.

    modified:
      storage/innobase/row/row0log.cc
      storage/innobase/row/row0merge.cc
 3964 Marko Mäkelä	2012-06-12
      Fix WL#6255 rb:1105 Problem 15.
      
      innobase_col_to_mysql(): Skip DATA_BLOB fields.
      The pointer can become invalid by the time it is dereferenced
      in handler::print_keydup_error().
      
      innobase_rec_to_mysql(): Skip externally stored fields.

    modified:
      storage/innobase/handler/handler0alter.cc
 3963 Marko Mäkelä	2012-06-11
      Fix WL#6255 rb:1105 Problem 14.
      
      row_log_table_apply_op(): Invoke rec_offs_set_n_fields() before
      recomputing offsets. This omission caused offsets to be computed on
      an index prefix only.
      
      row_log_table_apply_ops(): Allocate enough space for offsets, in case
      new_index->n_uniq + 2 exceeds index->n_fields.

    modified:
      storage/innobase/row/row0log.cc
=== modified file 'storage/innobase/handler/handler0alter.cc'
--- a/storage/innobase/handler/handler0alter.cc	revid:marko.makela@stripped6xlzvx
+++ b/storage/innobase/handler/handler0alter.cc	revid:marko.makela@stripped9v
@@ -990,10 +990,9 @@ innobase_col_to_mysql(
 		break;
 
 	case DATA_BLOB:
-		/* Store a pointer to the BLOB buffer to dest: the BLOB was
-		already copied to the buffer in row_sel_store_mysql_rec */
-
-		row_mysql_store_blob_ref(dest, flen, data, len);
+		/* Skip MySQL BLOBs when reporting an erroneous row
+		during index creation or table rebuild. */
+		field->set_null();
 		break;
 
 #ifdef UNIV_DEBUG
@@ -1068,7 +1067,8 @@ innobase_rec_to_mysql(
 
 		ipos = dict_index_get_nth_col_pos(index, col_no);
 
-		if (UNIV_UNLIKELY(ipos == ULINT_UNDEFINED)) {
+		if (ipos == ULINT_UNDEFINED
+		    || rec_offs_nth_extern(offsets, ipos)) {
 null_field:
 			field->set_null();
 			continue;
@@ -1814,7 +1814,7 @@ innobase_create_temporary_tablename(
 
 	char*	name = static_cast<char*>(mem_heap_alloc(heap, size));
 	memcpy(name, dbtab, dblen);
-	my_snprintf(name + dblen, size - dblen,
+	ut_snprintf(name + dblen, size - dblen,
 		    tmp_file_prefix "-ib" UINT64PF, id);
 	return(name);
 }

=== modified file 'storage/innobase/row/row0log.cc'
--- a/storage/innobase/row/row0log.cc	revid:marko.makela@strippedzvx
+++ b/storage/innobase/row/row0log.cc	revid:marko.makela@stripped
@@ -1022,21 +1022,28 @@ row_log_table_is_rollback(
 
 /******************************************************//**
 Converts a log record to a table row.
-@return converted row, or NULL if the conversion fails */
+@return converted row, or NULL if the conversion fails
+or the transaction has been rolled back */
 static __attribute__((nonnull, warn_unused_result))
 const dtuple_t*
 row_log_table_apply_convert_mrec(
 /*=============================*/
 	const mrec_t*		mrec,		/*!< in: merge record */
-	const dict_index_t*	index,		/*!< in: index of mrec */
+	dict_index_t*		index,		/*!< in: index of mrec */
 	const ulint*		offsets,	/*!< in: offsets of mrec */
 	const row_log_t*	log,		/*!< in: rebuild context */
 	mem_heap_t*		heap,		/*!< in/out: memory heap */
+	trx_id_t		trx_id,		/*!< in: DB_TRX_ID of mrec */
 	dberr_t*		error)		/*!< out: DB_SUCCESS or
 						reason of failure */
 {
 	dtuple_t*	row;
 
+	if (row_log_table_is_rollback(index, trx_id)) {
+		row = NULL;
+		goto func_exit;
+	}
+
 	/* This is based on row_build(). */
 	if (log->add_cols) {
 		row = dtuple_copy(log->add_cols, heap);
@@ -1051,6 +1058,15 @@ row_log_table_apply_convert_mrec(
 		dict_table_copy_types(row, log->table);
 	}
 
+	if (rec_offs_any_extern(offsets)) {
+		rw_lock_s_lock(&index->lock);
+	}
+
+	if (row_log_table_is_rollback(index, trx_id)) {
+		row = NULL;
+		goto skip_row;
+	}
+
 	for (ulint i = 0; i < rec_offs_n_fields(offsets); i++) {
 		const dict_field_t*	ind_field
 			= dict_index_get_nth_field(index, i);
@@ -1081,6 +1097,7 @@ row_log_table_apply_convert_mrec(
 		const void*		data;
 
 		if (rec_offs_nth_extern(offsets, i)) {
+			ut_ad(rec_offs_any_extern(offsets));
 			data = btr_rec_copy_externally_stored_field(
 				mrec, offsets,
 				dict_table_zip_size(index->table),
@@ -1120,6 +1137,12 @@ row_log_table_apply_convert_mrec(
 						 dfield_get_type(dfield)));
 	}
 
+skip_row:
+	if (rec_offs_any_extern(offsets)) {
+		rw_lock_s_unlock(&index->lock);
+	}
+
+func_exit:
 	*error = DB_SUCCESS;
 	return(row);
 }
@@ -1204,20 +1227,21 @@ dberr_t
 row_log_table_apply_insert(
 /*=======================*/
 	que_thr_t*		thr,		/*!< in: query graph */
-	const mrec_t*		mrec,		/*!< in: merge record */
+	const mrec_t*		mrec,		/*!< in: record to insert */
 	const ulint*		offsets,	/*!< in: offsets of mrec */
 	mem_heap_t*		offsets_heap,	/*!< in/out: memory heap
 						that can be emptied */
 	mem_heap_t*		heap,		/*!< in/out: memory heap */
-	row_merge_dup_t*	dup)		/*!< in/out: for reporting
+	row_merge_dup_t*	dup,		/*!< in/out: for reporting
 						duplicate key errors */
+	trx_id_t		trx_id)		/*!< in: DB_TRX_ID of mrec */
 {
 	const row_log_t*log	= dup->index->online_log;
 	dberr_t		error;
 	const dtuple_t*	row	= row_log_table_apply_convert_mrec(
-		mrec, dup->index, offsets, log, heap, &error);
+		mrec, dup->index, offsets, log, heap, trx_id, &error);
 
-	ut_ad(!row == (error != DB_SUCCESS));
+	ut_ad(error == DB_SUCCESS || !row);
 
 	if (row) {
 		error = row_log_table_apply_insert_low(
@@ -1431,13 +1455,14 @@ row_log_table_apply_update(
 	ulint			new_trx_id_col,	/*!< in: position of
 						DB_TRX_ID in the new
 						clustered index */
-	const mrec_t*		mrec,		/*!< in: merge record */
+	const mrec_t*		mrec,		/*!< in: new value */
 	const ulint*		offsets,	/*!< in: offsets of mrec */
 	mem_heap_t*		offsets_heap,	/*!< in/out: memory heap
 						that can be emptied */
 	mem_heap_t*		heap,		/*!< in/out: memory heap */
 	row_merge_dup_t*	dup,		/*!< in/out: for reporting
 						duplicate key errors */
+	trx_id_t		trx_id,		/*!< in: DB_TRX_ID of mrec */
 	const dtuple_t*		old_pk)		/*!< in: PRIMARY KEY and
 						DB_TRX_ID,DB_ROLL_PTR
 						of the old value,
@@ -1457,9 +1482,9 @@ row_log_table_apply_update(
 	      + (dup->index->online_log->same_pk ? 0 : 2));
 
 	row = row_log_table_apply_convert_mrec(
-		mrec, dup->index, offsets, log, heap, &error);
+		mrec, dup->index, offsets, log, heap, trx_id, &error);
 
-	ut_ad(!row == (error != DB_SUCCESS));
+	ut_ad(error == DB_SUCCESS || !row);
 
 	if (!row) {
 		goto err_exit;
@@ -1491,8 +1516,8 @@ insert:
 		error = row_log_table_apply_insert_low(
 			thr, row, offsets_heap, heap, log->table, dup);
 
-		if (error != DB_SUCCESS) {
 err_exit:
+		if (error != DB_SUCCESS) {
 			/* Report the erroneous row using the old
 			version of the table. */
 			innobase_rec_to_mysql(
@@ -1763,13 +1788,9 @@ row_log_table_apply_op(
 				= rec_get_nth_field(
 					mrec, offsets, trx_id_col, &len);
 			ut_ad(len == DATA_TRX_ID_LEN);
-			if (!row_log_table_is_rollback(
-				    dup->index,
-				    trx_read_trx_id(db_trx_id))) {
-				*error = row_log_table_apply_insert(
-					thr, mrec, offsets, offsets_heap,
-					heap, dup);
-			}
+			*error = row_log_table_apply_insert(
+				thr, mrec, offsets, offsets_heap,
+				heap, dup, trx_read_trx_id(db_trx_id));
 		}
 		break;
 
@@ -1934,14 +1955,10 @@ row_log_table_apply_op(
 				= rec_get_nth_field(
 					mrec, offsets, trx_id_col, &len);
 			ut_ad(len == DATA_TRX_ID_LEN);
-			if (!row_log_table_is_rollback(
-				    dup->index,
-				    trx_read_trx_id(db_trx_id))) {
-				*error = row_log_table_apply_update(
-					thr, trx_id_col, new_trx_id_col,
-					mrec, offsets, offsets_heap,
-					heap, dup, old_pk);
-			}
+			*error = row_log_table_apply_update(
+				thr, trx_id_col, new_trx_id_col,
+				mrec, offsets, offsets_heap,
+				heap, dup, trx_read_trx_id(db_trx_id), old_pk);
 		}
 
 		break;

=== modified file 'storage/innobase/row/row0merge.cc'
--- a/storage/innobase/row/row0merge.cc	revid:marko.makela@oracle.com-20120611200051-u6u5pzolfh6xlzvx
+++ b/storage/innobase/row/row0merge.cc	revid:marko.makela@stripped12094948-hypff3db1fz8rj9v
@@ -2312,10 +2312,43 @@ row_merge_insert_index_tuples(
 			}
 
 			if (n_ext) {
-				row_merge_copy_blobs(
-					mrec, offsets,
-					dict_table_zip_size(old_table),
-					dtuple, tuple_heap);
+				dict_index_t*	old_index
+					= dict_table_get_first_index(
+						old_table);
+
+				if (dict_index_is_online_ddl(old_index)) {
+					/* Copy the off-page columns while
+					holding old_index->lock, so
+					that they cannot be freed by
+					a rollback of a fresh insert. */
+					rw_lock_s_lock(&old_index->lock);
+
+					if (row_merge_skip_rec(
+						    mrec, index, old_index,
+						    offsets)) {
+						// Rollback: skip the record.
+						rw_lock_s_unlock(
+							&old_index->lock);
+						continue;
+					}
+
+					row_merge_copy_blobs(
+						mrec, offsets,
+						dict_table_zip_size(old_table),
+						dtuple, tuple_heap);
+
+					rw_lock_s_unlock(&old_index->lock);
+				} else {
+					/* TODO: Is this safe when creating
+					a secondary index online? We are not
+					suspending purge or preventing the
+					rollback of inserts. Thus, the data
+					could have been freed. */
+					row_merge_copy_blobs(
+						mrec, offsets,
+						dict_table_zip_size(old_table),
+						dtuple, tuple_heap);
+				}
 			}
 
 			ut_ad(dtuple_validate(dtuple));

No bundle (reason: useless for push emails).
Thread
bzr push into mysql-trunk-wl6255 branch (marko.makela:3963 to 3966) WL#6255marko.makela12 Jun