Fix for bug #12456: Cursor shows incorrect data - DML does not affect,

probably caching. Fixed test results.

This patch implements a high-granularity read view to be used with
cursors. In this high-granularity consistent read view modifications 
done by the creating transaction after the cursor is created or 
future transactions are not visible. But those modifications that 
transaction did before the cursor was created are visible.

include/read0read.h

@@ -25,7 +25,8 @@ read_view_t*
 read_view_open_now(
 /*===============*/
 				/* out, own: read view struct */
-	trx_t*		cr_trx,	/* in: creating transaction, or NULL */
+	dulint		cr_trx_id,/* in: trx_id of creating transaction, 
+				or (0, 0) used in purge */
 	mem_heap_t*	heap);	/* in: memory heap from which allocated */
 /*************************************************************************
 Makes a copy of the oldest existing read view, or opens a new. The view
@@ -35,7 +36,8 @@ read_view_t*
 read_view_oldest_copy_or_open_new(
 /*==============================*/
 				/* out, own: read view struct */
-	trx_t*		cr_trx,	/* in: creating transaction, or NULL */
+	dulint		cr_trx_id,/* in: trx_id of creating transaction, 
+				or (0, 0) used in purge */
 	mem_heap_t*	heap);	/* in: memory heap from which allocated */
 /*************************************************************************
 Closes a read view. */
