public inbox for [email protected]
help / color / mirror / Atom feedFrom: Bernice Southey <[email protected]>
To: Tender Wang <[email protected]>
Cc: Dean Rasheed <[email protected]>
Cc: Amit Langote <[email protected]>
Cc: Bh W <[email protected]>
Cc: Laurenz Albe <[email protected]>
Cc: [email protected]
Subject: Re: BUG #19355: Attempt to insert data unexpectedly during concurrent update
Date: Wed, 31 Dec 2025 16:16:15 +0000
Message-ID: <CAEDh4nw=h5NihhuBgq4GFCfPP9vcg54QJ+K0XsG9S0owWDUhfA@mail.gmail.com> (raw)
In-Reply-To: <CAHewXN=D2SUh1CvE0gnk1Ev_k4RAEO9aoEgQZG_7eAuSWxZA0Q@mail.gmail.com>
References: <[email protected]>
<[email protected]>
<CANjBVm3AMm=9-mB7=uqJjaWN-+RrA8+w-gXhZn6L7VHu7K_Urw@mail.gmail.com>
<CAEZATCVcV4cfqGnmPGVxwjsPh7rxsTD510=KOGFyi2_FMgqWaQ@mail.gmail.com>
<CA+HiwqE8gNK7Utqo2EAKh24KExWfypt06m1EFNXBd5TFDO2f6w@mail.gmail.com>
<CAHewXNn8ApcYfZKjZ1R8Z7yHRoi+C+mu5r9LbZUbb6691d6Rrw@mail.gmail.com>
<CAEZATCVr+=F=iTbi3XyABSEr5Cqw=ptm2PyGc75SaQ1JYrPnsg@mail.gmail.com>
<CAHewXN=D2SUh1CvE0gnk1Ev_k4RAEO9aoEgQZG_7eAuSWxZA0Q@mail.gmail.com>
Hi,
I've spent some time looking at the code and history of this bug
because I also ran into it [1] and I was deeply curious why concurrent
updates worked with a hash join + seq scan but not a nested loop +
index scan.
I was also curious if this bug is why Yuri didn't see deadlocks in
this related bug [2].
FWIW, I found a simpler way to reproduce this:
create table t(id int primary key, count int);
insert into t values (1, 0), (2, 0);
--session 1
begin;
update t set count = count + 1;
--session 2
set enable_seqscan = off;
update t set count = count + 1 from (values (1), (2)) v(id) where t.id = v.id;
Only one update succeeds in session 2.
Tender Wang wrote:
> Dean Rasheed wrote:
>> I'm somewhat conflicted as to which approach is better. I think maybe
>> there is less chance of other unintended side-effects if the set of
>> RTEs included in PlannerGlobal.allRelids, unprunableRelids, and
>> es_unpruned_relids is not changed. However, as it stands,
>> PlannerGlobal.allRelids is misnamed (it should probably have been
>> called "relationRelids", in line with the "relationOids" field).
>> Making it actually include all RTEs would solve that.
> Per the comment, I guess Amit didn't plan to include all RTEs in his design.
While digging, I noticed that allRelids seems only to be used to
create unprunableRelids. But unprunableRelids is used among other
things, to create es_unpruned_relids and as an unpruned_relids
parameter to ExecInitRangeTable. It's difficult to follow all the
paths where the missing relids will matter. So I think Tender's
approach is safer if amended to include all relids. I'm very new to
postgres source code, so this is a very tentative opinion. Amit can
best answer after his holiday.
I was curious enough to test in the meantime. I made an experimental
patch that just adds all rels to allRelids. This fixed all the issues
I'm aware of. It caused overexplain tests to fail by adding in new
non-relation relIds as expected. Apologies if I'm getting the
terminology wrong.
I ran Yuri's reproducer on my patched branch and indeed got several deadlocks.
psql:crash.sql:3: ERROR: deadlock detected
DETAIL: Process 71673 waits for ShareLock on transaction 2766;
blocked by process 71643.
Process 71643 waits for ShareLock on transaction 2777; blocked by process 71673.
Curiously, in the test I added (based on Dean's), enable_indexscan or
enable_seqscan didn't make any difference to the plan. But when I run
the reproducer manually, it does change the plan. Since the bug
disappears with a hash join + seq scan, the plan is critical. So I
included 'set enable_seqscan to 0' for reference and future proofing.
I didn't attempt to fix the comment because I don't understand it. I
also didn't check all my tabs vs spaces, or run TAP tests, as this
patch is just meant for info. Most of the code is still way over my
head, so my apologies if none of this is helpful.
As to why this works for a hash join but not a nested loop, I have
some ideas, but I'm still digging.
Happy New Year!
Bernice
[1] https://www.postgresql.org/message-id/flat/CAMbWs4-uC8DWRZvByHej%2B9E5em7V_%2BzNyRfFx-UUpR4HwifK4w%4...
[2] https://www.postgresql.org/message-id/ly6rss443iajiv5cfypzu6fgrmxdbwzdtuujkddr4eis7s2gcq%40gcdytiget...
Attachments:
[text/x-patch] concurrent-update-experiment.patch (5.4K, 2-concurrent-update-experiment.patch)
download | inline diff:
From 54526773321a525a0e806bf17353537a4bc23fef Mon Sep 17 00:00:00 2001
From: Bernice Southey <[email protected]>
Date: Wed, 31 Dec 2025 14:26:39 +0000
Subject: [PATCH] concurrent update experiment
---
.../pg_overexplain/expected/pg_overexplain.out | 9 +++++----
src/backend/optimizer/plan/setrefs.c | 5 +++--
src/test/isolation/expected/eval-plan-qual.out | 17 +++++++++++++++++
src/test/isolation/specs/eval-plan-qual.spec | 10 ++++++++++
4 files changed, 35 insertions(+), 6 deletions(-)
diff --git a/contrib/pg_overexplain/expected/pg_overexplain.out b/contrib/pg_overexplain/expected/pg_overexplain.out
index 55d34666d87..45a71af5776 100644
--- a/contrib/pg_overexplain/expected/pg_overexplain.out
+++ b/contrib/pg_overexplain/expected/pg_overexplain.out
@@ -47,7 +47,8 @@ EXPLAIN (RANGE_TABLE) SELECT 1;
RTIs: 1
RTI 1 (result):
Eref: "*RESULT*" ()
-(4 rows)
+ Unprunable RTIs: 1
+(5 rows)
-- Create a partitioned table.
CREATE TABLE vegetables (id serial, name text, genus text)
@@ -141,7 +142,7 @@ $$);
Relation: daucus
Relation Kind: relation
Relation Lock Mode: AccessShareLock
- Unprunable RTIs: 1 3 4
+ Unprunable RTIs: 1 2 3 4
(53 rows)
-- Test a different output format.
@@ -292,7 +293,7 @@ $$);
<Security-Barrier>false</Security-Barrier> +
<Lateral>false</Lateral> +
</Range-Table-Entry> +
- <Unprunable-RTIs>1 3 4</Unprunable-RTIs> +
+ <Unprunable-RTIs>1 2 3 4</Unprunable-RTIs> +
<Result-RTIs>none</Result-RTIs> +
</Range-Table> +
</Query> +
@@ -485,7 +486,7 @@ INSERT INTO vegetables (name, genus) VALUES ('broccoflower', 'brassica');
Permission Info Index: 1
RTI 2 (result):
Eref: "*RESULT*" ()
- Unprunable RTIs: 1
+ Unprunable RTIs: 1 2
Result RTIs: 1
(15 rows)
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index cd7ea1e6b58..4bfcbf4efb5 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -580,12 +580,13 @@ add_rte_to_flat_rtable(PlannerGlobal *glob, List *rteperminfos,
* Note we don't bother to avoid making duplicate list entries. We could,
* but it would probably cost more cycles than it would save.
*/
+ glob->allRelids = bms_add_member(glob->allRelids,
+ list_length(glob->finalrtable));
+
if (newrte->rtekind == RTE_RELATION ||
(newrte->rtekind == RTE_SUBQUERY && OidIsValid(newrte->relid)))
{
glob->relationOids = lappend_oid(glob->relationOids, newrte->relid);
- glob->allRelids = bms_add_member(glob->allRelids,
- list_length(glob->finalrtable));
}
/*
diff --git a/src/test/isolation/expected/eval-plan-qual.out b/src/test/isolation/expected/eval-plan-qual.out
index 05fffe0d570..d182bbe2722 100644
--- a/src/test/isolation/expected/eval-plan-qual.out
+++ b/src/test/isolation/expected/eval-plan-qual.out
@@ -1432,6 +1432,23 @@ a|b|c| d
(2 rows)
+starting permutation: updateforvalues updatevalues c1 c2 read
+step updateforvalues: UPDATE accounts SET balance = balance + 100;
+step updatevalues:
+ set local enable_seqscan to 0;
+ UPDATE accounts SET balance = v.balance FROM (VALUES('checking', 610), ('savings', 620)) v(accountid, balance) WHERE accounts.accountid = v.accountid;
+ <waiting ...>
+step c1: COMMIT;
+step updatevalues: <... completed>
+step c2: COMMIT;
+step read: SELECT * FROM accounts ORDER BY accountid;
+accountid|balance|balance2
+---------+-------+--------
+checking | 610| 1220
+savings | 620| 1240
+(2 rows)
+
+
starting permutation: sys1 sysupd2 c1 c2
step sys1:
UPDATE pg_class SET reltuples = 123 WHERE oid = 'accounts'::regclass;
diff --git a/src/test/isolation/specs/eval-plan-qual.spec b/src/test/isolation/specs/eval-plan-qual.spec
index 80e1e6bb307..4870690ac5b 100644
--- a/src/test/isolation/specs/eval-plan-qual.spec
+++ b/src/test/isolation/specs/eval-plan-qual.spec
@@ -206,6 +206,8 @@ step sys1 {
step s1pp1 { UPDATE another_parttbl SET b = b + 1 WHERE a = 1; }
+step updateforvalues { UPDATE accounts SET balance = balance + 100; }
+
session s2
setup { BEGIN ISOLATION LEVEL READ COMMITTED; }
step wx2 { UPDATE accounts SET balance = balance + 450 WHERE accountid = 'checking' RETURNING balance; }
@@ -318,6 +320,11 @@ step s2pp2 { PREPARE epd AS DELETE FROM another_parttbl WHERE a = $1; }
step s2pp3 { EXECUTE epd(1); }
step s2pp4 { DELETE FROM another_parttbl WHERE a = (SELECT 1); }
+step updatevalues {
+ set local enable_seqscan to 0;
+ UPDATE accounts SET balance = v.balance FROM (VALUES('checking', 610), ('savings', 620)) v(accountid, balance) WHERE accounts.accountid = v.accountid;
+}
+
session s3
setup { BEGIN ISOLATION LEVEL READ COMMITTED; }
step read { SELECT * FROM accounts ORDER BY accountid; }
@@ -419,6 +426,9 @@ permutation simplepartupdate_route1to2 complexpartupdate_route_err1 c1 c2 read_p
permutation simplepartupdate_noroute complexpartupdate_route c1 c2 read_part
permutation simplepartupdate_noroute complexpartupdate_doesnt_route c1 c2 read_part
+# test EPQ recheck in UPDATE from VALUES_RTE, cf bug #19355
+permutation updateforvalues updatevalues c1 c2 read
+
permutation sys1 sysupd2 c1 c2
permutation sys1 sysmerge2 c1 c2
--
2.43.0
view thread (12+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: BUG #19355: Attempt to insert data unexpectedly during concurrent update
In-Reply-To: <CAEDh4nw=h5NihhuBgq4GFCfPP9vcg54QJ+K0XsG9S0owWDUhfA@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox