Subject: BUG #19355: Attempt to insert data unexpectedly during concurrent update

From: PG Bug reporting form <[email-masked]>
Date: Mon, 15 Dec 2025 01:40:55 +0000

The following bug has been logged on the website:

Bug reference:      19355
Logged by:          Bihua Wang
Email address:      [email-masked]
PostgreSQL version: 18.1
Operating system:   linux
Description:        

Start two transaction and update on same tuple, raise concurrent update and
evalplanqual.  It will be found out that the session with evalplanqual did
not successfully update the data, but instead attempted to insert a row of
data incorrectly.

-- postgresql version
psql (18.1)
Type "help" for help.

postgres=# SELECT version();
                                                 version
---------------------------------------------------------------------------------------------------------
 PostgreSQL 18.1 on x86_64-pc-linux-gnu, compiled by gcc (GCC) 4.8.5
20150623 (EulerOS 4.8.5-28), 64-bit

-- prepare relation
drop table t1,t2,t3;
create table t1(a int primary key, b int);
create table t2(a int, b int);
create table t3(a int, b int);

insert into t1 values(1,0),(2,0);
insert into t2 values(1,1),(2,2);
insert into t3 values(3,3);
-- test sql
merge /*+ nestloop(t3 t1)) */ into t1 using
        (select t2.a as a from t2 group by t2.a) as t3
        on (t1.a = t3.a)
when matched then
        update set b = t1.b + 1
when not matched then
        insert (a,b) values (1,1);

--make sure plan like this, may need to execute  "set enable_hashjoin=off"
if necessay
postgres=*#  explain merge /*+ nestloop(t3 t1)) */ into t1 using
        (select t2.a as a from t2 group by t2.a) as t3
        on (t1.a = t3.a)
when matched then
        update set b = t1.b + 1
when not matched then
        insert (a,b) values (1,1);
postgres-*# postgres-*# postgres-*# postgres-*# postgres-*# postgres-*#
QUERY PLAN
-------------------------------------------------------------------------------
 Merge on t1  (cost=38.41..124.75 rows=0 width=0)
   ->  Nested Loop Left Join  (cost=38.41..124.75 rows=200 width=34)
         ->  Subquery Scan on t3  (cost=38.25..42.25 rows=200 width=32)
               ->  HashAggregate  (cost=38.25..40.25 rows=200 width=4)
                     Group Key: t2.a
                     ->  Seq Scan on t2  (cost=0.00..32.60 rows=2260
width=4)
         ->  Index Scan using t1_pkey on t1  (cost=0.15..0.41 rows=1
width=10)
               Index Cond: (a = t3.a)
(8 rows)

-- session 1:
postgres=# begin;
BEGIN
postgres=*#  explain merge /*+ nestloop(t3 t1)) */ into t1 using
        (select t2.a as a from t2 group by t2.a) as t3
        on (t1.a = t3.a)
when matched then
        update set b = t1.b + 1
when not matched then
        insert (a,b) values (1,1);
postgres-*# postgres-*# postgres-*# postgres-*# postgres-*# postgres-*#
QUERY PLAN
-------------------------------------------------------------------------------
 Merge on t1  (cost=38.41..124.75 rows=0 width=0)
   ->  Nested Loop Left Join  (cost=38.41..124.75 rows=200 width=34)
         ->  Subquery Scan on t3  (cost=38.25..42.25 rows=200 width=32)
               ->  HashAggregate  (cost=38.25..40.25 rows=200 width=4)
                     Group Key: t2.a
                     ->  Seq Scan on t2  (cost=0.00..32.60 rows=2260
width=4)
         ->  Index Scan using t1_pkey on t1  (cost=0.15..0.41 rows=1
width=10)
               Index Cond: (a = t3.a)
(8 rows)

postgres=*# merge /*+ nestloop(t3 t1)) */ into t1 using
        (select t2.a as a from t2 group by t2.a) as t3
        on (t1.a = t3.a)
when matched then
        update set b = t1.b + 1
when not matched then
        insert (a,b) values (1,1);
postgres-*# postgres-*# postgres-*# postgres-*# postgres-*# postgres-*#
MERGE 2
postgres=*# end;
COMMIT

-- session 2
postgres=# begin;
BEGIN
postgres=*# explain merge /*+ nestloop(t3 t1)) */ into t1 using
        (select t2.a as a from t2 group by t2.a) as t3
        on (t1.a = t3.a)
when matched then
        update set b = t1.b + 1
when not matched then
        insert (a,b) values (1,1);
postgres-*# postgres-*# postgres-*# postgres-*# postgres-*# postgres-*#
QUERY PLAN
-------------------------------------------------------------------------------
 Merge on t1  (cost=38.41..124.75 rows=0 width=0)
   ->  Nested Loop Left Join  (cost=38.41..124.75 rows=200 width=34)
         ->  Subquery Scan on t3  (cost=38.25..42.25 rows=200 width=32)
               ->  HashAggregate  (cost=38.25..40.25 rows=200 width=4)
                     Group Key: t2.a
                     ->  Seq Scan on t2  (cost=0.00..32.60 rows=2260
width=4)
         ->  Index Scan using t1_pkey on t1  (cost=0.15..0.41 rows=1
width=10)
               Index Cond: (a = t3.a)
(8 rows)

postgres=*# merge /*+ nestloop(t3 t1)) */ into t1 using
        (select t2.a as a from t2 group by t2.a) as t3
        on (t1.a = t3.a)
when matched then
        update set b = t1.b + 1
when not matched then
        insert (a,b) values (1,1);
postgres-*# postgres-*# postgres-*# postgres-*# postgres-*# postgres-*#
ERROR:  duplicate key value violates unique constraint "t1_pkey"
DETAIL:  Key (a)=(1) already exists.

---

From: Laurenz Albe <[email-masked]>
Date: Mon, 15 Dec 2025 07:25:18 +0100

> not successfully update the data, but instead attempted to insert a row of

I'd say that is expected.

If you need a guarantee that either INSERT or UPDATE succeed, you have to use
INSERT ... ON CONFLICT ... DO UPDATE

Yours,
Laurenz Albe

---

From: Bh W <[email-masked]>
Date: Mon, 22 Dec 2025 22:51:02 +0800

The issue is that the MERGE INTO match condition is not updated.
In the MATCHED path of MERGE INTO, when the target row satisfies the match
condition and the condition itself has not changed, the system should still
be able to handle concurrent updates to the same target row by relying on
EvalPlanQual (EPQ) to refetch the latest version of the tuple, and then
proceed with the intended update.
However, in the current implementation, even though the concurrent update
does not modify any columns relevant to the ON condition, the EPQ recheck
unexpectedly results in a match condition failure, causing the update path
that should remain MATCHED to be treated as NOT MATCHED.

In the scenario shown above, if no primary key exists, an extra row will be
inserted.

Further investigation shows that execution plans using Hash Join do not
exhibit this problem.

Could you please take a look at my explanation and let me know if anything
is inaccurate? I’d also appreciate it if you could check the test scenario
I provided. Thanks a lot!

---

From: Dean Rasheed <[email-masked]>
Date: Mon, 22 Dec 2025 22:37:18 +0000

On Mon, 22 Dec 2025 at 14:51, Bh W <[email-masked]> wrote:
> In the MATCHED path of MERGE INTO, when the target row satisfies the match condition and the condition itself has not changed, the system should still be able to handle concurrent updates to the same target row by relying on EvalPlanQual (EPQ) to refetch the latest version of the tuple, and then proceed with the intended update.
> However, in the current implementation, even though the concurrent update does not modify any columns relevant to the ON condition, the EPQ recheck unexpectedly results in a match condition failure, causing the update path that should remain MATCHED to be treated as NOT MATCHED.

I spent a little time looking at this, and managed to reduce the
reproducer test case down to this:

-- Setup
drop table if exists t1,t2;
create table t1(a int primary key, b int);
create table t2(a int, b int);

-- Session 1
begin;
update t1 set b = b+1;

-- Session 2
merge into t1 using (values(1,1),(2,2)) as t3(a,b) on (t1.a = t3.a)
when matched then
      update set b = t1.b + 1
when not matched then
      insert (a,b) values (1,1);

-- Session 1
commit;

This works fine in PG17, but fails with a PK violation in PG18.
Git-bisecting points to this commit:

cbc127917e04a978a788b8bc9d35a70244396d5b is the first bad commit
commit cbc127917e04a978a788b8bc9d35a70244396d5b
Author: Amit Langote <[email-masked]>
Date:   Fri Feb 7 17:15:09 2025 +0900

    Track unpruned relids to avoid processing pruned relations