@@ -101,6 +103,10 @@ read_cursor_set_for_mysql(
 read should not see the modifications to the database. */
 
 struct read_view_struct{
+	ulint	type;		/* VIEW_NORMAL, VIEW_HIGH_GRANULARITY */
+	dulint	undo_no;	/* (0, 0) or if type is VIEW_HIGH_GRANULARITY
+				transaction undo_no when this high-granularity
+				consistent read view was created */
 	ibool	can_be_too_old;	/* TRUE if the system has had to purge old
 				versions which this read view should be able
 				to access: the read view can bump into the
@@ -121,12 +127,23 @@ struct read_view_struct{
 				serialized, except the reading transaction
 				itself; the trx ids in this array are in a
 				descending order */
-	trx_t*	creator;	/* Pointer to the creating transaction, or
-				NULL if used in purge */
+	dulint	creator_trx_id;	/* trx id of creating transaction, or
+				(0, 0) used in purge */
 	UT_LIST_NODE_T(read_view_t) view_list;
 				/* List of read views in trx_sys */
 };
 
+/* Read view types */
+#define VIEW_NORMAL		1	/* Normal consistent read view
+					where transaction does not see changes
+					made by active transactions except
+					creating transaction. */
+#define VIEW_HIGH_GRANULARITY	2	/* High-granularity read view where
+					transaction does not see changes
+					made by active transactions and own
+					changes after a point in time when this
+					read view was created. */
+
 /* Implement InnoDB framework to support consistent read views in
 cursors. This struct holds both heap where consistent read view
 is allocated and pointer to a read view. */

read/read0read.c

@@ -15,6 +15,111 @@ Created 2/16/1997 Heikki Tuuri
 #include "srv0srv.h"
 #include "trx0sys.h"
 
+/*
+-------------------------------------------------------------------------------
+FACT A: Cursor read view on a secondary index sees only committed versions
+-------
+of the records in the secondary index or those versions of rows created 
+by transaction which created a cursor before cursor was created even 
+if transaction which created the cursor has changed that clustered index page. 
+
+PROOF: We must show that read goes always to the clustered index record
+to see that record is visible in the cursor read view. Consider e.g.
+following table and SQL-clauses:
+
+create table t1(a int not null, b int, primary key(a), index(b));
+insert into t1 values (1,1),(2,2);
+commit;
+
+Now consider that we have a cursor for a query
+
+select b from t1 where b >= 1;
+
+This query will use secondary key on the table t1. Now after the first fetch
+on this cursor if we do a update:
+
+update t1 set b = 5 where b = 2;
+
+Now second fetch of the cursor should not see record (2,5) instead it should
+see record (2,2).
+
+We also should show that if we have delete t1 where b = 5; we still
+can see record (2,2).
+
+When we access a secondary key record maximum transaction id is fetched 
+from this record and this trx_id is compared to up_limit_id in the view. 
+If trx_id in the record is greater or equal than up_limit_id in the view 
+cluster record is accessed.  Because trx_id of the creating 
+transaction is stored when this view was created to the list of 
+trx_ids not seen by this read view previous version of the
+record is requested to be built. This is build using clustered record.
+If the secondary key record is delete  marked it's corresponding 
+clustered record can be already be purged only if records 
+trx_id < low_limit_no. Purge can't remove any record deleted by a 
+transaction which was active when cursor was created. But, we still 
+may have a deleted secondary key record but no clustered record. But, 
+this is not a problem because this case is handled in 
+row_sel_get_clust_rec() function which is called
+whenever we note that this read view does not see trx_id in the
+record. Thus, we see correct version. Q. E. D.
+
+-------------------------------------------------------------------------------
+FACT B: Cursor read view on a clustered index sees only committed versions
+-------
+of the records in the clustered index or those versions of rows created 
+by transaction which created a cursor before cursor was created even 
+if transaction which created the cursor has changed that clustered index page. 
+
+PROOF:  Consider e.g.following table and SQL-clauses:
+
+create table t1(a int not null, b int, primary key(a));
+insert into t1 values (1),(2);
+commit;
+
+Now consider that we have a cursor for a query
+
+select a from t1 where a >= 1;
+
+This query will use clustered key on the table t1. Now after the first fetch
+on this cursor if we do a update:
+
+update t1 set a = 5 where a = 2;
+
+Now second fetch of the cursor should not see record (5) instead it should
+see record (2).
+
+We also should show that if we have execute delete t1 where a = 5; after
+the cursor is opened we still can see record (2).
+
+When accessing clustered record we always check if this read view sees
+trx_id stored to clustered record. By default we don't see any changes 
+if record trx_id >= low_limit_id i.e. change was made transaction 
+which started after transaction which created the cursor. If row 
+was changed by the future transaction a previous version of the 
+clustered record is created. Thus we see only committed version in 
+this case. We see all changes made by committed transactions i.e. 
+record trx_id < up_limit_id. In this case we don't need to do anything, 
+we already see correct version of the record. We don't see any changes 
+made by active transaction except creating transaction. We have stored 
+trx_id of creating transaction to list of trx_ids when this view was
+created. Thus we can easily see if this record was changed by the 
+creating transaction. Because we already have clustered record we can 
+access roll_ptr. Using this roll_ptr we can fetch undo record.
+We can now check that undo_no of the undo record is less than undo_no of the
+trancaction which created a view when cursor was created. We see this 
+clustered record only in case when record undo_no is less than undo_no 
+in the view. If this is not true we build based on undo_rec previous 
+version of the record. This record is found because purge can't remove 
+records accessed by active transaction. Thus we see correct version. Q. E. D.
+-------------------------------------------------------------------------------
+FACT C: Purge does not remove any delete marked row that is visible
+------- 
+to cursor view.
+
+TODO: proof this
+
+*/
+
 /*************************************************************************
 Creates a read view object. */
 UNIV_INLINE
@@ -45,7 +150,8 @@ read_view_t*
 read_view_oldest_copy_or_open_new(
 /*==============================*/
 				/* out, own: read view struct */
-	trx_t*		cr_trx,	/* in: creating transaction, or NULL */
+	dulint		cr_trx_id,/* in: trx_id of creating transaction, or 
+				(0, 0) used in purge*/
 	mem_heap_t*	heap)	/* in: memory heap from which allocated */
 {
 	read_view_t*	old_view;
@@ -62,12 +168,13 @@ read_view_oldest_copy_or_open_new(
 
 	if (old_view == NULL) {
 
-		return(read_view_open_now(cr_trx, heap));
+		return(read_view_open_now(cr_trx_id, heap));
 	}
 
 	n = old_view->n_trx_ids;
 
-	if (old_view->creator) {
+	if (ut_dulint_cmp(old_view->creator_trx_id, 
+					ut_dulint_create(0,0)) != 0) {
 		n++;
 	} else {
 		needs_insert = FALSE;
@@ -82,12 +189,12 @@ read_view_oldest_copy_or_open_new(
 	while (i < n) {
 		if (needs_insert
 			&& (i >= old_view->n_trx_ids
-				|| ut_dulint_cmp(old_view->creator->id,
+				|| ut_dulint_cmp(old_view->creator_trx_id,
 					read_view_get_nth_trx_id(old_view, i))
 				> 0)) {
 
 			read_view_set_nth_trx_id(view_copy, i,
-						old_view->creator->id);
+						old_view->creator_trx_id);
 			needs_insert = FALSE;
 			insert_done = 1;
 		} else {
@@ -99,7 +206,7 @@ read_view_oldest_copy_or_open_new(
 		i++;
 	}
 
-	view_copy->creator = cr_trx;
+	view_copy->creator_trx_id = cr_trx_id;
 
 	view_copy->low_limit_no = old_view->low_limit_no;
 	view_copy->low_limit_id = old_view->low_limit_id;
@@ -127,7 +234,8 @@ read_view_t*
 read_view_open_now(
 /*===============*/
 				/* out, own: read view struct */
-	trx_t*		cr_trx,	/* in: creating transaction, or NULL */
+	dulint		cr_trx_id,/* in: trx_id of creating transaction, 
+				or (0, 0) used in purge */
 	mem_heap_t*	heap)	/* in: memory heap from which allocated */
 {
 	read_view_t*	view;
@@ -138,7 +246,9 @@ read_view_open_now(
 #endif /* UNIV_SYNC_DEBUG */
 	view = read_view_create_low(UT_LIST_GET_LEN(trx_sys->trx_list), heap);
 
-	view->creator = cr_trx;
+	view->creator_trx_id = cr_trx_id;
+	view->type = VIEW_NORMAL;
+	view->undo_no = ut_dulint_create(0, 0);
 
 	/* No future transactions should be visible in the view */
 
@@ -153,8 +263,9 @@ read_view_open_now(
 	/* No active transaction should be visible, except cr_trx */
 
 	while (trx) {
-		if (trx != cr_trx && (trx->conc_state == TRX_ACTIVE ||
-					trx->conc_state == TRX_PREPARED)) {
+		if (ut_dulint_cmp(trx->id, cr_trx_id) != 0
+			&& (trx->conc_state == TRX_ACTIVE 
+				|| trx->conc_state == TRX_PREPARED)) {
 
 			read_view_set_nth_trx_id(view, n, trx->id);
 
@@ -184,6 +295,7 @@ read_view_open_now(
 		view->up_limit_id = view->low_limit_id;
 	}
 
+	
 	UT_LIST_ADD_FIRST(view_list, trx_sys->view_list, view);
 
 	return(view);
@@ -237,6 +349,15 @@ read_view_print(
 	ulint	n_ids;
 	ulint	i;
  
+	if (view->type == VIEW_HIGH_GRANULARITY) {
+		fprintf(stderr, 
+			"High-granularity read view undo_n:o %lu %lu\n", 
+			(ulong) ut_dulint_get_high(view->undo_no),
+			(ulong) ut_dulint_get_low(view->undo_no));
+	} else {
+		fprintf(stderr, "Normal read view\n");
+	}
+
 	fprintf(stderr, "Read view low limit trx n:o %lu %lu\n",
 			(ulong) ut_dulint_get_high(view->low_limit_no),
 			(ulong) ut_dulint_get_low(view->low_limit_no));
@@ -261,9 +382,10 @@ read_view_print(
 }
 
 /*************************************************************************
-Create a consistent cursor view for mysql to be used in cursors. In this
-consistent read view modifications done by the creating transaction or future
-transactions are not visible. */
+Create a high-granularity consistent cursor view for mysql to be used 
+in cursors. In this consistent read view modifications done by the 
+creating transaction after the cursor is created or future transactions 
+are not visible. */
 
 cursor_view_t*
 read_cursor_view_create_for_mysql(
@@ -298,7 +420,9 @@ read_cursor_view_create_for_mysql(
 					curview->heap);
 
 	view = curview->read_view;
-	view->creator = cr_trx;
+	view->creator_trx_id = cr_trx->id;
+	view->type = VIEW_HIGH_GRANULARITY;
+	view->undo_no = cr_trx->undo_no;
 
 	/* No future transactions should be visible in the view */
 
@@ -310,13 +434,12 @@ read_cursor_view_create_for_mysql(
 	n = 0;
 	trx = UT_LIST_GET_FIRST(trx_sys->trx_list);
 
-	/* No active transaction should be visible, except cr_trx.
-	This is quick fix for a bug 12456 and needs to be fixed when
-	semi-consistent high-granularity read view is implemented. */
+	/* No active transaction should be visible */
 
 	while (trx) {
-		if (trx != cr_trx && (trx->conc_state == TRX_ACTIVE ||
-					trx->conc_state == TRX_PREPARED)) {
+
+		if (trx->conc_state == TRX_ACTIVE 
+			|| trx->conc_state == TRX_PREPARED) {
 
 			read_view_set_nth_trx_id(view, n, trx->id);
 

row/row0sel.c

@@ -4329,7 +4329,7 @@ row_search_check_if_query_cache_permitted(
 		if (trx->isolation_level >= TRX_ISO_REPEATABLE_READ
 			&& !trx->read_view) {
 
-			trx->read_view = read_view_open_now(trx,
+			trx->read_view = read_view_open_now(trx->id,
 						trx->global_read_view_heap);
 			trx->global_read_view = trx->read_view;
 		}

row/row0vers.c

@@ -421,7 +421,7 @@ row_vers_build_for_consistent_read(
 {
 	rec_t*		version;
 	rec_t*		prev_version;
-	dulint		prev_trx_id;
+	dulint		trx_id;
 	mem_heap_t*	heap		= NULL;
 	byte*		buf;
 	ulint		err;
@@ -436,16 +436,48 @@ row_vers_build_for_consistent_read(
 
 	ut_ad(rec_offs_validate(rec, index, *offsets));
 
-	ut_ad(!read_view_sees_trx_id(view,
-				row_get_rec_trx_id(rec, index, *offsets)));
+	trx_id = row_get_rec_trx_id(rec, index, *offsets);
+
+	ut_ad(!read_view_sees_trx_id(view, trx_id));
 
 	rw_lock_s_lock(&(purge_sys->latch));
 	version = rec;
 
 	for (;;) {
 		mem_heap_t*	heap2	= heap;
+		trx_undo_rec_t* undo_rec;
+		dulint		roll_ptr;
+		dulint		undo_no;
 		heap = mem_heap_create(1024);
 
+		/* If we have high-granularity consistent read view and
+		creating transaction of the view is the same as trx_id in
+		the record we see this record only in the case when
+		undo_no of the record is < undo_no in the view. */
+
+		if (view->type == VIEW_HIGH_GRANULARITY 
+		&& ut_dulint_cmp(view->creator_trx_id, trx_id) == 0) {
+
+			roll_ptr = row_get_rec_roll_ptr(version, index, 
+								*offsets);
+			undo_rec = trx_undo_get_undo_rec_low(roll_ptr, heap);
+			undo_no = trx_undo_rec_get_undo_no(undo_rec);
+			mem_heap_empty(heap);
+
+			if (ut_dulint_cmp(view->undo_no, undo_no) > 0) {
+				/* The view already sees this version: we can 
+				copy it to in_heap and return */
+
+				buf = mem_heap_alloc(in_heap, 
+						rec_offs_size(*offsets));
+				*old_vers = rec_copy(buf, version, *offsets);
+				rec_offs_make_valid(*old_vers, index, *offsets);
+				err = DB_SUCCESS;
+
+				break;
+			}
+		}
+
 		err = trx_undo_prev_version_build(rec, mtr, version, index,
 						*offsets, heap, &prev_version);
 		if (heap2) {
@@ -466,10 +498,10 @@ row_vers_build_for_consistent_read(
 
 		*offsets = rec_get_offsets(prev_version, index, *offsets,
 					ULINT_UNDEFINED, offset_heap);
-		prev_trx_id = row_get_rec_trx_id(prev_version, index,
-					*offsets);
 
-		if (read_view_sees_trx_id(view, prev_trx_id)) {
+		trx_id = row_get_rec_trx_id(prev_version, index, *offsets);
+
+		if (read_view_sees_trx_id(view, trx_id)) {
 
 			/* The view already sees this version: we can copy
 			it to in_heap and return */

trx/trx0purge.c

@@ -231,8 +231,8 @@ trx_purge_sys_create(void)
 
 	purge_sys->query = trx_purge_graph_build();
 
-	purge_sys->view = read_view_oldest_copy_or_open_new(NULL,
-							purge_sys->heap);
+	purge_sys->view = read_view_oldest_copy_or_open_new(
+				ut_dulint_create(0,0), purge_sys->heap);
 }
 
 /*================ UNDO LOG HISTORY LIST =============================*/
@@ -1090,7 +1090,8 @@ trx_purge(void)
 		}
 	}
 
-	purge_sys->view = read_view_oldest_copy_or_open_new(NULL,
+	purge_sys->view = read_view_oldest_copy_or_open_new(
+						ut_dulint_create(0, 0),
 							purge_sys->heap);
 	mutex_exit(&kernel_mutex);
 

trx/trx0trx.c

@@ -999,7 +999,7 @@ trx_assign_read_view(
 	mutex_enter(&kernel_mutex);
 
 	if (!trx->read_view) {
-		trx->read_view = read_view_open_now(trx,
+		trx->read_view = read_view_open_now(trx->id,
 					trx->global_read_view_heap);
 		trx->global_read_view = trx->read_view;
 	}