From 95942f0459d3e1db08ae14c5ee1f65e12be2e036 Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Tue, 6 Aug 2019 13:49:58 +1200 Subject: [PATCH] Predicate-lock the visible heap tuple, not the HOT root. Commit 81fbbfe3352 fixed a bug where we predicate-locked the HOT root. Commit b89e151054a unfixed it. Refix it. Add an isolation test that demonstrates that we locked the row version that we actually emitted. Back-patch to all supported version. Author: Thomas Munro Reported-by: Andres Freund Discussion: https://postgr.es/m/CALfoeiv0k3hkEb3Oqk%3DziWqtyk2Jys1UOK5hwRBNeANT_yX%2Bng%40mail.gmail.com --- src/backend/access/heap/heapam.c | 6 ++--- .../isolation/expected/serializable-hot.out | 16 +++++++++++ src/test/isolation/isolation_schedule | 1 + .../isolation/specs/serializable-hot.spec | 27 +++++++++++++++++++ 4 files changed, 46 insertions(+), 4 deletions(-) create mode 100644 src/test/isolation/expected/serializable-hot.out create mode 100644 src/test/isolation/specs/serializable-hot.spec diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index 94309949fa..107605d276 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -1576,7 +1576,6 @@ heap_hot_search_buffer(ItemPointer tid, Relation relation, Buffer buffer, heapTuple->t_data = (HeapTupleHeader) PageGetItem(dp, lp); heapTuple->t_len = ItemIdGetLength(lp); heapTuple->t_tableOid = RelationGetRelid(relation); - ItemPointerSetOffsetNumber(&heapTuple->t_self, offnum); /* * Shouldn't see a HEAP_ONLY tuple at chain start. @@ -1608,15 +1607,14 @@ heap_hot_search_buffer(ItemPointer tid, Relation relation, Buffer buffer, * 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. + * SSI also needs offnum. */ - ItemPointerSet(&(heapTuple->t_self), BufferGetBlockNumber(buffer), offnum); + ItemPointerSetOffsetNumber(&heapTuple->t_self, 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) { diff --git a/src/test/isolation/expected/serializable-hot.out b/src/test/isolation/expected/serializable-hot.out new file mode 100644 index 0000000000..d2bacd77fa --- /dev/null +++ b/src/test/isolation/expected/serializable-hot.out @@ -0,0 +1,16 @@ +Parsed test spec with 1 sessions + +starting permutation: r1 s1 c1 +step r1: SELECT t FROM test WHERE i = 42 +t + +banana sundae +step s1: SELECT ('('||page||','||tuple||')')::tid = (SELECT ctid FROM test) AS locked_visible + FROM pg_locks + WHERE locktype = 'tuple' + AND relation = 'test'::regclass + AND mode = 'SIReadLock' +locked_visible + +t +step c1: COMMIT; diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule index 009c2ec54b..8ac88e9c77 100644 --- a/src/test/isolation/isolation_schedule +++ b/src/test/isolation/isolation_schedule @@ -86,3 +86,4 @@ test: plpgsql-toast test: truncate-conflict test: serializable-parallel test: serializable-parallel-2 +test: serializable-hot diff --git a/src/test/isolation/specs/serializable-hot.spec b/src/test/isolation/specs/serializable-hot.spec new file mode 100644 index 0000000000..21d1050cca --- /dev/null +++ b/src/test/isolation/specs/serializable-hot.spec @@ -0,0 +1,27 @@ +# Assert that when we access a tuple that is part of a HOT chain, we acquire a +# tuple lock on the tuple we actually see, even though the index points at +# an older version. + +setup +{ + CREATE TABLE test (i int PRIMARY KEY, t text); + INSERT INTO test VALUES (42, 'banana'); + UPDATE test SET t = t || ' sundae'; +} + +teardown +{ + DROP TABLE test; +} + +session "s1" +setup { BEGIN ISOLATION LEVEL SERIALIZABLE; SET enable_seqscan=off; } +step "r1" { SELECT t FROM test WHERE i = 42 } +step "s1" { SELECT ('('||page||','||tuple||')')::tid = (SELECT ctid FROM test) AS locked_visible + FROM pg_locks + WHERE locktype = 'tuple' + AND relation = 'test'::regclass + AND mode = 'SIReadLock' } +step "c1" { COMMIT; } + +permutation "r1" "s1" "c1" -- 2.22.0