Doing a little more debugging, it looks like the problem might be this
change in InitPlan():

-           /* ignore "parent" rowmarks; they are irrelevant at runtime */
-           if (rc->isParent)
+           /*
+            * Ignore "parent" rowmarks, because they are irrelevant at
+            * runtime.  Also ignore the rowmarks belonging to child tables
+            * that have been pruned in ExecDoInitialPruning().
+            */
+           if (rc->isParent ||
+               !bms_is_member(rc->rti, estate->es_unpruned_relids))
                continue;

which seems to cause it to incorrectly skip a rowmark, which I suspect
is what is causing EvalPlanQual() to return the wrong result.

Regards,
Dean

---

From: Amit Langote <[email-masked]>
Date: Wed, 24 Dec 2025 13:38:33 +0530

Hi,

On Tue, Dec 23, 2025 at 4:07 Dean Rasheed <[email-masked]> wrote:

> > In the MATCHED path of MERGE INTO, when the target row satisfies the
> match condition and the condition itself has not changed, the system should
> still be able to handle concurrent updates to the same target row by
> relying on EvalPlanQual (EPQ) to refetch the latest version of the tuple,
> and then proceed with the intended update.
> > However, in the current implementation, even though the concurrent
> update does not modify any columns relevant to the ON condition, the EPQ
> recheck unexpectedly results in a match condition failure, causing the
> update path that should remain MATCHED to be treated as NOT MATCHED.
>

Thanks for the detailed analysis and adding me to the thread, Dean.

I would think that a case that involves no partitioning at all would be
untouchable by this code, but it looks like the logic I added is
incorrectly affecting cases where pruning isn’t even relevant. I’ll need to
look more carefully at why such a rowmark would exist in the rowmarks list
if its relation isn’t in es_unpruned_relids. Maybe the set population is
incorrect at some point, or perhaps it matters that the set is a copy in
the EPQ estate.

I’m afk (on vacation) at the moment, so won’t be able to dig into this
until next week.

— Amit

---

From: Tender Wang <[email-masked]>
Date: Wed, 24 Dec 2025 20:07:00 +0800

> incorrect at some point, or perhaps it matters that the set is a copy in

I did some debugging, and I found that:
In  add_rte_to_flat_rtable(),  the RTE of value was not added into
glob->AllRelids, because below codes:
.....
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));
}
....

The VALUE rte was not satisfied above if, so it was not added into the
glob->allRelids.
Then in standard_planner(), we have:
....
result->unprunableRelids = bms_difference(glob->allRelids,
 glob->prunableRelids);
....
So the result->unprunableRelids contains only 1.

In InitPlan(), we have:
.....
/*
* Ignore "parent" rowmarks, because they are irrelevant at
* runtime.  Also ignore the rowmarks belonging to child tables
* that have been pruned in ExecDoInitialPruning().
*/
if (rc->isParent ||
   !bms_is_member(rc->rti, estate->es_unpruned_relids))
    continue;
.....
the estate->es_unpruned_relids equals with result->unprunableRelids
contains. So the rowMark was skipped incorrectly.

I did a quick fix as the attached patch.
Any thoughts?

-- 
Thanks,
Tender Wang

---

From: Dean Rasheed <[email-masked]>
Date: Wed, 24 Dec 2025 23:11:53 +0000

On Wed, 24 Dec 2025 at 12:07, Tender Wang <[email-masked]> wrote:
> In  add_rte_to_flat_rtable(),  the RTE of value was not added into glob->AllRelids, because below codes:
> .....
> the estate->es_unpruned_relids equals with result->unprunableRelids contains. So the rowMark was skipped incorrectly.
>

Yes. However, it's not sufficient to only add RTE_VALUES RTEs to what
gets included in PlannerGlobal.allRelids. Rowmarks can be attached to
other kinds of RTEs. An obvious example is an RTE_SUBQUERY RTE that is
an actual subquery that did not come from a view. So, for this
approach to work robustly, it really should include *all* RTEs in
PlannerGlobal.allRelids.

I took a slightly different approach, which was to change the test in
InitPlan() (and also in ExecInitLockRows() and ExecInitModifyTable())
to only ignore rowmarks for pruned relations that are plain
RTE_RELATION relations, since those are the only relations that are
ever actually pruned. So rowmarks attached to any other kind of RTE
are not ignored. I also added an isolation test case.

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.

