From c610ea97a5cd103b1db0906c7501ffa69316d9e0 Mon Sep 17 00:00:00 2001 From: Aleksander Alekseev Date: Mon, 21 Sep 2026 14:31:46 +0300 Subject: [PATCH v3] Fix write skew under SERIALIZABLE for Tid Range Scans A TID range scan reads heap blocks directly with no index involved, yet it took no predicate lock at all, so concurrent SERIALIZABLE transactions scanning the same range failed to see the rw-conflicts between them and could silently commit a write skew. Lock the whole relation, as a seqscan does; heap page locks merely aggregate tuple locks and don't cover gaps, so nothing finer would conflict with an insert into the scanned range. Author: Jacob Brazeal Reviewed-by: Aleksander Alekseev Reviewed-by: Andrey Borodin Reviewed-by: Matthias van de Meent Discussion: https://postgr.es/m/CA%2BCOZaBo%2BZpKgMvxcdACUjNtdYipe9Em06iX5KHLTVaTmFibiw%40mail.gmail.com --- src/backend/access/heap/heapam.c | 30 ++-- src/backend/storage/lmgr/README-SSI | 6 + .../expected/predicate-tidrangescan.out | 148 ++++++++++++++++++ src/test/isolation/isolation_schedule | 1 + .../specs/predicate-tidrangescan.spec | 73 +++++++++ 5 files changed, 245 insertions(+), 13 deletions(-) create mode 100644 src/test/isolation/expected/predicate-tidrangescan.out create mode 100644 src/test/isolation/specs/predicate-tidrangescan.spec diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index 9ebb1b35d37..3bdbe4686e9 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -1224,19 +1224,23 @@ heap_beginscan(Relation relation, Snapshot snapshot, } /* - * For seqscan and sample scans in a serializable transaction, acquire a - * predicate lock on the entire relation. This is required not only to - * lock all the matching tuples, but also to conflict with new insertions - * into the table. In an indexscan, we take page locks on the index pages - * covering the range specified in the scan qual, but in a heap scan there - * is nothing more fine-grained to lock. A bitmap scan is a different - * story, there we have already scanned the index and locked the index - * pages covering the predicate. But in that case we still have to lock - * any matching heap tuples. For sample scan we could optimize the locking - * to be at least page-level granularity, but we'd need to add per-tuple - * locking for that. - */ - if (scan->rs_base.rs_flags & (SO_TYPE_SEQSCAN | SO_TYPE_SAMPLESCAN)) + * For seqscan, sample and TID range scans in a serializable transaction, + * acquire a predicate lock on the entire relation. This is required not + * only to lock all the matching tuples, but also to conflict with new + * insertions into the table. In an indexscan, we take page locks on the + * index pages covering the range specified in the scan qual, but in a + * heap scan there is nothing more fine-grained to lock. A bitmap scan is + * a different story, there we have already scanned the index and locked + * the index pages covering the predicate. But in that case we still have + * to lock any matching heap tuples. For sample scan we could optimize the + * locking to be at least page-level granularity, but we'd need to add + * per-tuple locking for that. A TID range scan is like a seqscan in this + * respect: it reads heap blocks directly with no index involved, so there + * is nothing finer to lock, and heap_insert() only checks for conflicts + * against relation-level predicate locks anyway. + */ + if (scan->rs_base.rs_flags & (SO_TYPE_SEQSCAN | SO_TYPE_SAMPLESCAN | + SO_TYPE_TIDRANGESCAN)) { /* * Ensure a missing snapshot is noticed reliably, even if the diff --git a/src/backend/storage/lmgr/README-SSI b/src/backend/storage/lmgr/README-SSI index 50d2ecca9d7..76558256146 100644 --- a/src/backend/storage/lmgr/README-SSI +++ b/src/backend/storage/lmgr/README-SSI @@ -305,6 +305,12 @@ Predicate locks will be acquired for the heap based on the following: * For a table scan, the entire relation will be locked. + * For a TID range scan, the entire relation will also be locked. +Such a scan only reads a range of blocks, but there is nothing finer +to lock, because heap page locks don't cover "gaps" (see below); a +lock on just the pages in the range would not conflict with an insert +of a new tuple into that range. + * Each tuple read which is visible to the reading transaction will be locked, whether or not it meets selection criteria; except that there is no need to acquire an SIREAD lock on a tuple when the diff --git a/src/test/isolation/expected/predicate-tidrangescan.out b/src/test/isolation/expected/predicate-tidrangescan.out new file mode 100644 index 00000000000..d60bdb28202 --- /dev/null +++ b/src/test/isolation/expected/predicate-tidrangescan.out @@ -0,0 +1,148 @@ +Parsed test spec with 2 sessions + +starting permutation: rxy1 rxy2 wx1 wy2 c1 c2 +step rxy1: select sum(p) from tidrange_tbl + where ctid >= '(0,0)' and ctid < '(1,0)'; +sum +--- + 0 +(1 row) + +step rxy2: select sum(p) from tidrange_tbl + where ctid >= '(0,0)' and ctid < '(1,0)'; +sum +--- + 0 +(1 row) + +step wx1: update tidrange_tbl set p = 1 where ctid = '(0,1)'; +step wy2: update tidrange_tbl set p = 1 where ctid = '(0,2)'; +step c1: commit; +step c2: commit; +ERROR: could not serialize access due to read/write dependencies among transactions + +starting permutation: rxy1 rxy2 wy2 wx1 c1 c2 +step rxy1: select sum(p) from tidrange_tbl + where ctid >= '(0,0)' and ctid < '(1,0)'; +sum +--- + 0 +(1 row) + +step rxy2: select sum(p) from tidrange_tbl + where ctid >= '(0,0)' and ctid < '(1,0)'; +sum +--- + 0 +(1 row) + +step wy2: update tidrange_tbl set p = 1 where ctid = '(0,2)'; +step wx1: update tidrange_tbl set p = 1 where ctid = '(0,1)'; +step c1: commit; +step c2: commit; +ERROR: could not serialize access due to read/write dependencies among transactions + +starting permutation: rxy2 rxy1 wx1 wy2 c2 c1 +step rxy2: select sum(p) from tidrange_tbl + where ctid >= '(0,0)' and ctid < '(1,0)'; +sum +--- + 0 +(1 row) + +step rxy1: select sum(p) from tidrange_tbl + where ctid >= '(0,0)' and ctid < '(1,0)'; +sum +--- + 0 +(1 row) + +step wx1: update tidrange_tbl set p = 1 where ctid = '(0,1)'; +step wy2: update tidrange_tbl set p = 1 where ctid = '(0,2)'; +step c2: commit; +step c1: commit; +ERROR: could not serialize access due to read/write dependencies among transactions + +starting permutation: rxy1 rxy2 wi1 wi2 c1 c2 +step rxy1: select sum(p) from tidrange_tbl + where ctid >= '(0,0)' and ctid < '(1,0)'; +sum +--- + 0 +(1 row) + +step rxy2: select sum(p) from tidrange_tbl + where ctid >= '(0,0)' and ctid < '(1,0)'; +sum +--- + 0 +(1 row) + +step wi1: insert into tidrange_tbl values (3, 10); +step wi2: insert into tidrange_tbl values (4, 20); +step c1: commit; +step c2: commit; +ERROR: could not serialize access due to read/write dependencies among transactions + +starting permutation: rxy2 rxy1 wi2 wi1 c2 c1 +step rxy2: select sum(p) from tidrange_tbl + where ctid >= '(0,0)' and ctid < '(1,0)'; +sum +--- + 0 +(1 row) + +step rxy1: select sum(p) from tidrange_tbl + where ctid >= '(0,0)' and ctid < '(1,0)'; +sum +--- + 0 +(1 row) + +step wi2: insert into tidrange_tbl values (4, 20); +step wi1: insert into tidrange_tbl values (3, 10); +step c2: commit; +step c1: commit; +ERROR: could not serialize access due to read/write dependencies among transactions + +starting permutation: rz1 rz2 wz1 wz2 c1 c2 +step rz1: select sum(p) from tidrange_empty_tbl + where ctid >= '(0,0)' and ctid < '(1,0)'; +sum +--- + +(1 row) + +step rz2: select sum(p) from tidrange_empty_tbl + where ctid >= '(0,0)' and ctid < '(1,0)'; +sum +--- + +(1 row) + +step wz1: insert into tidrange_empty_tbl values (3, 10); +step wz2: insert into tidrange_empty_tbl values (4, 20); +step c1: commit; +step c2: commit; +ERROR: could not serialize access due to read/write dependencies among transactions + +starting permutation: rz2 rz1 wz2 wz1 c2 c1 +step rz2: select sum(p) from tidrange_empty_tbl + where ctid >= '(0,0)' and ctid < '(1,0)'; +sum +--- + +(1 row) + +step rz1: select sum(p) from tidrange_empty_tbl + where ctid >= '(0,0)' and ctid < '(1,0)'; +sum +--- + +(1 row) + +step wz2: insert into tidrange_empty_tbl values (4, 20); +step wz1: insert into tidrange_empty_tbl values (3, 10); +step c2: commit; +step c1: 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 8470d50d2bc..317fccffa2a 100644 --- a/src/test/isolation/isolation_schedule +++ b/src/test/isolation/isolation_schedule @@ -108,6 +108,7 @@ test: vacuum-conflict test: vacuum-skip-locked test: stats test: horizons +test: predicate-tidrangescan test: predicate-hash test: predicate-gist test: predicate-gin diff --git a/src/test/isolation/specs/predicate-tidrangescan.spec b/src/test/isolation/specs/predicate-tidrangescan.spec new file mode 100644 index 00000000000..3716d9d2aa6 --- /dev/null +++ b/src/test/isolation/specs/predicate-tidrangescan.spec @@ -0,0 +1,73 @@ +# Test for relation level predicate locking in TID range scans +# +# A TID range scan reads a range of heap blocks directly, with no index +# involved, so like a sequential scan it has nothing finer to lock than the +# whole relation. Verify that the relation level SIREAD lock is acquired, by +# checking that write skew and phantom rows seen through a TID range scan are +# detected. + +setup +{ + create table tidrange_tbl (id int, p int); + insert into tidrange_tbl values (1, 0), (2, 0); + create table tidrange_empty_tbl (id int, p int); +} + +teardown +{ + drop table tidrange_tbl; + drop table tidrange_empty_tbl; +} + +session s1 +setup +{ + begin isolation level serializable; + set enable_seqscan = off; +} +step rxy1 { select sum(p) from tidrange_tbl + where ctid >= '(0,0)' and ctid < '(1,0)'; } +step wx1 { update tidrange_tbl set p = 1 where ctid = '(0,1)'; } +step wi1 { insert into tidrange_tbl values (3, 10); } +step rz1 { select sum(p) from tidrange_empty_tbl + where ctid >= '(0,0)' and ctid < '(1,0)'; } +step wz1 { insert into tidrange_empty_tbl values (3, 10); } +step c1 { commit; } + +session s2 +setup +{ + begin isolation level serializable; + set enable_seqscan = off; +} +step rxy2 { select sum(p) from tidrange_tbl + where ctid >= '(0,0)' and ctid < '(1,0)'; } +step wy2 { update tidrange_tbl set p = 1 where ctid = '(0,2)'; } +step wi2 { insert into tidrange_tbl values (4, 20); } +step rz2 { select sum(p) from tidrange_empty_tbl + where ctid >= '(0,0)' and ctid < '(1,0)'; } +step wz2 { insert into tidrange_empty_tbl values (4, 20); } +step c2 { commit; } + +# Both transactions read the whole TID range, then each updates a row that the +# other one read. In either serial order the second transaction would have +# read sum(p) = 1, so both reads returning 0 is not serializable and one of +# the transactions has to be aborted. (The final sum(p) = 2 is reachable +# serially; only the reads reveal the anomaly.) + +permutation rxy1 rxy2 wx1 wy2 c1 c2 +permutation rxy1 rxy2 wy2 wx1 c1 c2 +permutation rxy2 rxy1 wx1 wy2 c2 c1 + +# Both transactions read the whole TID range, then each inserts a row that +# falls inside the range the other one read. + +permutation rxy1 rxy2 wi1 wi2 c1 c2 +permutation rxy2 rxy1 wi2 wi1 c2 c1 + +# The same, but the relation is still empty when it is scanned. There are no +# existing pages, so a page level lock would have nothing to attach to; only a +# relation level lock can conflict with these inserts. + +permutation rz1 rz2 wz1 wz2 c1 c2 +permutation rz2 rz1 wz2 wz1 c2 c1 -- 2.43.0