From 729b40a99ef755ac9448e32a90139baa234b0d18 Mon Sep 17 00:00:00 2001 From: Aleksander Alekseev Date: Mon, 21 Sep 2026 14:31:46 +0300 Subject: [PATCH v4] Fix write skew under SERIALIZABLE for TID range scans A TID range scan reads heap blocks directly with no index involved, yet acquires no predicate lock at all, so concurrent SERIALIZABLE transactions scanning the same range fail to see the rw-conflicts between them and can both commit, producing write skew. Lock the whole relation, as a sequential scan 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. While on it, also add an explicit test that write skew is detected when the rows are read through a bitmap heap scan. Author: Jacob Brazeal Author: Aleksander Alekseev Reviewed-by: Andrey Borodin Reviewed-by: Andres Freund 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 + src/include/access/tableam.h | 7 +- .../expected/predicate-bitmap-scan.out | 43 +++++ .../expected/predicate-tid-range-scan.out | 148 ++++++++++++++++++ src/test/isolation/isolation_schedule | 2 + .../specs/predicate-bitmap-scan.spec | 39 +++++ .../specs/predicate-tid-range-scan.spec | 73 +++++++++ 8 files changed, 334 insertions(+), 14 deletions(-) create mode 100644 src/test/isolation/expected/predicate-bitmap-scan.out create mode 100644 src/test/isolation/expected/predicate-tid-range-scan.out create mode 100644 src/test/isolation/specs/predicate-bitmap-scan.spec create mode 100644 src/test/isolation/specs/predicate-tid-range-scan.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/include/access/tableam.h b/src/include/access/tableam.h index ea3f2a6be99..bead2e7b5e0 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -49,7 +49,12 @@ typedef enum ScanOptions { SO_NONE = 0, - /* one of SO_TYPE_* may be specified */ + /* + * One of SO_TYPE_* may be specified. When adding a scan type, check + * whether it must take predicate locks to be safe under SERIALIZABLE: see + * the flag test in heap_beginscan() and "Heap locking" in + * src/backend/storage/lmgr/README-SSI. + */ SO_TYPE_SEQSCAN = 1 << 0, SO_TYPE_BITMAPSCAN = 1 << 1, SO_TYPE_SAMPLESCAN = 1 << 2, diff --git a/src/test/isolation/expected/predicate-bitmap-scan.out b/src/test/isolation/expected/predicate-bitmap-scan.out new file mode 100644 index 00000000000..60b98b8bdd2 --- /dev/null +++ b/src/test/isolation/expected/predicate-bitmap-scan.out @@ -0,0 +1,43 @@ +Parsed test spec with 2 sessions + +starting permutation: r1 r2 w1 w2 c1 c2 +step r1: SELECT * FROM test WHERE i IN (5, 7); +i|t +-+----- +5|apple +7|pear +(2 rows) + +step r2: SELECT * FROM test WHERE i IN (5, 7); +i|t +-+----- +5|apple +7|pear +(2 rows) + +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 + +starting permutation: r2 r1 w2 w1 c2 c1 +step r2: SELECT * FROM test WHERE i IN (5, 7); +i|t +-+----- +5|apple +7|pear +(2 rows) + +step r1: SELECT * FROM test WHERE i IN (5, 7); +i|t +-+----- +5|apple +7|pear +(2 rows) + +step w2: UPDATE test SET t = 'apple_xact2' WHERE i = 5; +step w1: UPDATE test SET t = 'pear_xact1' WHERE i = 7; +step c2: COMMIT; +step c1: COMMIT; +ERROR: could not serialize access due to read/write dependencies among transactions diff --git a/src/test/isolation/expected/predicate-tid-range-scan.out b/src/test/isolation/expected/predicate-tid-range-scan.out new file mode 100644 index 00000000000..d60bdb28202 --- /dev/null +++ b/src/test/isolation/expected/predicate-tid-range-scan.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..5b9c534cb87 100644 --- a/src/test/isolation/isolation_schedule +++ b/src/test/isolation/isolation_schedule @@ -108,6 +108,8 @@ test: vacuum-conflict test: vacuum-skip-locked test: stats test: horizons +test: predicate-bitmap-scan +test: predicate-tid-range-scan test: predicate-hash test: predicate-gist test: predicate-gin diff --git a/src/test/isolation/specs/predicate-bitmap-scan.spec b/src/test/isolation/specs/predicate-bitmap-scan.spec new file mode 100644 index 00000000000..ba98468f64a --- /dev/null +++ b/src/test/isolation/specs/predicate-bitmap-scan.spec @@ -0,0 +1,39 @@ +# Test for write skew under SERIALIZABLE with a bitmap heap scan + +setup +{ + CREATE TABLE test (i int PRIMARY KEY, t text); + INSERT INTO test VALUES (5, 'apple'), (7, 'pear'), (11, 'banana'); +} + +teardown +{ + DROP TABLE test; +} + +session s1 +setup +{ + BEGIN ISOLATION LEVEL SERIALIZABLE; + SET enable_seqscan = off; + SET enable_indexscan = off; + SET enable_bitmapscan = on; +} +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 +setup +{ + BEGIN ISOLATION LEVEL SERIALIZABLE; + SET enable_seqscan = off; + SET enable_indexscan = off; + SET enable_bitmapscan = on; +} +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 r1 r2 w1 w2 c1 c2 +permutation r2 r1 w2 w1 c2 c1 diff --git a/src/test/isolation/specs/predicate-tid-range-scan.spec b/src/test/isolation/specs/predicate-tid-range-scan.spec new file mode 100644 index 00000000000..3716d9d2aa6 --- /dev/null +++ b/src/test/isolation/specs/predicate-tid-range-scan.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