[Dmitry Koval <d.koval@postgrespro.ru>]

Hi!

I got an Assert when executing an "INSERT ... ON CONFLICT ... UPDATE 
..." query on partitioned table. Managed to reproduce this situation.

Reproduction order.
-------------------

1) Apply the patch 
[v1-0001-Triggering-Assert-on-query-with-ON-CONFLICT.patch] to "master" 
branch.

2) Build postgres with key "--enable-injection-points" + build an 
extension "injection_points" (src/test/modules/injection_points).

3) Run isolation test onconflict.spec:
make check -C src/test/modules/injection_points

Assert is triggered in postgres with stack, see attached file [stack.txt].

Clarification.
--------------
In the query "INSERT ... ON CONFLICT ... UPDATE ..." when executing 
INSERT, a conflict is triggered. But when trying to execute UPDATE, our 
tuple has already been moved to another partition and Assert is triggered.

Fixing.
-------
I suggest replace Assert with an error message, see 
[v1-0001-draft-of-fix.patch]. This is not a final fix as I am confused 
by the comment for Assert: "we don't support an UPDATE of INSERT ON 
CONFLICT for a partitioned table".
(Why "don't support an UPDATE"?
  It's not forbidden by syntax or errors ...)

-- 
With best regards,
Dmitry Koval

Postgres Professional: http://postgrespro.com

Attachments: download all (3 files, .tar.gz)

stack.txt (text/plain, 3.1K) download
#0  0x000072313029eb1c in pthread_kill () from /lib/x86_64-linux-gnu/libc.so.6
#1  0x000072313024526e in raise () from /lib/x86_64-linux-gnu/libc.so.6
#2  0x00007231302288ff in abort () from /lib/x86_64-linux-gnu/libc.so.6
#3  0x00005e3fccec9ff8 in ExceptionalCondition (conditionName=0x5e3fcd0b49c0 "!ItemPointerIndicatesMovedPartitions(&tmfd.ctid)", fileName=0x5e3fcd0b3fe0 "nodeModifyTable.c", lineNumber=2797) at assert.c:66
#4  0x00005e3fcca2264f in ExecOnConflictUpdate (context=0x7ffd08eee220, resultRelInfo=0x5e3fce06a310, conflictTid=0x7ffd08eee0cc, excludedSlot=0x5e3fce069640, canSetTag=true, returning=0x7ffd08eee0d8) at nodeModifyTable.c:2797
#5  0x00005e3fcca1fd6f in ExecInsert (context=0x7ffd08eee220, resultRelInfo=0x5e3fce06a310, slot=0x5e3fce069640, canSetTag=true, inserted_tuple=0x0, insert_destrel=0x0) at nodeModifyTable.c:1133
#6  0x00005e3fcca250e3 in ExecModifyTable (pstate=0x5e3fce068cf0) at nodeModifyTable.c:4324
#7  0x00005e3fcc9e0454 in ExecProcNodeFirst (node=0x5e3fce068cf0) at execProcnode.c:469
#8  0x00005e3fcc9d2f5b in ExecProcNode (node=0x5e3fce068cf0) at ../../../src/include/executor/executor.h:272
#9  0x00005e3fcc9d5de8 in ExecutePlan (queryDesc=0x5e3fce10edf0, operation=CMD_INSERT, sendTuples=false, numberTuples=0, direction=ForwardScanDirection, dest=0x5e3fce1149e8) at execMain.c:1675
#10 0x00005e3fcc9d3618 in standard_ExecutorRun (queryDesc=0x5e3fce10edf0, direction=ForwardScanDirection, count=0) at execMain.c:364
#11 0x00005e3fcc9d3479 in ExecutorRun (queryDesc=0x5e3fce10edf0, direction=ForwardScanDirection, count=0) at execMain.c:301
#12 0x00005e3fcccc0ab0 in ProcessQuery (plan=0x5e3fce042778, sourceText=0x5e3fce040ef0 "INSERT INTO t_int VALUES (1, 11, 111) ON CONFLICT (i) DO UPDATE SET x = excluded.x;", params=0x0, queryEnv=0x0, dest=0x5e3fce1149e8, qc=0x7ffd08eee670) at pquery.c:160
#13 0x00005e3fcccc25e0 in PortalRunMulti (portal=0x5e3fce0bf8a0, isTopLevel=true, setHoldSnapshot=false, dest=0x5e3fce1149e8, altdest=0x5e3fce1149e8, qc=0x7ffd08eee670) at pquery.c:1271
#14 0x00005e3fcccc1aec in PortalRun (portal=0x5e3fce0bf8a0, count=9223372036854775807, isTopLevel=true, dest=0x5e3fce1149e8, altdest=0x5e3fce1149e8, qc=0x7ffd08eee670) at pquery.c:787
#15 0x00005e3fcccba5bf in exec_simple_query (query_string=0x5e3fce040ef0 "INSERT INTO t_int VALUES (1, 11, 111) ON CONFLICT (i) DO UPDATE SET x = excluded.x;") at postgres.c:1271
#16 0x00005e3fcccbf954 in PostgresMain (dbname=0x5e3fce078fa0 "isolation_regression", username=0x5e3fce078f88 "kovdb") at postgres.c:4691
#17 0x00005e3fcccb5fdf in BackendMain (startup_data=0x7ffd08eee8f0 "", startup_data_len=4) at backend_startup.c:107
#18 0x00005e3fccbc24d7 in postmaster_child_launch (child_type=B_BACKEND, child_slot=2, startup_data=0x7ffd08eee8f0 "", startup_data_len=4, client_sock=0x7ffd08eee950) at launch_backend.c:274
#19 0x00005e3fccbc8ec3 in BackendStartup (client_sock=0x7ffd08eee950) at postmaster.c:3519
#20 0x00005e3fccbc646c in ServerLoop () at postmaster.c:1688
#21 0x00005e3fccbc5d64 in PostmasterMain (argc=8, argv=0x5e3fcdffe970) at postmaster.c:1386
#22 0x00005e3fcca67011 in main (argc=8, argv=0x5e3fcdffe970) at main.c:230

v1-0001-Triggering-Assert-on-query-with-ON-CONFLICT.patch (text/plain, 3.2K) download
From 02b89799a2f139a3cab41b1df761103cf8627098 Mon Sep 17 00:00:00 2001
From: Koval Dmitry <d.koval@postgrespro.ru>
Date: Wed, 19 Feb 2025 13:09:33 +0300
Subject: [PATCH v1] Triggering Assert on query with ON CONFLICT

---
 src/backend/executor/nodeModifyTable.c        |  2 ++
 src/test/modules/injection_points/Makefile    | 11 +++----
 .../injection_points/specs/onconflict.spec    | 30 +++++++++++++++++++
 3 files changed, 38 insertions(+), 5 deletions(-)
 create mode 100644 src/test/modules/injection_points/specs/onconflict.spec

diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index b0fe50075ad..01946fdb0d8 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -67,6 +67,7 @@
 #include "storage/lmgr.h"
 #include "utils/builtins.h"
 #include "utils/datum.h"
+#include "utils/injection_point.h"
 #include "utils/rel.h"
 #include "utils/snapmgr.h"
 
@@ -1128,6 +1129,7 @@ ExecInsert(ModifyTableContext *context,
 					 */
 					TupleTableSlot *returning = NULL;
 
+					INJECTION_POINT("before-on-conflict-update");
 					if (ExecOnConflictUpdate(context, resultRelInfo,
 											 &conflictTid, slot, canSetTag,
 											 &returning))
diff --git a/src/test/modules/injection_points/Makefile b/src/test/modules/injection_points/Makefile
index e680991f8d4..b1915800760 100644
--- a/src/test/modules/injection_points/Makefile
+++ b/src/test/modules/injection_points/Makefile
@@ -11,15 +11,16 @@ EXTENSION = injection_points
 DATA = injection_points--1.0.sql
 PGFILEDESC = "injection_points - facility for injection points"
 
-REGRESS = injection_points hashagg reindex_conc
-REGRESS_OPTS = --dlpath=$(top_builddir)/src/test/regress
+#REGRESS = injection_points hashagg reindex_conc
+#REGRESS_OPTS = --dlpath=$(top_builddir)/src/test/regress
 
-ISOLATION = basic inplace syscache-update-pruned
+#ISOLATION = basic inplace syscache-update-pruned
+ISOLATION = onconflict
 
-TAP_TESTS = 1
+#TAP_TESTS = 1
 
 # The injection points are cluster-wide, so disable installcheck
-NO_INSTALLCHECK = 1
+#NO_INSTALLCHECK = 1
 
 export enable_injection_points
 
diff --git a/src/test/modules/injection_points/specs/onconflict.spec b/src/test/modules/injection_points/specs/onconflict.spec
new file mode 100644
index 00000000000..cdfe7942029
--- /dev/null
+++ b/src/test/modules/injection_points/specs/onconflict.spec
@@ -0,0 +1,30 @@
+setup
+{
+  CREATE EXTENSION injection_points;
+
+  CREATE TABLE t_int (i int PRIMARY KEY, v int, x int) PARTITION BY RANGE (i);
+  CREATE TABLE t_int_1 PARTITION OF t_int FOR VALUES FROM (1) TO (100);
+  CREATE TABLE t_int_2 PARTITION OF t_int FOR VALUES FROM (100) TO (200);
+
+  INSERT INTO t_int VALUES (1, 10, 100);
+}
+
+teardown
+{
+  DROP TABLE t_int;
+  DROP EXTENSION injection_points;
+}
+
+session s1
+step s1		{ BEGIN; }
+step s1ipa	{ SELECT injection_points_attach('before-on-conflict-update', 'wait'); }
+step s1i	{ INSERT INTO t_int VALUES (1, 11, 111) ON CONFLICT (i) DO UPDATE SET x = excluded.x; }
+step s1c	{ COMMIT; }
+
+session s2
+step s2		{ BEGIN; }
+step s2ipw	{ SELECT injection_points_wakeup('before-on-conflict-update'); }
+step s2u	{ UPDATE t_int SET i = i + 150 WHERE i = 1; }
+step s2c	{ COMMIT; }
+
+permutation s1 s1ipa s1i(s2c) s2 s2u s2ipw s2c
-- 
2.34.1



v1-0001-draft-of-fix.patch (text/plain, 759B) download
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index b0fe50075ad..4b0ae8bd32c 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -2792,7 +2792,10 @@ ExecOnConflictUpdate(ModifyTableContext *context,
 			 * be lock is moved to another partition due to concurrent update
 			 * of the partition key.
 			 */
-			Assert(!ItemPointerIndicatesMovedPartitions(&tmfd.ctid));
+			if (ItemPointerIndicatesMovedPartitions(&tmfd.ctid))
+				ereport(ERROR,
+						(errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
+						 errmsg("tuple to be updated was already moved to another partition due to concurrent update")));
 
 			/*
 			 * Tell caller to try again from the very start.