Regards,
Dean
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
new file mode 100644
index 797d8b1..01dffde
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -880,21 +880,25 @@ InitPlan(QueryDesc *queryDesc, int eflag
 		foreach(l, plannedstmt->rowMarks)
 		{
 			PlanRowMark *rc = (PlanRowMark *) lfirst(l);
+			RangeTblEntry *rte = exec_rt_fetch(rc->rti, estate);
 			Oid			relid;
 			Relation	relation;
 			ExecRowMark *erm;
 
+			/* ignore "parent" rowmarks; they are irrelevant at runtime */
+			if (rc->isParent)
+				continue;
+
 			/*
-			 * Ignore "parent" rowmarks, because they are irrelevant at
-			 * runtime.  Also ignore the rowmarks belonging to child tables
-			 * that have been pruned in ExecDoInitialPruning().
+			 * Also ignore rowmarks belonging to child tables that have been
+			 * pruned in ExecDoInitialPruning().
 			 */
-			if (rc->isParent ||
+			if (rte->rtekind == RTE_RELATION &&
 				!bms_is_member(rc->rti, estate->es_unpruned_relids))
 				continue;
 
 			/* get relation's OID (will produce InvalidOid if subquery) */
-			relid = exec_rt_fetch(rc->rti, estate)->relid;
+			relid = rte->relid;
 
 			/* open relation, if we need to access it for this mark type */
 			switch (rc->markType)
diff --git a/src/backend/executor/nodeLockRows.c b/src/backend/executor/nodeLockRows.c
new file mode 100644
index a8afbf9..5d38bd4
--- a/src/backend/executor/nodeLockRows.c
+++ b/src/backend/executor/nodeLockRows.c
@@ -344,15 +344,19 @@ ExecInitLockRows(LockRows *node, EState
 	foreach(lc, node->rowMarks)
 	{
 		PlanRowMark *rc = lfirst_node(PlanRowMark, lc);
+		RangeTblEntry *rte = exec_rt_fetch(rc->rti, estate);
 		ExecRowMark *erm;
 		ExecAuxRowMark *aerm;
 
+		/* ignore "parent" rowmarks; they are irrelevant at runtime */
+		if (rc->isParent)
+			continue;
+
 		/*
-		 * Ignore "parent" rowmarks, because they are irrelevant at runtime.
-		 * Also ignore the rowmarks belonging to child tables that have been
+		 * Also ignore rowmarks belonging to child tables that have been
 		 * pruned in ExecDoInitialPruning().
 		 */
-		if (rc->isParent ||
+		if (rte->rtekind == RTE_RELATION &&
 			!bms_is_member(rc->rti, estate->es_unpruned_relids))
 			continue;
 
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
new file mode 100644
index 874b71e..9066a58
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -5092,15 +5092,19 @@ ExecInitModifyTable(ModifyTable *node, E
 	foreach(l, node->rowMarks)
 	{
 		PlanRowMark *rc = lfirst_node(PlanRowMark, l);
+		RangeTblEntry *rte = exec_rt_fetch(rc->rti, estate);
 		ExecRowMark *erm;
 		ExecAuxRowMark *aerm;
 
+		/* ignore "parent" rowmarks; they are irrelevant at runtime */
+		if (rc->isParent)
+			continue;
+
 		/*
-		 * Ignore "parent" rowmarks, because they are irrelevant at runtime.
-		 * Also ignore the rowmarks belonging to child tables that have been
+		 * Also ignore rowmarks belonging to child tables that have been
 		 * pruned in ExecDoInitialPruning().
 		 */
-		if (rc->isParent ||
+		if (rte->rtekind == RTE_RELATION &&
 			!bms_is_member(rc->rti, estate->es_unpruned_relids))
 			continue;
 
diff --git a/src/test/isolation/expected/eval-plan-qual.out b/src/test/isolation/expected/eval-plan-qual.out
new file mode 100644
index 05fffe0..a122047
--- a/src/test/isolation/expected/eval-plan-qual.out
+++ b/src/test/isolation/expected/eval-plan-qual.out
@@ -1473,3 +1473,23 @@ step s2pp4: DELETE FROM another_parttbl
 step c1: COMMIT;
 step s2pp4: <... completed>
 step c2: COMMIT;
+
+starting permutation: updateformergevalues mergevalues c1 c2 read
+step updateformergevalues: UPDATE accounts SET balance = balance + 100;
+step mergevalues: 
+	MERGE INTO accounts
+	USING (VALUES ('checking', 610), ('savings', 620)) v(accountid, balance)
+	ON v.accountid = accounts.accountid
+	WHEN MATCHED THEN UPDATE SET balance = v.balance
+	WHEN NOT MATCHED THEN INSERT VALUES ('unmatched', -1);
+ <waiting ...>
+step c1: COMMIT;
+step mergevalues: <... completed>
+step c2: COMMIT;
+step read: SELECT * FROM accounts ORDER BY accountid;
+accountid|balance|balance2
+---------+-------+--------
+checking |    610|    1220
+savings  |    620|    1240
+(2 rows)
+
diff --git a/src/test/isolation/specs/eval-plan-qual.spec b/src/test/isolation/specs/eval-plan-qual.spec
new file mode 100644
index 80e1e6b..fb57fb2
--- 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 updateformergevalues { 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,14 @@ step s2pp2 { PREPARE epd AS DELETE FROM
 step s2pp3 { EXECUTE epd(1); }
 step s2pp4 { DELETE FROM another_parttbl WHERE a = (SELECT 1); }
 
+step mergevalues {
+	MERGE INTO accounts
+	USING (VALUES ('checking', 610), ('savings', 620)) v(accountid, balance)
+	ON v.accountid = accounts.accountid
+	WHEN MATCHED THEN UPDATE SET balance = v.balance
+	WHEN NOT MATCHED THEN INSERT VALUES ('unmatched', -1);
+}
+
 session s3
 setup		{ BEGIN ISOLATION LEVEL READ COMMITTED; }
 step read	{ SELECT * FROM accounts ORDER BY accountid; }
@@ -425,3 +435,6 @@ permutation sys1 sysmerge2 c1 c2
 # Exercise run-time partition pruning code in an EPQ recheck
 permutation s1pp1 s2pp1 s2pp2 s2pp3 c1 c2
 permutation s1pp1 s2pp4 c1 c2
+
+# test EPQ recheck in MERGE from VALUES_RTE, cf bug #19355
+permutation updateformergevalues mergevalues c1 c2 read

---

From: Tender Wang <[email-masked]>
Date: Thu, 25 Dec 2025 12:48:08 +0800

> approach to work robustly, it really should include *all* RTEs in

Yes,  it is. I can reproduce the same with a subquery as below:

merge into t1 using (select a from (values(1,1),(2,2) as t4(a,b)) as t3(a)
on (t1.a = t3.a)
when matched then
      update set b = t1.b + 1
when not matched then
      insert (a,b) values (1,1);
or
merge into t1 using (select generate_series(1,2) as a) as t3  on (t1.a =
t3.a)
when matched then
      update set b = t1.b + 1
when not matched then
      insert (a,b) values (1,1);

All reported the same error. My approach obviously can not cover all cases.

> are not ignored. I also added an isolation test case.

I failed to apply your patch, some errors, like "error: git apply: bad
git-diff - expected /dev/null on line 2"
or "Patch format detection failed."
 I looked through the patch. It worked for me.

> Making it actually include all RTEs would solve that.
.....
 /*
* RT indexes of all relation RTEs in finalrtable (RTE_RELATION and
* RTE_SUBQUERY RTEs of views)
*/
Bitmapset  *allRelids;
......
Per the comment, I guess Amit didn't plan to include all RTEs in his design.

-- 
Thanks,
Tender Wang

---

From: Bernice Southey <[email-masked]>
Date: Wed, 31 Dec 2025 16:16:15 +0000

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:

> 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%40mail.gmail.com
[2] https://www.postgresql.org/message-id/ly6rss443iajiv5cfypzu6fgrmxdbwzdtuujkddr4eis7s2gcq%40gcdytigety6e
From 54526773321a525a0e806bf17353537a4bc23fef Mon Sep 17 00:00:00 2001
From: Bernice Southey <[email-masked]>
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 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

---

From: Bernice Southey <[email-masked]>
Date: Thu, 1 Jan 2026 21:58:04 +0000

> >> Making it actually include all RTEs would solve that.

I did some more digging (postgres source code is addictive).

Up until v56-0004-Defer-locking-of-runtime-prunable-relations-to-e.patch
[1], the existing unfiltered rtable is used to create
unprunableRelids.

+++ b/src/backend/optimizer/plan/planner.c
@@ -555,6 +555,8 @@ standard_planner(Query *parse, const char
*query_string, int cursorOptions,
  result->planTree = top_plan;
  result->partPruneInfos = glob->partPruneInfos;
  result->rtable = glob->finalrtable;
+ result->unprunableRelids = bms_difference(bms_add_range(NULL, 1,
list_length(result->rtable)),
+   glob->prunableRelids);

In v56 [2], the filtered allRelids was added. I think this is when the
bug was introduced, because the three places from Dean's patch are in
both versions.

I've looked much harder at the code (I'm still at
stumbling-around-in-the-dark-with-a-match level) and AFAICT, the two
approaches are very similar. I think equal effort is required to check
that PlannerGlobal.allRelids, unprunableRelids, and es_unpruned_relids
are correct, whichever approach is used. I can't find any missed cases
in either approach - with my matchlight.

Sorry for my ignorance: does a relId refer to a range table index and
a relation to a...for lack of a better word...table+?
I can really see these much appreciated extra features and
enhancements don't come cheap.

Thanks,
Bernice

[1] https://www.postgresql.org/message-id/CA%2BHiwqE7%2BiwMH4NYtFi28Pt9fT_gRW%2BGt-%3DCvOX%3DPkquo%3DAN8w%40mail.gmail.com
[2] https://www.postgresql.org/message-id/CA%2BHiwqHRRFQN6yZ54fBydOTM6ncqZBCmewZ6n519RjRdDsO44g%40mail.gmail.com

---

From: Amit Langote <[email-masked]>
Date: Wed, 7 Jan 2026 18:45:01 +0900

Hi Bernice,

> in either approach - with my matchlight.

Good catch on the history -- that's exactly when the bug was
introduced. I'd say Dean's approach is easier to verify since it's a
simple check at the point of use, rather than ensuring allRelids is
built correctly across all planner code paths.

> a relation to a...for lack of a better word...table+?

Depends on the context, but "relid" in the planner internal data
structures refer to RT index.  In the planner output data structures
(plan nodes, etc.), we typically use "rti" or "rtindex".

-- 
Thanks, Amit Langote

---

From: Amit Langote <[email-masked]>
Date: Wed, 7 Jan 2026 21:16:34 +0900

On Wed, Jan 7, 2026 at 6:45 PM Amit Langote <[email-masked]> wrote:
> introduced.

I remembered more on why I insisted on putting only relations with OID
(RTE_RELATION and RTE_SUBQUERY with relid set (views)) into
PlannerGlobal.allRelids and thus PlannedStmt.unprunableRelids. The
reason is that I did not want loops iterating over unprunableRelids to
have to skip relations that do not have an OID. For example,
AcquireExecutorLocks() currently iterates over the rtable and skips
non-RELATION RTEs; in a now-reverted commit (525392d57) I had changed
it to iterate over unprunableRelids instead, which avoided the need
for that skip logic entirely, because unprunableRelids by design only
contains OID-bearing relations:

-       foreach(lc2, plannedstmt->rtable)
+       while ((rtindex = bms_next_member(plannedstmt->unprunableRelids,
+                                         rtindex)) >= 0)
        {
-           RangeTblEntry *rte = (RangeTblEntry *) lfirst(lc2);
+           RangeTblEntry *rte = list_nth_node(RangeTblEntry,
+                                              plannedstmt->rtable,
+                                              rtindex - 1);

-           if (!(rte->rtekind == RTE_RELATION ||
-                 (rte->rtekind == RTE_SUBQUERY && OidIsValid(rte->relid))))
-               continue;
+           Assert(rte->rtekind == RTE_RELATION ||
+                  (rte->rtekind == RTE_SUBQUERY && OidIsValid(rte->relid)));

This is why adding all RTEs to allRelids, as Tender suggested, would
be problematic -- it would defeat the design intent that allows loops
over unprunableRelids to skip the filtering logic. While there are no
such loops today (I reverted the patch that added one), keeping this
design makes sense. Dean's fix addresses the buggy bms_is_member()
check directly, without changing allRelids - albeit the name's a bit
misleading as has been mentioned.

--
Thanks, Amit Langote