postgres.git / summary / log / commit / refs
commit fb60892f4034017e77b8d4376d3ca8937dcbbab2
Author: Dean Rasheed <dean.a.rasheed@gmail.com>
Date: Tue Sep 15 10:47:05 2026 +0000
Fix missing SIREAD lock on the row found by ON CONFLICT.
INSERT ... ON CONFLICT decides what to do based on the conflicting row
found by the arbiter index probe, but SSI never saw that read: the
probe runs with a dirty snapshot, which predicate locking ignores, and
the later fetch of the row uses SnapshotAny. When the statement then
writes nothing, as with DO NOTHING, DO UPDATE with a WHERE clause
rejecting the row, or DO SELECT, nothing records the read at all. A
concurrent writer of that row went unnoticed and write skew could
commit at SERIALIZABLE, even though the same schedule with a plain
SELECT of the row fails with a serialization error.
To fix, read the conflicting tuple again with the query snapshot,
right where the probe finds it. The table AM takes the SIREAD lock
and checks for a concurrent writer of the tuple as part of that read,
both under the buffer lock, so a writer either sees the lock or is
seen. A predicate lock by itself acquired separately after the probe
could not offer that: a writer passing its conflict check in between
would be missed. Doing this in the probe covers every conflict
action, including rows that the WHERE clause of DO UPDATE or DO SELECT
then rejects.
The DO NOTHING and DO UPDATE cases have been broken since ON CONFLICT
was added in 9.5; DO SELECT is new in v19. Backpatch to all supported
branches.
Author: Zsolt Parragi <zsolt.parragi@percona.com>
Author: Andrey Borodin <x4mmm@yandex-team.ru>
Reported-by: Andrey Borodin <x4mmm@yandex-team.ru>
Reported-by: Zsolt Parragi <zsolt.parragi@percona.com>
Discussion: https://postgr.es/m/787936C5-4155-4CF9-939D-39DC0EC1C892@yandex-team.ru
Discussion: https://postgr.es/m/CAN4CZFM1GkHJkpMeo4G5rxtacVsfeKCJYiik9E9AKX1E9VYQ1w@mail.gmail.com
Backpatch-through: 14
src/backend/executor/execIndexing.c | 19 ++++
.../expected/insert-conflict-serializable.out | 116 +++++++++++++++++++++
src/test/isolation/isolation_schedule | 1 +
.../specs/insert-conflict-serializable.spec | 71 +++++++++++++
src/test/modules/injection_points/Makefile | 3 +-
.../expected/on_conflict_probe_window.out | 113 ++++++++++++++++++++
src/test/modules/injection_points/meson.build | 1 +
.../specs/on_conflict_probe_window.spec | 57 ++++++++++
8 files changed, 380 insertions(+), 1 deletion(-)
diff --git a/src/backend/executor/execIndexing.c b/src/backend/executor/execIndexing.c
index eb383812901..1e3a86aca1b 100644
--- a/src/backend/executor/execIndexing.c
+++ b/src/backend/executor/execIndexing.c
@@ -908,7 +908,26 @@ retry:
{
conflict = true;
if (conflictTid)
+ {
*conflictTid = existing_slot->tts_tid;
+
+ /*
+ * The conflicting tuple decides the outcome of INSERT ... ON
+ * CONFLICT, so for SSI purposes it has been read, even when
+ * nothing gets written afterwards. The dirty snapshot used
+ * by the scan is not an MVCC snapshot, so SSI ignored that
+ * read. Read the tuple again with the query snapshot to
+ * record it. The result is of no interest here, the caller
+ * checks visibility itself.
+ */
+ if (IsolationIsSerializable())
+ {
+ INJECTION_POINT("check-exclusion-or-unique-constraint-conflict", NULL);
+ (void) table_tuple_fetch_row_version(heap, conflictTid,
+ estate->es_snapshot,
+ existing_slot);
+ }
+ }
break;
}
diff --git a/src/test/isolation/expected/insert-conflict-serializable.out b/src/test/isolation/expected/insert-conflict-serializable.out
new file mode 100644
index 00000000000..3ce77f94f1a
--- /dev/null
+++ b/src/test/isolation/expected/insert-conflict-serializable.out
@@ -0,0 +1,116 @@
+Parsed test spec with 2 sessions
+
+starting permutation: ioc_nothing1 count2 delete2 insert1 c1 c2
+step ioc_nothing1: INSERT INTO ioc_a VALUES (1, 99) ON CONFLICT (key) DO NOTHING;
+step count2: SELECT count(*) FROM ioc_b;
+count
+-----
+ 0
+(1 row)
+
+step delete2: DELETE FROM ioc_a WHERE key = 1;
+step insert1: INSERT INTO ioc_b VALUES (1, 10);
+step c1: COMMIT;
+step c2: COMMIT;
+ERROR: could not serialize access due to read/write dependencies among transactions
+
+starting permutation: ioc_nothing1 count2 update2 insert1 c1 c2
+step ioc_nothing1: INSERT INTO ioc_a VALUES (1, 99) ON CONFLICT (key) DO NOTHING;
+step count2: SELECT count(*) FROM ioc_b;
+count
+-----
+ 0
+(1 row)
+
+step update2: UPDATE ioc_a SET val = 1 WHERE key = 1;
+step insert1: INSERT INTO ioc_b VALUES (1, 10);
+step c1: COMMIT;
+step c2: COMMIT;
+ERROR: could not serialize access due to read/write dependencies among transactions
+
+starting permutation: ioc_update1_where count2 update2 insert1 c1 c2
+step ioc_update1_where: INSERT INTO ioc_a VALUES (1, 99) ON CONFLICT (key) DO UPDATE SET val = 99 WHERE ioc_a.val > 100;
+step count2: SELECT count(*) FROM ioc_b;
+count
+-----
+ 0
+(1 row)
+
+step update2: UPDATE ioc_a SET val = 1 WHERE key = 1; <waiting ...>
+step insert1: INSERT INTO ioc_b VALUES (1, 10);
+step c1: COMMIT;
+step update2: <... completed>
+ERROR: could not serialize access due to read/write dependencies among transactions
+step c2: COMMIT;
+
+starting permutation: ioc_select1 count2 update2 insert1 c1 c2
+step ioc_select1: INSERT INTO ioc_a VALUES (1, 99) ON CONFLICT (key) DO SELECT RETURNING val;
+val
+---
+ 0
+(1 row)
+
+step count2: SELECT count(*) FROM ioc_b;
+count
+-----
+ 0
+(1 row)
+
+step update2: UPDATE ioc_a SET val = 1 WHERE key = 1;
+step insert1: INSERT INTO ioc_b VALUES (1, 10);
+step c1: COMMIT;
+step c2: COMMIT;
+ERROR: could not serialize access due to read/write dependencies among transactions
+
+starting permutation: ioc_select1_keyshare count2 update2 insert1 c1 c2
+step ioc_select1_keyshare: INSERT INTO ioc_a VALUES (1, 99) ON CONFLICT (key) DO SELECT FOR KEY SHARE RETURNING val;
+val
+---
+ 0
+(1 row)
+
+step count2: SELECT count(*) FROM ioc_b;
+count
+-----
+ 0
+(1 row)
+
+step update2: UPDATE ioc_a SET val = 1 WHERE key = 1;
+step insert1: INSERT INTO ioc_b VALUES (1, 10);
+step c1: COMMIT;
+step c2: COMMIT;
+ERROR: could not serialize access due to read/write dependencies among transactions
+
+starting permutation: ioc_select1_where count2 update2 insert1 c1 c2
+step ioc_select1_where: INSERT INTO ioc_a VALUES (1, 99) ON CONFLICT (key) DO SELECT WHERE ioc_a.val > 100 RETURNING val;
+val
+---
+(0 rows)
+
+step count2: SELECT count(*) FROM ioc_b;
+count
+-----
+ 0
+(1 row)
+
+step update2: UPDATE ioc_a SET val = 1 WHERE key = 1;
+step insert1: INSERT INTO ioc_b VALUES (1, 10);
+step c1: COMMIT;
+step c2: COMMIT;
+ERROR: could not serialize access due to read/write dependencies among transactions
+
+starting permutation: count2 update2 ioc_select1 c2 insert1 c1
+step count2: SELECT count(*) FROM ioc_b;
+count
+-----
+ 0
+(1 row)
+
+step update2: UPDATE ioc_a SET val = 1 WHERE key = 1;
+step ioc_select1: INSERT INTO ioc_a VALUES (1, 99) ON CONFLICT (key) DO SELECT RETURNING val; <waiting ...>
+step c2: COMMIT;
+step ioc_select1: <... completed>
+ERROR: could not serialize access due to concurrent update
+step insert1: INSERT INTO ioc_b VALUES (1, 10);
+ERROR: current transaction is aborted, commands ignored until end of transaction block
+step c1: COMMIT;
diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule
index 63bd57919bf..8470d50d2bc 100644
--- a/src/test/isolation/isolation_schedule
+++ b/src/test/isolation/isolation_schedule
@@ -58,6 +58,7 @@ test: insert-conflict-do-update-3
test: insert-conflict-do-update-4
test: insert-conflict-specconflict
test: insert-conflict-do-select
+test: insert-conflict-serializable
test: merge-insert-update
test: merge-delete
test: merge-update
diff --git a/src/test/isolation/specs/insert-conflict-serializable.spec b/src/test/isolation/specs/insert-conflict-serializable.spec
new file mode 100644
index 00000000000..c177003279b
--- /dev/null
+++ b/src/test/isolation/specs/insert-conflict-serializable.spec
@@ -0,0 +1,71 @@
+# INSERT ... ON CONFLICT at SERIALIZABLE
+#
+# The conflicting row decides the outcome of the statement, so it counts
+# as a read for SSI purposes, whether or not the statement then writes
+# anything: a concurrent transaction writing that row must create a
+# rw-antidependency. These permutations build the classic write-skew
+# cycle: s1 reads a and writes b, while s2 reads b and writes a. One of
+# the two transactions must fail with a serialization error.
+
+setup
+{
+ CREATE TABLE ioc_a (key int PRIMARY KEY, val int);
+ CREATE TABLE ioc_b (key int PRIMARY KEY, val int);
+ INSERT INTO ioc_a VALUES (1, 0);
+}
+
+teardown
+{
+ DROP TABLE ioc_a, ioc_b;
+}
+
+session s1
+setup
+{
+ BEGIN ISOLATION LEVEL SERIALIZABLE;
+}
+step ioc_nothing1 { INSERT INTO ioc_a VALUES (1, 99) ON CONFLICT (key) DO NOTHING; }
+step ioc_update1_where { INSERT INTO ioc_a VALUES (1, 99) ON CONFLICT (key) DO UPDATE SET val = 99 WHERE ioc_a.val > 100; }
+step ioc_select1 { INSERT INTO ioc_a VALUES (1, 99) ON CONFLICT (key) DO SELECT RETURNING val; }
+step ioc_select1_keyshare { INSERT INTO ioc_a VALUES (1, 99) ON CONFLICT (key) DO SELECT FOR KEY SHARE RETURNING val; }
+step ioc_select1_where { INSERT INTO ioc_a VALUES (1, 99) ON CONFLICT (key) DO SELECT WHERE ioc_a.val > 100 RETURNING val; }
+step insert1 { INSERT INTO ioc_b VALUES (1, 10); }
+step c1 { COMMIT; }
+
+session s2
+setup
+{
+ BEGIN ISOLATION LEVEL SERIALIZABLE;
+}
+step count2 { SELECT count(*) FROM ioc_b; }
+step update2 { UPDATE ioc_a SET val = 1 WHERE key = 1; }
+step delete2 { DELETE FROM ioc_a WHERE key = 1; }
+step c2 { COMMIT; }
+
+# DO NOTHING skips the insert because of the existing row, which s2 then
+# deletes or updates: s2 must fail to commit
+permutation ioc_nothing1 count2 delete2 insert1 c1 c2
+permutation ioc_nothing1 count2 update2 insert1 c1 c2
+
+# DO UPDATE with a WHERE clause rejecting the existing row writes nothing,
+# but the row still decided the outcome: s2 must fail
+permutation ioc_update1_where count2 update2 insert1 c1 c2
+
+# DO SELECT returns the existing row: s2 must fail
+permutation ioc_select1 count2 update2 insert1 c1 c2
+
+# DO SELECT FOR KEY SHARE: the non-key update does not conflict with the
+# tuple lock, so update2 proceeds without blocking and only the SIREAD
+# lock makes s2 fail
+permutation ioc_select1_keyshare count2 update2 insert1 c1 c2
+
+# DO SELECT with a WHERE clause rejecting the existing row: the row is not
+# returned, but it decided the outcome and was examined by the WHERE
+# clause, so it still counts as a read
+permutation ioc_select1_where count2 update2 insert1 c1 c2
+
+# If the update is already in flight when DO SELECT runs, the arbiter
+# probe waits for it to commit, leaving a conflicting row that is not
+# visible to the query snapshot. The row cannot be returned, so DO
+# SELECT must fail instead
+permutation count2 update2 ioc_select1 c2 insert1 c1
diff --git a/src/test/modules/injection_points/Makefile b/src/test/modules/injection_points/Makefile
index 408a35c3c21..9d8b4b3540c 100644
--- a/src/test/modules/injection_points/Makefile
+++ b/src/test/modules/injection_points/Makefile
@@ -26,7 +26,8 @@ ISOLATION = basic \
ri_fastpath_snapshot \
syscache-update-pruned \
wait_cleanup \
- heap_lock_update
+ heap_lock_update \
+ on_conflict_probe_window
# some isolation tests require wal_level=replica
ISOLATION_OPTS = --temp-config $(top_srcdir)/src/test/modules/injection_points/extra.conf
diff --git a/src/test/modules/injection_points/expected/on_conflict_probe_window.out b/src/test/modules/injection_points/expected/on_conflict_probe_window.out
new file mode 100644
index 00000000000..964a98a3aff
--- /dev/null
+++ b/src/test/modules/injection_points/expected/on_conflict_probe_window.out
@@ -0,0 +1,113 @@
+Parsed test spec with 2 sessions
+
+starting permutation: ioc_nothing1 count2 update2 c2 wake2 insert1 c1
+injection_points_attach
+-----------------------
+
+(1 row)
+
+step ioc_nothing1: INSERT INTO probe_a VALUES (1, 99) ON CONFLICT (key) DO NOTHING; <waiting ...>
+step count2: SELECT count(*) FROM probe_b;
+count
+-----
+ 0
+(1 row)
+
+step update2: UPDATE probe_a SET val = 1 WHERE key = 1;
+step c2: COMMIT;
+step wake2:
+ SELECT injection_points_detach('check-exclusion-or-unique-constraint-conflict');
+ SELECT injection_points_wakeup('check-exclusion-or-unique-constraint-conflict');
+
+injection_points_detach
+-----------------------
+
+(1 row)
+
+injection_points_wakeup
+-----------------------
+
+(1 row)
+
+step ioc_nothing1: <... completed>
+step insert1: INSERT INTO probe_b VALUES (1, 10);
+ERROR: could not serialize access due to read/write dependencies among transactions
+step c1: COMMIT;
+
+starting permutation: ioc_select1 count2 update2 c2 wake2 insert1 c1
+injection_points_attach
+-----------------------
+
+(1 row)
+
+step ioc_select1: INSERT INTO probe_a VALUES (1, 99) ON CONFLICT (key) DO SELECT RETURNING val; <waiting ...>
+step count2: SELECT count(*) FROM probe_b;
+count
+-----
+ 0
+(1 row)
+
+step update2: UPDATE probe_a SET val = 1 WHERE key = 1;
+step c2: COMMIT;
+step wake2:
+ SELECT injection_points_detach('check-exclusion-or-unique-constraint-conflict');
+ SELECT injection_points_wakeup('check-exclusion-or-unique-constraint-conflict');
+
+injection_points_detach
+-----------------------
+
+(1 row)
+
+injection_points_wakeup
+-----------------------
+
+(1 row)
+
+step ioc_select1: <... completed>
+val
+---
+ 0
+(1 row)
+
+step insert1: INSERT INTO probe_b VALUES (1, 10);
+ERROR: could not serialize access due to read/write dependencies among transactions
+step c1: COMMIT;
+
+starting permutation: ioc_select1_keyshare count2 update2 c2 wake2 insert1 c1
+injection_points_attach
+-----------------------
+
+(1 row)
+
+step ioc_select1_keyshare: INSERT INTO probe_a VALUES (1, 99) ON CONFLICT (key) DO SELECT FOR KEY SHARE RETURNING val; <waiting ...>
+step count2: SELECT count(*) FROM probe_b;
+count
+-----
+ 0
+(1 row)
+
+step update2: UPDATE probe_a SET val = 1 WHERE key = 1;
+step c2: COMMIT;
+step wake2:
+ SELECT injection_points_detach('check-exclusion-or-unique-constraint-conflict');
+ SELECT injection_points_wakeup('check-exclusion-or-unique-constraint-conflict');
+
+injection_points_detach
+-----------------------
+
+(1 row)
+
+injection_points_wakeup
+-----------------------
+
+(1 row)
+
+step ioc_select1_keyshare: <... completed>
+val
+---
+ 0
+(1 row)
+
+step insert1: INSERT INTO probe_b VALUES (1, 10);
+ERROR: could not serialize access due to read/write dependencies among transactions
+step c1: COMMIT;
diff --git a/src/test/modules/injection_points/meson.build b/src/test/modules/injection_points/meson.build
index a7b40e084f6..80a09f34d78 100644
--- a/src/test/modules/injection_points/meson.build
+++ b/src/test/modules/injection_points/meson.build
@@ -56,6 +56,7 @@ tests += {
'syscache-update-pruned',
'wait_cleanup',
'heap_lock_update',
+ 'on_conflict_probe_window',
],
'runningcheck': false, # see syscache-update-pruned
# Some tests wait for all snapshots, so avoid parallel execution
diff --git a/src/test/modules/injection_points/specs/on_conflict_probe_window.spec b/src/test/modules/injection_points/specs/on_conflict_probe_window.spec
new file mode 100644
index 00000000000..78dddbe8b92
--- /dev/null
+++ b/src/test/modules/injection_points/specs/on_conflict_probe_window.spec
@@ -0,0 +1,57 @@
+# INSERT ... ON CONFLICT arbiter probe vs. a concurrent writer at SERIALIZABLE
+#
+# The arbiter probe reads the conflicting row to decide the statement's
+# outcome, so the read must be visible to SSI atomically: any SIREAD lock
+# taken after the probe leaves a window in which a concurrent writer of
+# the row checks for conflicts without seeing it. The injection point
+# holds the statement right after the probe picked the conflicting tuple,
+# while s2 updates the row and commits. ON CONFLICT then decides based
+# on a row version that s2 already replaced, completing the write-skew
+# cycle: s1 reads a and writes b, s2 reads b and writes a. s1 must fail.
+# DO SELECT without a lock and with FOR KEY SHARE (which does not conflict
+# with the non-key update) rely on SSI alone to notice this.
+
+setup
+{
+ CREATE EXTENSION injection_points;
+ CREATE TABLE probe_a (key int PRIMARY KEY, val int);
+ CREATE TABLE probe_b (key int PRIMARY KEY, val int);
+ INSERT INTO probe_a VALUES (1, 0);
+ -- attached globally, the point has to fire in session s1
+ SELECT injection_points_attach('check-exclusion-or-unique-constraint-conflict', 'wait');
+}
+
+teardown
+{
+ DROP TABLE probe_a, probe_b;
+ DROP EXTENSION injection_points;
+}
+
+session s1
+setup
+{
+ BEGIN ISOLATION LEVEL SERIALIZABLE;
+}
+step ioc_nothing1 { INSERT INTO probe_a VALUES (1, 99) ON CONFLICT (key) DO NOTHING; }
+step ioc_select1 { INSERT INTO probe_a VALUES (1, 99) ON CONFLICT (key) DO SELECT RETURNING val; }
+step ioc_select1_keyshare { INSERT INTO probe_a VALUES (1, 99) ON CONFLICT (key) DO SELECT FOR KEY SHARE RETURNING val; }
+step insert1 { INSERT INTO probe_b VALUES (1, 10); }
+step c1 { COMMIT; }
+
+session s2
+setup
+{
+ BEGIN ISOLATION LEVEL SERIALIZABLE;
+}
+step count2 { SELECT count(*) FROM probe_b; }
+step update2 { UPDATE probe_a SET val = 1 WHERE key = 1; }
+step c2 { COMMIT; }
+step wake2
+{
+ SELECT injection_points_detach('check-exclusion-or-unique-constraint-conflict');
+ SELECT injection_points_wakeup('check-exclusion-or-unique-constraint-conflict');
+}
+
+permutation ioc_nothing1 count2 update2 c2 wake2 insert1 c1
+permutation ioc_select1 count2 update2 c2 wake2 insert1 c1
+permutation ioc_select1_keyshare count2 update2 c2 wake2 insert1 c1
[parent: a4b26b8f7cd0]