4 files changed, 70 insertions, 17 deletions

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index e33f019939d..6b42bdc77b2 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -1529,6 +1529,7 @@ heap_hot_search_buffer(ItemPointer tid, Relation relation, Buffer buffer,
 {
 	Page		dp = (Page) BufferGetPage(buffer);
 	TransactionId prev_xmax = InvalidTransactionId;
+	BlockNumber blkno;
 	OffsetNumber offnum;
 	bool		at_chain_start;
 	bool		valid;
@@ -1538,14 +1539,13 @@ heap_hot_search_buffer(ItemPointer tid, Relation relation, Buffer buffer,
 	if (all_dead)
 		*all_dead = first_call;
 
-	Assert(TransactionIdIsValid(RecentGlobalXmin));
-
-	Assert(ItemPointerGetBlockNumber(tid) == BufferGetBlockNumber(buffer));
+	blkno = ItemPointerGetBlockNumber(tid);
 	offnum = ItemPointerGetOffsetNumber(tid);
 	at_chain_start = first_call;
 	skip = !first_call;
 
-	heapTuple->t_self = *tid;
+	Assert(TransactionIdIsValid(RecentGlobalXmin));
+	Assert(BufferGetBlockNumber(buffer) == blkno);
 
 	/* Scan through possible multiple members of HOT-chain */
 	for (;;)
@@ -1573,10 +1573,16 @@ heap_hot_search_buffer(ItemPointer tid, Relation relation, Buffer buffer,
 			break;
 		}
 
+		/*
+		 * Update heapTuple to point to the element of the HOT chain we're
+		 * currently investigating. Having t_self set correctly is important
+		 * because the SSI checks and the *Satisfies routine for historical
+		 * MVCC snapshots need the correct tid to decide about the visibility.
+		 */
 		heapTuple->t_data = (HeapTupleHeader) PageGetItem(dp, lp);
 		heapTuple->t_len = ItemIdGetLength(lp);
 		heapTuple->t_tableOid = RelationGetRelid(relation);
-		ItemPointerSetOffsetNumber(&heapTuple->t_self, offnum);
+		ItemPointerSet(&heapTuple->t_self, blkno, offnum);
 
 		/*
 		 * Shouldn't see a HEAP_ONLY tuple at chain start.
@@ -1602,21 +1608,10 @@ heap_hot_search_buffer(ItemPointer tid, Relation relation, Buffer buffer,
 		 */
 		if (!skip)
 		{
-			/*
-			 * For the benefit of logical decoding, have t_self point at the
-			 * element of the HOT chain we're currently investigating instead
-			 * of the root tuple of the HOT chain. This is important because
-			 * the *Satisfies routine for historical mvcc snapshots needs the
-			 * correct tid to decide about the visibility in some cases.
-			 */
-			ItemPointerSet(&(heapTuple->t_self), BufferGetBlockNumber(buffer), offnum);
-
 			/* If it's visible per the snapshot, we must return it */
 			valid = HeapTupleSatisfiesVisibility(heapTuple, snapshot, buffer);
 			CheckForSerializableConflictOut(valid, relation, heapTuple,
 											buffer, snapshot);
-			/* reset to original, non-redirected, tid */
-			heapTuple->t_self = *tid;
 
 			if (valid)
 			{
@@ -1648,7 +1643,7 @@ heap_hot_search_buffer(ItemPointer tid, Relation relation, Buffer buffer,
 		if (HeapTupleIsHotUpdated(heapTuple))
 		{
 			Assert(ItemPointerGetBlockNumber(&heapTuple->t_data->t_ctid) ==
-				   ItemPointerGetBlockNumber(tid));
+				   blkno);
 			offnum = ItemPointerGetOffsetNumber(&heapTuple->t_data->t_ctid);
 			at_chain_start = false;
 			prev_xmax = HeapTupleHeaderGetUpdateXid(heapTuple->t_data);
diff --git a/src/test/isolation/expected/predicate-lock-hot-tuple.out b/src/test/isolation/expected/predicate-lock-hot-tuple.out
new file mode 100644
index 00000000000..d1c69bbbd03
--- /dev/null
+++ b/src/test/isolation/expected/predicate-lock-hot-tuple.out
@@ -0,0 +1,20 @@
+Parsed test spec with 2 sessions
+
+starting permutation: b1 b2 r1 r2 w1 w2 c1 c2
+step b1: BEGIN ISOLATION LEVEL SERIALIZABLE;
+step b2: BEGIN ISOLATION LEVEL SERIALIZABLE;
+step r1: SELECT * FROM test WHERE i IN (5, 7)
+i              t              
+
+5              apple          
+7              pear_hot_updated
+step r2: SELECT * FROM test WHERE i IN (5, 7)
+i              t              
+
+5              apple          
+7              pear_hot_updated
+step w1: UPDATE test SET t = 'pear_xact1' WHERE i = 7
+step w2: UPDATE test SET t = 'apple_xact2' WHERE i = 5
+step c1: COMMIT;
+step c2: COMMIT;
+ERROR:  could not serialize access due to read/write dependencies among transactions
diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule
index 009c2ec54bd..69ae2279533 100644
--- a/src/test/isolation/isolation_schedule
+++ b/src/test/isolation/isolation_schedule
@@ -17,6 +17,7 @@ test: partial-index
 test: two-ids
 test: multiple-row-versions
 test: index-only-scan
+test: predicate-lock-hot-tuple
 test: deadlock-simple
 test: deadlock-hard
 test: deadlock-soft
diff --git a/src/test/isolation/specs/predicate-lock-hot-tuple.spec b/src/test/isolation/specs/predicate-lock-hot-tuple.spec
new file mode 100644
index 00000000000..d16fb60533b
--- /dev/null
+++ b/src/test/isolation/specs/predicate-lock-hot-tuple.spec
@@ -0,0 +1,37 @@
+# Test predicate locks on HOT updated tuples.
+#
+# This test has two serializable transactions. Both select two rows
+# from the table, and then update one of them.
+# If these were serialized (run one at a time), the transaction that
+# runs later would see one of the rows to be updated.
+#
+# Any overlap between the transactions must cause a serialization failure.
+# We used to have a bug in predicate locking HOT updated tuples, which
+# caused the conflict to be missed, if the row was HOT updated.
+
+setup
+{
+  CREATE TABLE test (i int PRIMARY KEY, t text);
+  INSERT INTO test VALUES (5, 'apple'), (7, 'pear'), (11, 'banana');
+  -- HOT-update 'pear' row.
+  UPDATE test SET t = 'pear_hot_updated' WHERE i = 7;
+}
+
+teardown
+{
+  DROP TABLE test;
+}
+
+session "s1"
+step "b1" { BEGIN ISOLATION LEVEL SERIALIZABLE; }
+step "r1" { SELECT * FROM test WHERE i IN (5, 7) }
+step "w1" { UPDATE test SET t = 'pear_xact1' WHERE i = 7 }
+step "c1" { COMMIT; }
+
+session "s2"
+step "b2" { BEGIN ISOLATION LEVEL SERIALIZABLE; }
+step "r2" { SELECT * FROM test WHERE i IN (5, 7) }
+step "w2" { UPDATE test SET t = 'apple_xact2' WHERE i = 5 }
+step "c2" { COMMIT; }
+
+permutation "b1" "b2" "r1" "r2" "w1" "w2" "c1" "c2"