branches/zip: Merge revisions 4359:4400 from branches/5.1:

  ------------------------------------------------------------------------
  r4399 | marko | 2009-03-12 09:38:05 +0200 (Thu, 12 Mar 2009) | 2 lines

  branches/5.1: row_sel_get_clust_rec_for_mysql(): Store the cursor position
  also for unlock_row().  (Bug #39320)
  ------------------------------------------------------------------------
  r4400 | marko | 2009-03-12 10:06:44 +0200 (Thu, 12 Mar 2009) | 5 lines

  branches/5.1: Fix a bug in multi-table semi-consistent reads.
  Remember the acquired record locks per table handle (row_prebuilt_t)
  rather than per transaction (trx_t), so that unlock_row should successfully
  unlock all non-matching rows in multi-table operations.
  This deficiency was found while investigating Bug #39320.
  ------------------------------------------------------------------------

These were submitted as rb://94 and rb://96 and approved by Heikki Tuuri.

include/row0mysql.h

@@ -695,6 +695,21 @@ struct row_prebuilt_struct {
 					This eliminates lock waits in some
 					cases; note that this breaks
 					serializability. */
+	ulint		new_rec_locks;	/* normally 0; if
+					srv_locks_unsafe_for_binlog is
+					TRUE or session is using READ
+					COMMITTED isolation level, in a
+					cursor search, if we set a new
+					record lock on an index, this is
+					incremented; this is used in
+					releasing the locks under the
+					cursors if we are performing an
+					UPDATE and we determine after
+					retrieving the row that it does
+					not need to be locked; thus,
+					these can be used to implement a
+					'mini-rollback' that releases
+					the latest record locks */
 	ulint		mysql_prefix_len;/* byte offset of the end of
 					the last requested column */
 	ulint		mysql_row_len;	/* length in bytes of a row in the

include/trx0trx.h

@@ -43,34 +43,6 @@ extern sess_t*	trx_dummy_sess;
 the kernel mutex */
 extern ulint	trx_n_mysql_transactions;
 
-/*****************************************************************
-Resets the new record lock info in a transaction struct. */
-UNIV_INLINE
-void
-trx_reset_new_rec_lock_info(
-/*========================*/
-	trx_t*	trx);	/* in: transaction struct */
-/*****************************************************************
-Registers that we have set a new record lock on an index. We only have space
-to store 2 indexes! If this is called to store more than 2 indexes after
-trx_reset_new_rec_lock_info(), then this function does nothing. */
-UNIV_INLINE
-void
-trx_register_new_rec_lock(
-/*======================*/
-	trx_t*		trx,	/* in: transaction struct */
-	dict_index_t*	index);	/* in: trx sets a new record lock on this
-				index */
-/*****************************************************************
-Checks if trx has set a new record lock on an index. */
-UNIV_INLINE
-ibool
-trx_new_rec_locks_contain(
-/*======================*/
-				/* out: TRUE if trx has set a new record lock
-				on index */
-	trx_t*		trx,	/* in: transaction struct */
-	dict_index_t*	index);	/* in: index */
 /************************************************************************
 Releases the search latch if trx has reserved it. */
 UNIV_INTERN
@@ -609,20 +581,6 @@ struct trx_struct{
 					to srv_conc_innodb_enter, if the value
 					here is > 0, we decrement this by 1 */
 	/*------------------------------*/
-	dict_index_t*	new_rec_locks[2];/* these are normally NULL; if
-					srv_locks_unsafe_for_binlog is TRUE
-					or session is using READ COMMITTED
-					isolation level,
-					in a cursor search, if we set a new
-					record lock on an index, this is set
-					to point to the index; this is
-					used in releasing the locks under the
-					cursors if we are performing an UPDATE
-					and we determine after retrieving
-					the row that it does not need to be
-					locked; thus, these can be used to
-					implement a 'mini-rollback' that
-					releases the latest record locks */
 	UT_LIST_NODE_T(trx_t)
 			trx_list;	/* list of transactions */
 	UT_LIST_NODE_T(trx_t)

include/trx0trx.ic

@@ -55,64 +55,6 @@ trx_start_if_not_started_low(
 	}
 }
 
-/*****************************************************************
-Resets the new record lock info in a transaction struct. */
-UNIV_INLINE
-void
-trx_reset_new_rec_lock_info(
-/*========================*/
-	trx_t*	trx)	/* in: transaction struct */
-{
-	trx->new_rec_locks[0] = NULL;
-	trx->new_rec_locks[1] = NULL;
-}
-
-/*****************************************************************
-Registers that we have set a new record lock on an index. We only have space
-to store 2 indexes! If this is called to store more than 2 indexes after
-trx_reset_new_rec_lock_info(), then this function does nothing. */
-UNIV_INLINE
-void
-trx_register_new_rec_lock(
-/*======================*/
-	trx_t*		trx,	/* in: transaction struct */
-	dict_index_t*	index)	/* in: trx sets a new record lock on this
-				index */
-{
-	if (trx->new_rec_locks[0] == NULL) {
-		trx->new_rec_locks[0] = index;
-
-		return;
-	}
-
-	if (trx->new_rec_locks[0] == index) {
-
-		return;
-	}
-
-	if (trx->new_rec_locks[1] != NULL) {
-
-		return;
-	}
-
-	trx->new_rec_locks[1] = index;
-}
-
-/*****************************************************************
-Checks if trx has set a new record lock on an index. */
-UNIV_INLINE
-ibool
-trx_new_rec_locks_contain(
-/*======================*/
-				/* out: TRUE if trx has set a new record lock
-				on index */
-	trx_t*		trx,	/* in: transaction struct */
-	dict_index_t*	index)	/* in: index */
-{
-	return(trx->new_rec_locks[0] == index
-	       || trx->new_rec_locks[1] == index);
-}
-
 /********************************************************************
 Retrieves the error_info field from a trx. */
 UNIV_INLINE

lock/lock0lock.c

@@ -1976,12 +1976,6 @@ lock_rec_lock_fast(
 	if (lock == NULL) {
 		if (!impl) {
 			lock_rec_create(mode, block, heap_no, index, trx);
-
-			if (srv_locks_unsafe_for_binlog
-			    || trx->isolation_level
-			    == TRX_ISO_READ_COMMITTED) {
-				trx_register_new_rec_lock(trx, index);
-			}
 		}
 
 		return(TRUE);
@@ -2005,11 +1999,6 @@ lock_rec_lock_fast(
 
 		if (!lock_rec_get_nth_bit(lock, heap_no)) {
 			lock_rec_set_nth_bit(lock, heap_no);
-			if (srv_locks_unsafe_for_binlog
-			    || trx->isolation_level
-			    == TRX_ISO_READ_COMMITTED) {
-				trx_register_new_rec_lock(trx, index);
-			}
 		}
 	}
 
@@ -2069,22 +2058,12 @@ lock_rec_lock_slow(
 
 		err = lock_rec_enqueue_waiting(mode, block, heap_no,
 					       index, thr);
-
-		if (srv_locks_unsafe_for_binlog
-		    || trx->isolation_level == TRX_ISO_READ_COMMITTED) {
-			trx_register_new_rec_lock(trx, index);
-		}
 	} else {
 		if (!impl) {
 			/* Set the requested lock on the record */
 
 			lock_rec_add_to_queue(LOCK_REC | mode, block,
 					      heap_no, index, trx);
-			if (srv_locks_unsafe_for_binlog
-			    || trx->isolation_level
-			    == TRX_ISO_READ_COMMITTED) {
-				trx_register_new_rec_lock(trx, index);
-			}
 		}
 
 		err = DB_SUCCESS;

mysql-test/innodb-semi-consistent.result

@@ -38,3 +38,10 @@ a
 11
 7
 drop table t1;
+create table t1 (a int, b int) engine=myisam;
+create table t2 (c int, d int, key (c)) engine=innodb;
+insert into t1 values (1,1);
+insert into t2 values (1,2);
+set session transaction isolation level read committed;
+delete from t1 using t1 join t2 on t1.a = t2.c where t2.d in (1);
+drop table t1, t2;

mysql-test/innodb-semi-consistent.test

@@ -53,3 +53,16 @@ drop table t1;
 connection default;
 disconnect a;
 disconnect b;
+
+# Bug 39320
+create table t1 (a int, b int) engine=myisam;
+create table t2 (c int, d int, key (c)) engine=innodb;
+insert into t1 values (1,1);
+insert into t2 values (1,2);
+connect (a,localhost,root,,);
+connection a;
+set session transaction isolation level read committed;
+delete from t1 using t1 join t2 on t1.a = t2.c where t2.d in (1);
+connection default;
+disconnect a;
+drop table t1, t2;

row/row0mysql.c

@@ -1449,7 +1449,6 @@ row_unlock_for_mysql(
 					and clust_pcur, and we do not need to
 					reposition the cursors. */
 {
-	dict_index_t*	index;
 	btr_pcur_t*	pcur		= prebuilt->pcur;
 	btr_pcur_t*	clust_pcur	= prebuilt->clust_pcur;
 	trx_t*		trx		= prebuilt->trx;
@@ -1474,9 +1473,7 @@ row_unlock_for_mysql(
 
 	trx->op_info = "unlock_row";
 
-	index = btr_pcur_get_btr_cur(pcur)->index;
-
-	if (index != NULL && trx_new_rec_locks_contain(trx, index)) {
+	if (prebuilt->new_rec_locks >= 1) {
 
 		mtr_start(&mtr);
 
@@ -1492,22 +1489,9 @@ row_unlock_for_mysql(
 				rec, prebuilt->select_lock_type);
 
 		mtr_commit(&mtr);
-
-		/* If the search was done through the clustered index, then
-		we have not used clust_pcur at all, and we must NOT try to
-		reset locks on clust_pcur. The values in clust_pcur may be
-		garbage! */
-
-		if (dict_index_is_clust(index)) {
-
-			goto func_exit;
-		}
 	}
 
-	index = btr_pcur_get_btr_cur(clust_pcur)->index;
-
-	if (index != NULL && trx_new_rec_locks_contain(trx, index)) {
-
+	if (prebuilt->new_rec_locks >= 2) {
 		mtr_start(&mtr);
 
 		/* Restore the cursor position and find the record */
@@ -1525,7 +1509,6 @@ row_unlock_for_mysql(
 		mtr_commit(&mtr);
 	}
 
-func_exit:
 	trx->op_info = "";
 
 	return(DB_SUCCESS);

row/row0sel.c

@@ -3002,8 +3002,9 @@ row_sel_get_clust_rec_for_mysql(
 func_exit:
 	*out_rec = clust_rec;
 
-	if (prebuilt->select_lock_type == LOCK_X) {
-		/* We may use the cursor in update: store its position */
+	if (prebuilt->select_lock_type != LOCK_NONE) {
+		/* We may use the cursor in update or in unlock_row():
+		store its position */
 
 		btr_pcur_store_position(prebuilt->clust_pcur, mtr);
 	}
@@ -3405,13 +3406,7 @@ row_search_for_mysql(
 	is set or session is using a READ COMMITED isolation level. Then
 	we are able to remove the record locks set here on an individual
 	row. */
-
-	if ((srv_locks_unsafe_for_binlog
-	     || trx->isolation_level == TRX_ISO_READ_COMMITTED)
-	    && prebuilt->select_lock_type != LOCK_NONE) {
-
-		trx_reset_new_rec_lock_info(trx);
-	}
+	prebuilt->new_rec_locks = 0;
 
 	/*-------------------------------------------------------------*/
 	/* PHASE 1: Try to pop the row from the prefetch cache */
@@ -4056,6 +4051,12 @@ no_gap_lock:
 		switch (err) {
 			const rec_t*	old_vers;
 		case DB_SUCCESS:
+			if (srv_locks_unsafe_for_binlog
+			    || trx->isolation_level == TRX_ISO_READ_COMMITTED) {
+				/* Note that a record of
+				prebuilt->index was locked. */
+				prebuilt->new_rec_locks = 1;
+			}
 			break;
 		case DB_LOCK_WAIT:
 			if (UNIV_LIKELY(prebuilt->row_read_type
@@ -4086,7 +4087,7 @@ no_gap_lock:
 			if (UNIV_LIKELY(trx->wait_lock != NULL)) {
 				lock_cancel_waiting_and_release(
 					trx->wait_lock);
-				trx_reset_new_rec_lock_info(trx);
+				prebuilt->new_rec_locks = 0;
 			} else {
 				mutex_exit(&kernel_mutex);
 
@@ -4098,6 +4099,9 @@ no_gap_lock:
 							  ULINT_UNDEFINED,
 							  &heap);
 				err = DB_SUCCESS;
+				/* Note that a record of
+				prebuilt->index was locked. */
+				prebuilt->new_rec_locks = 1;
 				break;
 			}
 			mutex_exit(&kernel_mutex);
@@ -4246,6 +4250,15 @@ requires_clust_rec:
 			goto next_rec;
 		}
 
+		if ((srv_locks_unsafe_for_binlog
+		     || trx->isolation_level == TRX_ISO_READ_COMMITTED)
+		    && prebuilt->select_lock_type != LOCK_NONE) {
+			/* Note that both the secondary index record
+			and the clustered index record were locked. */
+			ut_ad(prebuilt->new_rec_locks == 1);
+			prebuilt->new_rec_locks = 2;
+		}
+
 		if (UNIV_UNLIKELY(rec_get_deleted_flag(clust_rec, comp))) {
 
 			/* The record is delete marked: we can skip it */
@@ -4375,13 +4388,7 @@ next_rec:
 		prebuilt->row_read_type = ROW_READ_TRY_SEMI_CONSISTENT;
 	}
 	did_semi_consistent_read = FALSE;
-
-	if (UNIV_UNLIKELY(srv_locks_unsafe_for_binlog
-			  || trx->isolation_level == TRX_ISO_READ_COMMITTED)
-	    && prebuilt->select_lock_type != LOCK_NONE) {
-
-		trx_reset_new_rec_lock_info(trx);
-	}
+	prebuilt->new_rec_locks = 0;
 
 	/*-------------------------------------------------------------*/
 	/* PHASE 5: Move the cursor to the next index record */
@@ -4487,7 +4494,7 @@ lock_wait_or_error:
 			rec_loop we will again try to set a lock, and
 			new_rec_lock_info in trx will be right at the end. */
 
-			trx_reset_new_rec_lock_info(trx);
+			prebuilt->new_rec_locks = 0;
 		}
 
 		mode = pcur->search_mode;

trx/trx0trx.c

@@ -183,8 +183,6 @@ trx_create(
 	trx->autoinc_locks = ib_vector_create(
 		mem_heap_create(sizeof(ib_vector_t) + sizeof(void*) * 4), 4);
 
-	trx_reset_new_rec_lock_info(trx);
-
 	return(trx);
 }