diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index 6a10d9defbf..14e4ac2e2b8 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -1677,6 +1677,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; @@ -1686,14 +1687,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 (;;) @@ -1721,10 +1721,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. @@ -1750,21 +1756,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) { @@ -1793,7 +1788,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 2f75d00edfa..c40a0c1e32e 100644 --- a/src/test/isolation/isolation_schedule +++ b/src/test/isolation/isolation_schedule @@ -14,6 +14,7 @@ test: partial-index test: two-ids test: multiple-row-versions test: index-only-scan +test: predicate-lock-hot-tuple test: fk-contention test: fk-deadlock test: fk-deadlock2 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"