pg.ddx.io  pgsql-bugs@postgresql.org mailing list archive  
help / color / mirror / Atom feed
BUG #18986: SIGSEGV in nodeModifyTable.c during Parallel Execution
12+ messages / 5 participants
[nested] [flat]

* BUG #18986: SIGSEGV in nodeModifyTable.c during Parallel Execution
@ 2025-07-15 16:11  PG Bug reporting form <noreply@postgresql.org>
  0 siblings, 1 reply; 12+ messages in thread

From: PG Bug reporting form @ 2025-07-15 16:11 UTC (permalink / raw)
  To: pgsql-bugs@lists.postgresql.org; +Cc: y@lll.gd

The following bug has been logged on the website:

Bug reference:      18986
Logged by:          Yaroslav Syrytsia
Email address:      y@lll.gd
PostgreSQL version: 17.5
Operating system:   Ubuntu 22.04.5
Description:        

Hello,

During testing a flow in an app, I encountered a segfault that only happens
during parallel execution:
https://gist.github.com/joy4eg/708458e204f52129a8e54a13534586b7

Looking into the coredump, it looks like a NULL pointer dereference is
happening at
https://github.com/postgres/postgres/blob/5e2f3df49d4298c6097789364a5a53be172f6e85/src/backend/execu...

After further investigation, that part of the code looks suspicious as well:
static void
ExecInitInsertProjection(ModifyTableState *mtstate,
                                                 ResultRelInfo
*resultRelInfo)
{
    ...
        /* Build ProjectionInfo if needed (it probably isn't). */
        if (need_projection)
        {
                TupleDesc       relDesc =
RelationGetDescr(resultRelInfo->ri_RelationDesc);

                /* need an expression context to do the projection */
                if (mtstate->ps.ps_ExprContext == NULL)
                        ExecAssignExprContext(estate, &mtstate->ps);

                resultRelInfo->ri_projectNew =
                        ExecBuildProjectionInfo(insertTargetList,
mtstate->ps.ps_ExprContext,
resultRelInfo->ri_newTupleSlot,
&mtstate->ps,
relDesc);
        }

        resultRelInfo->ri_projectNewInfoValid = true; // <--- it's set to
true even if need_projection may be NULL.
}

(gdb) frame 0
#0  ExecGetUpdateNewTuple (oldSlot=0x55defb35ae20, planSlot=0x55defb41d9d8,
relinfo=0x55defb312108) at
executor/./build/../src/backend/executor/nodeModifyTable.c:752
752             econtext = newProj->pi_exprContext;
(gdb) p relinfo
$1 = (ResultRelInfo *) 0x55defb312108
(gdb) p *relinfo
$2 = {type = T_ResultRelInfo, ri_RangeTableIndex = 15, ri_RelationDesc =
0x7f0c0eb80500, ri_NumIndices = 21, ri_IndexRelationDescs = 0x55defb358248,
ri_IndexRelationInfo = 0x55defb358350, ri_RowIdAttNo = 1,
  ri_extraUpdatedCols = 0x55defb357300, ri_projectNew = 0x0, ri_newTupleSlot
= 0x55defb323cf0, ri_oldTupleSlot = 0x55defb323ae8, ri_projectNewInfoValid =
true, ri_needLockTagTuple = false, ri_TrigDesc = 0x55defb312310,
  ri_TrigFunctions = 0x55defb31a318, ri_TrigWhenExprs = 0x55defb2d8cf8,
ri_TrigInstrument = 0x0, ri_ReturningSlot = 0x0, ri_TrigOldSlot =
0x55defb35ae20, ri_TrigNewSlot = 0x0, ri_FdwRoutine = 0x0, ri_FdwState =
0x0,
  ri_usesFdwDirectModify = false, ri_NumSlots = 0, ri_NumSlotsInitialized =
0, ri_BatchSize = 0, ri_Slots = 0x0, ri_PlanSlots = 0x0, ri_WithCheckOptions
= 0x0, ri_WithCheckOptionExprs = 0x0, ri_ConstraintExprs = 0x0,
  ri_GeneratedExprsI = 0x0, ri_GeneratedExprsU = 0x55defb358140,
ri_NumGeneratedNeededI = 0, ri_NumGeneratedNeededU = 2, ri_returningList =
0x55defb39c630, ri_projectReturning = 0x55defb323590,
ri_onConflictArbiterIndexes = 0x0,
  ri_onConflict = 0x0, ri_MergeActions = {0x55defb324518, 0x0,
0x55defb328e50}, ri_MergeJoinCondition = 0x0, ri_PartitionCheckExpr = 0x0,
ri_ChildToRootMap = 0x0, ri_ChildToRootMapValid = false, ri_RootToChildMap =
0x0,
  ri_RootToChildMapValid = false, ri_RootResultRelInfo = 0x0,
ri_PartitionTupleSlot = 0x0, ri_CopyMultiInsertBuffer = 0x0,
ri_ancestorResultRels = 0x0}

Where:
- ri_projectNewInfoValid = true
- ri_projectNew = 0x0

Version tested:
- 17.5 (ubuntu, apt, with and without pgroonga)
- postgres:18beta1-bookworm (docker, without pgroonga)
- postgres:17.5-bookworm (docker, without pgroonga)

Yaroslav.



^ permalink  raw  reply  [nested|flat] 12+ messages in thread

* Re: BUG #18986: SIGSEGV in nodeModifyTable.c during Parallel Execution
@ 2025-07-15 18:13  Tom Lane <tgl@sss.pgh.pa.us>
  parent: PG Bug reporting form <noreply@postgresql.org>
  0 siblings, 2 replies; 12+ messages in thread

From: Tom Lane @ 2025-07-15 18:13 UTC (permalink / raw)
  To: y@lll.gd; +Cc: pgsql-bugs@lists.postgresql.org

PG Bug reporting form <noreply@postgresql.org> writes:
> During testing a flow in an app, I encountered a segfault that only happens
> during parallel execution:
> https://gist.github.com/joy4eg/708458e204f52129a8e54a13534586b7

This report isn't terribly helpful, since you have not explained
how to trigger the crash.  If we can't duplicate it, it's hard
to decide what is the appropriate fix.

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 12+ messages in thread

* Re: BUG #18986: SIGSEGV in nodeModifyTable.c during Parallel Execution
@ 2025-07-16 00:10  Michael Paquier <michael@paquier.xyz>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  1 sibling, 1 reply; 12+ messages in thread

From: Michael Paquier @ 2025-07-16 00:10 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: y@lll.gd; pgsql-bugs@lists.postgresql.org

On Tue, Jul 15, 2025 at 02:13:53PM -0400, Tom Lane wrote:
> PG Bug reporting form <noreply@postgresql.org> writes:
> > During testing a flow in an app, I encountered a segfault that only happens
> > during parallel execution:
> > https://gist.github.com/joy4eg/708458e204f52129a8e54a13534586b7
> 
> This report isn't terribly helpful, since you have not explained
> how to trigger the crash.  If we can't duplicate it, it's hard
> to decide what is the appropriate fix.
		
For the sake of visibility if the information on github gets lost, the
change can be summarized by this query pattern, but we don't have any
information about the end of the query:

WITH replaced AS (
   DELETE FROM replaceable_events_before_update
    WHERE replaced_by_id = ANY($1)
    RETURNING attr_list_1, [...] ),
  source_data (attr_list_2) AS (
    SELECT * from replaced UNION ALL
      VALUES (attr_list_3 ...

That's most likely generated a MERGE by an ORM to have these many
parameters and columns listed.  Without the full pattern or an idea of
the schema as this includes triggers, what we can do is limited, as
Tom says.

And the relevant portion of the backtrace:
#7  ExecGetUpdateNewTuple (oldSlot=0x55b0842d0288,
#planSlot=0x55b084249d68, relinfo=0x55b0836a9b88) at
#executor/./build/../src/backend/executor/nodeModifyTable.c:752
#8  ExecBRUpdateTriggers (estate=0x55b08369a708,
#epqstate=0x55b0836a9a68, relinfo=0x55b0836a9b88,
#tupleid=0x7fffc373628a, fdw_trigtuple=0x0, newslot=0x55b083720eb0,
#tmresult=0x7fffc37361d0, tmfd=0x7fffc37362e0)    at
#commands/./build/../src/backend/commands/trigger.c:2979
#9  0x000055b08158628e in ExecUpdatePrologue
#(context=context@entry=0x7fffc37362c0,
#resultRelInfo=resultRelInfo@entry=0x55b0836a9b88,
#tupleid=tupleid@entry=0x7fffc373628a, oldtuple=oldtuple@entry=0x0,
#slot=slot@entry=0x55b083720eb0, result=result@entry=0x7fffc37361d0)
#at executor/./build/../src/backend/executor/nodeModifyTable.c:1949
#10 0x000055b081587e75 in ExecMergeMatched (context=<optimized out>,
#resultRelInfo=<optimized out>, tupleid=0x7fffc373628a, oldtuple=0x0,
#canSetTag=false, matched=0x7fffc3736290)    at
#executor/./build/../src/backend/executor/nodeModifyTable.c:3023 

If you could extract the query string from the backtrace, that would
be one piece of information.  With triggers and MERGE in mind, it may
be possible to reproduce something.
--
Michael

Attachments:

  [application/pgp-signature] signature.asc (832B, ../../aHbt_HkrM3TTH61j@paquier.xyz/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 12+ messages in thread

* Re: BUG #18986: SIGSEGV in nodeModifyTable.c during Parallel Execution
@ 2025-07-16 09:33  Yaroslav Syrytsia <me@ys.lc>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  1 sibling, 0 replies; 12+ messages in thread

From: Yaroslav Syrytsia @ 2025-07-16 09:33 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-bugs@lists.postgresql.org

Hello,

I made a small reproducer here - https://github.com/joy4eg/pg-merge-crash

It creates a few tables and then executes MERGE statements in parallel. 

Yaroslav.

> On 2025-07-15 20:13 CEST Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 
>  
> PG Bug reporting form <noreply@postgresql.org> writes:
> > During testing a flow in an app, I encountered a segfault that only happens
> > during parallel execution:
> > https://gist.github.com/joy4eg/708458e204f52129a8e54a13534586b7
> 
> This report isn't terribly helpful, since you have not explained
> how to trigger the crash.  If we can't duplicate it, it's hard
> to decide what is the appropriate fix.
> 
> 			regards, tom lane





^ permalink  raw  reply  [nested|flat] 12+ messages in thread

* Re: BUG #18986: SIGSEGV in nodeModifyTable.c during Parallel Execution
@ 2025-07-16 09:55  Dean Rasheed <dean.a.rasheed@gmail.com>
  parent: Michael Paquier <michael@paquier.xyz>
  0 siblings, 1 reply; 12+ messages in thread

From: Dean Rasheed @ 2025-07-16 09:55 UTC (permalink / raw)
  To: Michael Paquier <michael@paquier.xyz>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; y@lll.gd; pgsql-bugs@lists.postgresql.org

On Wed, 16 Jul 2025 at 01:10, Michael Paquier <michael@paquier.xyz> wrote:
>
> And the relevant portion of the backtrace:
> #7  ExecGetUpdateNewTuple (oldSlot=0x55b0842d0288,
> #planSlot=0x55b084249d68, relinfo=0x55b0836a9b88) at
> #executor/./build/../src/backend/executor/nodeModifyTable.c:752
> #8  ExecBRUpdateTriggers (estate=0x55b08369a708,
> #epqstate=0x55b0836a9a68, relinfo=0x55b0836a9b88,
> #tupleid=0x7fffc373628a, fdw_trigtuple=0x0, newslot=0x55b083720eb0,
> #tmresult=0x7fffc37361d0, tmfd=0x7fffc37362e0)    at
> #commands/./build/../src/backend/commands/trigger.c:2979
> #9  0x000055b08158628e in ExecUpdatePrologue
> #(context=context@entry=0x7fffc37362c0,
> #resultRelInfo=resultRelInfo@entry=0x55b0836a9b88,
> #tupleid=tupleid@entry=0x7fffc373628a, oldtuple=oldtuple@entry=0x0,
> #slot=slot@entry=0x55b083720eb0, result=result@entry=0x7fffc37361d0)
> #at executor/./build/../src/backend/executor/nodeModifyTable.c:1949
> #10 0x000055b081587e75 in ExecMergeMatched (context=<optimized out>,
> #resultRelInfo=<optimized out>, tupleid=0x7fffc373628a, oldtuple=0x0,
> #canSetTag=false, matched=0x7fffc3736290)    at
> #executor/./build/../src/backend/executor/nodeModifyTable.c:3023

Hmm, I can see the problem. This particular stack trace should never happen:

#7  ExecGetUpdateNewTuple
#8  ExecBRUpdateTriggers
#9  ExecUpdatePrologue
#10 ExecMergeMatched

ExecBRUpdateTriggers() only calls ExecGetUpdateNewTuple() if
GetTupleForTrigger() sets epqslot_candidate to a non-null value, and
GetTupleForTrigger() should only set epqslot_candidate (*epqslot) to
non-null if we're not doing a MERGE (see commit 9321c79).

The problem, however, is that GetTupleForTrigger() tests for MERGE by doing

    if (estate->es_plannedstmt->commandType != CMD_MERGE)

which doesn't work if the MERGE is inside a CTE. So we need a
different way for ExecBRUpdateTriggers() / GetTupleForTrigger() to
know that it's being called from within a MERGE.

Regards,
Dean





^ permalink  raw  reply  [nested|flat] 12+ messages in thread

* Re: BUG #18986: SIGSEGV in nodeModifyTable.c during Parallel Execution
@ 2025-07-16 10:42  Dean Rasheed <dean.a.rasheed@gmail.com>
  parent: Dean Rasheed <dean.a.rasheed@gmail.com>
  0 siblings, 1 reply; 12+ messages in thread

From: Dean Rasheed @ 2025-07-16 10:42 UTC (permalink / raw)
  To: Michael Paquier <michael@paquier.xyz>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; y@lll.gd; pgsql-bugs@lists.postgresql.org

On Wed, 16 Jul 2025 at 10:55, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
>
> The problem, however, is that GetTupleForTrigger() tests for MERGE by doing
>
>     if (estate->es_plannedstmt->commandType != CMD_MERGE)
>
> which doesn't work if the MERGE is inside a CTE. So we need a
> different way for ExecBRUpdateTriggers() / GetTupleForTrigger() to
> know that it's being called from within a MERGE.

Attached is a reproducer using the merge-match-recheck isolation test.

I believe that the bug only goes back to v17, because MERGE could not
appear inside a CTE prior to that.

I think that best thing to do is pass the commandType to
ExecBRUpdateTriggers(), except that in v17 we shouldn't change the
signature of ExecBRUpdateTriggers(), so a wrapper function will be
needed, similar to what 9321c79 did.

Question: Is it OK to change the signature of ExecBRUpdateTriggers() in v18?

Regards,
Dean

Attachments:

  [text/x-patch] merge-bug.patch (1.2K, ../../CAEZATCUahLe8Lvi7XHV6Eo=Tg46iJ2Rmfw0k6AH5GQbfOjENHQ@mail.gmail.com/2-merge-bug.patch)
  download | inline diff:
diff --git a/src/test/isolation/specs/merge-match-recheck.spec b/src/test/isolation/specs/merge-match-recheck.spec
new file mode 100644
index 26266b8..15226e4
--- a/src/test/isolation/specs/merge-match-recheck.spec
+++ b/src/test/isolation/specs/merge-match-recheck.spec
@@ -99,15 +99,19 @@ step "merge_bal_pa"
 }
 step "merge_bal_tg"
 {
-  MERGE INTO target_tg t
-  USING (SELECT 1 as key) s
-  ON s.key = t.key
-  WHEN MATCHED AND balance < 100 THEN
-	UPDATE SET balance = balance * 2, val = t.val || ' when1'
-  WHEN MATCHED AND balance < 200 THEN
-	UPDATE SET balance = balance * 4, val = t.val || ' when2'
-  WHEN MATCHED AND balance < 300 THEN
-	UPDATE SET balance = balance * 8, val = t.val || ' when3';
+  WITH t AS (
+    MERGE INTO target_tg t
+    USING (SELECT 1 as key) s
+    ON s.key = t.key
+    WHEN MATCHED AND balance < 100 THEN
+      UPDATE SET balance = balance * 2, val = t.val || ' when1'
+    WHEN MATCHED AND balance < 200 THEN
+      UPDATE SET balance = balance * 4, val = t.val || ' when2'
+    WHEN MATCHED AND balance < 300 THEN
+      UPDATE SET balance = balance * 8, val = t.val || ' when3'
+    RETURNING t.*
+  )
+  SELECT * FROM t;
 }
 
 step "merge_delete"


^ permalink  raw  reply  [nested|flat] 12+ messages in thread

* Re: BUG #18986: SIGSEGV in nodeModifyTable.c during Parallel Execution
@ 2025-07-16 10:48  Michael Paquier <michael@paquier.xyz>
  parent: Dean Rasheed <dean.a.rasheed@gmail.com>
  0 siblings, 1 reply; 12+ messages in thread

From: Michael Paquier @ 2025-07-16 10:48 UTC (permalink / raw)
  To: Dean Rasheed <dean.a.rasheed@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; y@lll.gd; pgsql-bugs@lists.postgresql.org

On Wed, Jul 16, 2025 at 11:42:33AM +0100, Dean Rasheed wrote:
> Attached is a reproducer using the merge-match-recheck isolation test.
> 
> I believe that the bug only goes back to v17, because MERGE could not
> appear inside a CTE prior to that.

That was fast, nice!

> I think that best thing to do is pass the commandType to
> ExecBRUpdateTriggers(), except that in v17 we shouldn't change the
> signature of ExecBRUpdateTriggers(), so a wrapper function will be
> needed, similar to what 9321c79 did.

Yes, changing ExecBRUpdateTriggers() would not be a good idea on a
stable branch..  I can see that at least timescaledb does a direct
call to it.  A minor upgrade breakage would be bad for them.

> Question: Is it OK to change the signature of ExecBRUpdateTriggers()
> in v18?

We are still in beta, so that's not a problem for v18 and HEAD.
--
Michael

Attachments:

  [application/pgp-signature] signature.asc (832B, ../../aHeDl-0OuIA-MJtt@paquier.xyz/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 12+ messages in thread

* Re: BUG #18986: SIGSEGV in nodeModifyTable.c during Parallel Execution
@ 2025-07-16 13:05  Dean Rasheed <dean.a.rasheed@gmail.com>
  parent: Michael Paquier <michael@paquier.xyz>
  0 siblings, 1 reply; 12+ messages in thread

From: Dean Rasheed @ 2025-07-16 13:05 UTC (permalink / raw)
  To: Michael Paquier <michael@paquier.xyz>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; y@lll.gd; pgsql-bugs@lists.postgresql.org

On Wed, 16 Jul 2025 at 11:49, Michael Paquier <michael@paquier.xyz> wrote:
>
> > Question: Is it OK to change the signature of ExecBRUpdateTriggers()
> > in v18?
>
> We are still in beta, so that's not a problem for v18 and HEAD.

Cool. I thought so, but I wanted to check.

Attached is a patch for HEAD/v18, and a slightly different one for
v17, preserving the trigger ABI in the standard way.

I decided to do this by adding an extra "is_merge_update" boolean
parameter, rather than passing the commandType because that looked
slightly neater. It was also necessary to update
ExecBRDeleteTriggers(), since otherwise a concurrent MERGE DELETE
could do the wrong thing (execute the wrong action, rather than
seg-faulting). That was picked up by an existing isolation test case
added by 9321c79, so no need for more tests.

I haven't tested this against the OP's reproducer.

Regards,
Dean

Attachments:

  [text/x-patch] fix-merge-bug.patch (10.4K, ../../CAEZATCWZahtK-OT5fa2=8b=+BUuDtsspkvur4x8WkjgHbxJxkw@mail.gmail.com/2-fix-merge-bug.patch)
  download | inline diff:
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
new file mode 100644
index 67f8e70..7dc121f
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -80,6 +80,7 @@ static bool GetTupleForTrigger(EState *e
 							   ItemPointer tid,
 							   LockTupleMode lockmode,
 							   TupleTableSlot *oldslot,
+							   bool do_epq_recheck,
 							   TupleTableSlot **epqslot,
 							   TM_Result *tmresultp,
 							   TM_FailureData *tmfdp);
@@ -2693,7 +2694,8 @@ ExecBRDeleteTriggers(EState *estate, EPQ
 					 HeapTuple fdw_trigtuple,
 					 TupleTableSlot **epqslot,
 					 TM_Result *tmresult,
-					 TM_FailureData *tmfd)
+					 TM_FailureData *tmfd,
+					 bool is_merge_delete)
 {
 	TupleTableSlot *slot = ExecGetTriggerOldSlot(estate, relinfo);
 	TriggerDesc *trigdesc = relinfo->ri_TrigDesc;
@@ -2708,9 +2710,17 @@ ExecBRDeleteTriggers(EState *estate, EPQ
 	{
 		TupleTableSlot *epqslot_candidate = NULL;
 
+		/*
+		 * Get a copy of the on-disk tuple we are planning to delete.  In
+		 * general, if the tuple has been concurrently updated, we should
+		 * recheck it using EPQ.  However, if this is a MERGE DELETE action,
+		 * we skip this EPQ recheck and leave it to the caller (it must do
+		 * additional rechecking, and might end up executing a different
+		 * action entirely).
+		 */
 		if (!GetTupleForTrigger(estate, epqstate, relinfo, tupleid,
-								LockTupleExclusive, slot, &epqslot_candidate,
-								tmresult, tmfd))
+								LockTupleExclusive, slot, !is_merge_delete,
+								&epqslot_candidate, tmresult, tmfd))
 			return false;
 
 		/*
@@ -2800,6 +2810,7 @@ ExecARDeleteTriggers(EState *estate,
 							   tupleid,
 							   LockTupleExclusive,
 							   slot,
+							   false,
 							   NULL,
 							   NULL,
 							   NULL);
@@ -2944,7 +2955,8 @@ ExecBRUpdateTriggers(EState *estate, EPQ
 					 HeapTuple fdw_trigtuple,
 					 TupleTableSlot *newslot,
 					 TM_Result *tmresult,
-					 TM_FailureData *tmfd)
+					 TM_FailureData *tmfd,
+					 bool is_merge_update)
 {
 	TriggerDesc *trigdesc = relinfo->ri_TrigDesc;
 	TupleTableSlot *oldslot = ExecGetTriggerOldSlot(estate, relinfo);
@@ -2965,10 +2977,17 @@ ExecBRUpdateTriggers(EState *estate, EPQ
 	{
 		TupleTableSlot *epqslot_candidate = NULL;
 
-		/* get a copy of the on-disk tuple we are planning to update */
+		/*
+		 * Get a copy of the on-disk tuple we are planning to update.  In
+		 * general, if the tuple has been concurrently updated, we should
+		 * recheck it using EPQ.  However, if this is a MERGE UPDATE action,
+		 * we skip this EPQ recheck and leave it to the caller (it must do
+		 * additional rechecking, and might end up executing a different
+		 * action entirely).
+		 */
 		if (!GetTupleForTrigger(estate, epqstate, relinfo, tupleid,
-								lockmode, oldslot, &epqslot_candidate,
-								tmresult, tmfd))
+								lockmode, oldslot, !is_merge_update,
+								&epqslot_candidate, tmresult, tmfd))
 			return false;		/* cancel the update action */
 
 		/*
@@ -3142,6 +3161,7 @@ ExecARUpdateTriggers(EState *estate, Res
 							   tupleid,
 							   LockTupleExclusive,
 							   oldslot,
+							   false,
 							   NULL,
 							   NULL,
 							   NULL);
@@ -3298,6 +3318,7 @@ GetTupleForTrigger(EState *estate,
 				   ItemPointer tid,
 				   LockTupleMode lockmode,
 				   TupleTableSlot *oldslot,
+				   bool do_epq_recheck,
 				   TupleTableSlot **epqslot,
 				   TM_Result *tmresultp,
 				   TM_FailureData *tmfdp)
@@ -3357,29 +3378,30 @@ GetTupleForTrigger(EState *estate,
 				if (tmfd.traversed)
 				{
 					/*
-					 * Recheck the tuple using EPQ. For MERGE, we leave this
-					 * to the caller (it must do additional rechecking, and
-					 * might end up executing a different action entirely).
+					 * Recheck the tuple using EPQ, if requested.  Otherwise,
+					 * just return that it was concurrently updated.
 					 */
-					if (estate->es_plannedstmt->commandType == CMD_MERGE)
+					if (do_epq_recheck)
 					{
-						if (tmresultp)
-							*tmresultp = TM_Updated;
-						return false;
-					}
-
-					*epqslot = EvalPlanQual(epqstate,
-											relation,
-											relinfo->ri_RangeTableIndex,
-											oldslot);
+						*epqslot = EvalPlanQual(epqstate,
+												relation,
+												relinfo->ri_RangeTableIndex,
+												oldslot);
 
-					/*
-					 * If PlanQual failed for updated tuple - we must not
-					 * process this tuple!
-					 */
-					if (TupIsNull(*epqslot))
+						/*
+						 * If PlanQual failed for updated tuple - we must not
+						 * process this tuple!
+						 */
+						if (TupIsNull(*epqslot))
+						{
+							*epqslot = NULL;
+							return false;
+						}
+					}
+					else
 					{
-						*epqslot = NULL;
+						if (tmresultp)
+							*tmresultp = TM_Updated;
 						return false;
 					}
 				}
diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c
new file mode 100644
index 53ddd25..f262e7a
--- a/src/backend/executor/execReplication.c
+++ b/src/backend/executor/execReplication.c
@@ -670,7 +670,7 @@ ExecSimpleRelationUpdate(ResultRelInfo *
 		resultRelInfo->ri_TrigDesc->trig_update_before_row)
 	{
 		if (!ExecBRUpdateTriggers(estate, epqstate, resultRelInfo,
-								  tid, NULL, slot, NULL, NULL))
+								  tid, NULL, slot, NULL, NULL, false))
 			skip_tuple = true;	/* "do nothing" */
 	}
 
@@ -746,7 +746,7 @@ ExecSimpleRelationDelete(ResultRelInfo *
 		resultRelInfo->ri_TrigDesc->trig_delete_before_row)
 	{
 		skip_tuple = !ExecBRDeleteTriggers(estate, epqstate, resultRelInfo,
-										   tid, NULL, NULL, NULL, NULL);
+										   tid, NULL, NULL, NULL, NULL, false);
 	}
 
 	if (!skip_tuple)
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
new file mode 100644
index 54da8e7..7c6c2c1
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -1474,7 +1474,8 @@ ExecDeletePrologue(ModifyTableContext *c
 
 		return ExecBRDeleteTriggers(context->estate, context->epqstate,
 									resultRelInfo, tupleid, oldtuple,
-									epqreturnslot, result, &context->tmfd);
+									epqreturnslot, result, &context->tmfd,
+									context->mtstate->operation == CMD_MERGE);
 	}
 
 	return true;
@@ -2117,7 +2118,8 @@ ExecUpdatePrologue(ModifyTableContext *c
 
 		return ExecBRUpdateTriggers(context->estate, context->epqstate,
 									resultRelInfo, tupleid, oldtuple, slot,
-									result, &context->tmfd);
+									result, &context->tmfd,
+									context->mtstate->operation == CMD_MERGE);
 	}
 
 	return true;
diff --git a/src/include/commands/trigger.h b/src/include/commands/trigger.h
new file mode 100644
index 2ed2c4b..cfd7daa
--- a/src/include/commands/trigger.h
+++ b/src/include/commands/trigger.h
@@ -213,7 +213,8 @@ extern bool ExecBRDeleteTriggers(EState
 								 HeapTuple fdw_trigtuple,
 								 TupleTableSlot **epqslot,
 								 TM_Result *tmresult,
-								 TM_FailureData *tmfd);
+								 TM_FailureData *tmfd,
+								 bool is_merge_delete);
 extern void ExecARDeleteTriggers(EState *estate,
 								 ResultRelInfo *relinfo,
 								 ItemPointer tupleid,
@@ -235,7 +236,8 @@ extern bool ExecBRUpdateTriggers(EState
 								 HeapTuple fdw_trigtuple,
 								 TupleTableSlot *newslot,
 								 TM_Result *tmresult,
-								 TM_FailureData *tmfd);
+								 TM_FailureData *tmfd,
+								 bool is_merge_update);
 extern void ExecARUpdateTriggers(EState *estate,
 								 ResultRelInfo *relinfo,
 								 ResultRelInfo *src_partinfo,
diff --git a/src/test/isolation/expected/merge-match-recheck.out b/src/test/isolation/expected/merge-match-recheck.out
new file mode 100644
index 9a44a59..90300f1
--- a/src/test/isolation/expected/merge-match-recheck.out
+++ b/src/test/isolation/expected/merge-match-recheck.out
@@ -241,19 +241,28 @@ starting permutation: update_bal1_tg mer
 s2: NOTICE:  Update: (1,160,s1,setup) -> (1,50,s1,"setup updated by update_bal1_tg")
 step update_bal1_tg: UPDATE target_tg t SET balance = 50, val = t.val || ' updated by update_bal1_tg' WHERE t.key = 1;
 step merge_bal_tg: 
-  MERGE INTO target_tg t
-  USING (SELECT 1 as key) s
-  ON s.key = t.key
-  WHEN MATCHED AND balance < 100 THEN
-	UPDATE SET balance = balance * 2, val = t.val || ' when1'
-  WHEN MATCHED AND balance < 200 THEN
-	UPDATE SET balance = balance * 4, val = t.val || ' when2'
-  WHEN MATCHED AND balance < 300 THEN
-	UPDATE SET balance = balance * 8, val = t.val || ' when3';
+  WITH t AS (
+    MERGE INTO target_tg t
+    USING (SELECT 1 as key) s
+    ON s.key = t.key
+    WHEN MATCHED AND balance < 100 THEN
+      UPDATE SET balance = balance * 2, val = t.val || ' when1'
+    WHEN MATCHED AND balance < 200 THEN
+      UPDATE SET balance = balance * 4, val = t.val || ' when2'
+    WHEN MATCHED AND balance < 300 THEN
+      UPDATE SET balance = balance * 8, val = t.val || ' when3'
+    RETURNING t.*
+  )
+  SELECT * FROM t;
  <waiting ...>
 step c2: COMMIT;
 s1: NOTICE:  Update: (1,50,s1,"setup updated by update_bal1_tg") -> (1,100,s1,"setup updated by update_bal1_tg when1")
 step merge_bal_tg: <... completed>
+key|balance|status|val                                  
+---+-------+------+-------------------------------------
+  1|    100|s1    |setup updated by update_bal1_tg when1
+(1 row)
+
 step select1_tg: SELECT * FROM target_tg;
 key|balance|status|val                                  
 ---+-------+------+-------------------------------------
diff --git a/src/test/isolation/specs/merge-match-recheck.spec b/src/test/isolation/specs/merge-match-recheck.spec
new file mode 100644
index 26266b8..15226e4
--- a/src/test/isolation/specs/merge-match-recheck.spec
+++ b/src/test/isolation/specs/merge-match-recheck.spec
@@ -99,15 +99,19 @@ step "merge_bal_pa"
 }
 step "merge_bal_tg"
 {
-  MERGE INTO target_tg t
-  USING (SELECT 1 as key) s
-  ON s.key = t.key
-  WHEN MATCHED AND balance < 100 THEN
-	UPDATE SET balance = balance * 2, val = t.val || ' when1'
-  WHEN MATCHED AND balance < 200 THEN
-	UPDATE SET balance = balance * 4, val = t.val || ' when2'
-  WHEN MATCHED AND balance < 300 THEN
-	UPDATE SET balance = balance * 8, val = t.val || ' when3';
+  WITH t AS (
+    MERGE INTO target_tg t
+    USING (SELECT 1 as key) s
+    ON s.key = t.key
+    WHEN MATCHED AND balance < 100 THEN
+      UPDATE SET balance = balance * 2, val = t.val || ' when1'
+    WHEN MATCHED AND balance < 200 THEN
+      UPDATE SET balance = balance * 4, val = t.val || ' when2'
+    WHEN MATCHED AND balance < 300 THEN
+      UPDATE SET balance = balance * 8, val = t.val || ' when3'
+    RETURNING t.*
+  )
+  SELECT * FROM t;
 }
 
 step "merge_delete"


  [text/x-patch] fix-merge-bug-17.patch (12.8K, ../../CAEZATCWZahtK-OT5fa2=8b=+BUuDtsspkvur4x8WkjgHbxJxkw@mail.gmail.com/3-fix-merge-bug-17.patch)
  download | inline diff:
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
new file mode 100644
index ea5cc10..ad28abe
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -79,6 +79,7 @@ static bool GetTupleForTrigger(EState *e
 							   ItemPointer tid,
 							   LockTupleMode lockmode,
 							   TupleTableSlot *oldslot,
+							   bool do_epq_recheck,
 							   TupleTableSlot **epqslot,
 							   TM_Result *tmresultp,
 							   TM_FailureData *tmfdp);
@@ -2679,13 +2680,14 @@ ExecASDeleteTriggers(EState *estate, Res
  * back the concurrently updated tuple if any.
  */
 bool
-ExecBRDeleteTriggers(EState *estate, EPQState *epqstate,
-					 ResultRelInfo *relinfo,
-					 ItemPointer tupleid,
-					 HeapTuple fdw_trigtuple,
-					 TupleTableSlot **epqslot,
-					 TM_Result *tmresult,
-					 TM_FailureData *tmfd)
+ExecBRDeleteTriggersNew(EState *estate, EPQState *epqstate,
+						ResultRelInfo *relinfo,
+						ItemPointer tupleid,
+						HeapTuple fdw_trigtuple,
+						TupleTableSlot **epqslot,
+						TM_Result *tmresult,
+						TM_FailureData *tmfd,
+						bool is_merge_delete)
 {
 	TupleTableSlot *slot = ExecGetTriggerOldSlot(estate, relinfo);
 	TriggerDesc *trigdesc = relinfo->ri_TrigDesc;
@@ -2700,9 +2702,17 @@ ExecBRDeleteTriggers(EState *estate, EPQ
 	{
 		TupleTableSlot *epqslot_candidate = NULL;
 
+		/*
+		 * Get a copy of the on-disk tuple we are planning to delete.  In
+		 * general, if the tuple has been concurrently updated, we should
+		 * recheck it using EPQ.  However, if this is a MERGE DELETE action,
+		 * we skip this EPQ recheck and leave it to the caller (it must do
+		 * additional rechecking, and might end up executing a different
+		 * action entirely).
+		 */
 		if (!GetTupleForTrigger(estate, epqstate, relinfo, tupleid,
-								LockTupleExclusive, slot, &epqslot_candidate,
-								tmresult, tmfd))
+								LockTupleExclusive, slot, !is_merge_delete,
+								&epqslot_candidate, tmresult, tmfd))
 			return false;
 
 		/*
@@ -2766,6 +2776,24 @@ ExecBRDeleteTriggers(EState *estate, EPQ
 }
 
 /*
+ * ABI-compatible wrapper to emulate old version of the above function.
+ * Do not call this version in new code.
+ */
+bool
+ExecBRDeleteTriggers(EState *estate, EPQState *epqstate,
+					 ResultRelInfo *relinfo,
+					 ItemPointer tupleid,
+					 HeapTuple fdw_trigtuple,
+					 TupleTableSlot **epqslot,
+					 TM_Result *tmresult,
+					 TM_FailureData *tmfd)
+{
+	return ExecBRDeleteTriggersNew(estate, epqstate, relinfo, tupleid,
+								   fdw_trigtuple, epqslot, tmresult, tmfd,
+								   false);
+}
+
+/*
  * Note: is_crosspart_update must be true if the DELETE is being performed
  * as part of a cross-partition update.
  */
@@ -2792,6 +2820,7 @@ ExecARDeleteTriggers(EState *estate,
 							   tupleid,
 							   LockTupleExclusive,
 							   slot,
+							   false,
 							   NULL,
 							   NULL,
 							   NULL);
@@ -2930,13 +2959,14 @@ ExecASUpdateTriggers(EState *estate, Res
 }
 
 bool
-ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
-					 ResultRelInfo *relinfo,
-					 ItemPointer tupleid,
-					 HeapTuple fdw_trigtuple,
-					 TupleTableSlot *newslot,
-					 TM_Result *tmresult,
-					 TM_FailureData *tmfd)
+ExecBRUpdateTriggersNew(EState *estate, EPQState *epqstate,
+						ResultRelInfo *relinfo,
+						ItemPointer tupleid,
+						HeapTuple fdw_trigtuple,
+						TupleTableSlot *newslot,
+						TM_Result *tmresult,
+						TM_FailureData *tmfd,
+						bool is_merge_update)
 {
 	TriggerDesc *trigdesc = relinfo->ri_TrigDesc;
 	TupleTableSlot *oldslot = ExecGetTriggerOldSlot(estate, relinfo);
@@ -2957,10 +2987,17 @@ ExecBRUpdateTriggers(EState *estate, EPQ
 	{
 		TupleTableSlot *epqslot_candidate = NULL;
 
-		/* get a copy of the on-disk tuple we are planning to update */
+		/*
+		 * Get a copy of the on-disk tuple we are planning to update.  In
+		 * general, if the tuple has been concurrently updated, we should
+		 * recheck it using EPQ.  However, if this is a MERGE UPDATE action,
+		 * we skip this EPQ recheck and leave it to the caller (it must do
+		 * additional rechecking, and might end up executing a different
+		 * action entirely).
+		 */
 		if (!GetTupleForTrigger(estate, epqstate, relinfo, tupleid,
-								lockmode, oldslot, &epqslot_candidate,
-								tmresult, tmfd))
+								lockmode, oldslot, !is_merge_update,
+								&epqslot_candidate, tmresult, tmfd))
 			return false;		/* cancel the update action */
 
 		/*
@@ -3083,6 +3120,24 @@ ExecBRUpdateTriggers(EState *estate, EPQ
 }
 
 /*
+ * ABI-compatible wrapper to emulate old version of the above function.
+ * Do not call this version in new code.
+ */
+bool
+ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
+					 ResultRelInfo *relinfo,
+					 ItemPointer tupleid,
+					 HeapTuple fdw_trigtuple,
+					 TupleTableSlot *newslot,
+					 TM_Result *tmresult,
+					 TM_FailureData *tmfd)
+{
+	return ExecBRUpdateTriggersNew(estate, epqstate, relinfo, tupleid,
+								   fdw_trigtuple, newslot, tmresult, tmfd,
+								   false);
+}
+
+/*
  * Note: 'src_partinfo' and 'dst_partinfo', when non-NULL, refer to the source
  * and destination partitions, respectively, of a cross-partition update of
  * the root partitioned table mentioned in the query, given by 'relinfo'.
@@ -3132,6 +3187,7 @@ ExecARUpdateTriggers(EState *estate, Res
 							   tupleid,
 							   LockTupleExclusive,
 							   oldslot,
+							   false,
 							   NULL,
 							   NULL,
 							   NULL);
@@ -3288,6 +3344,7 @@ GetTupleForTrigger(EState *estate,
 				   ItemPointer tid,
 				   LockTupleMode lockmode,
 				   TupleTableSlot *oldslot,
+				   bool do_epq_recheck,
 				   TupleTableSlot **epqslot,
 				   TM_Result *tmresultp,
 				   TM_FailureData *tmfdp)
@@ -3347,29 +3404,30 @@ GetTupleForTrigger(EState *estate,
 				if (tmfd.traversed)
 				{
 					/*
-					 * Recheck the tuple using EPQ. For MERGE, we leave this
-					 * to the caller (it must do additional rechecking, and
-					 * might end up executing a different action entirely).
+					 * Recheck the tuple using EPQ, if requested.  Otherwise,
+					 * just return that it was concurrently updated.
 					 */
-					if (estate->es_plannedstmt->commandType == CMD_MERGE)
+					if (do_epq_recheck)
 					{
-						if (tmresultp)
-							*tmresultp = TM_Updated;
-						return false;
-					}
-
-					*epqslot = EvalPlanQual(epqstate,
-											relation,
-											relinfo->ri_RangeTableIndex,
-											oldslot);
+						*epqslot = EvalPlanQual(epqstate,
+												relation,
+												relinfo->ri_RangeTableIndex,
+												oldslot);
 
-					/*
-					 * If PlanQual failed for updated tuple - we must not
-					 * process this tuple!
-					 */
-					if (TupIsNull(*epqslot))
+						/*
+						 * If PlanQual failed for updated tuple - we must not
+						 * process this tuple!
+						 */
+						if (TupIsNull(*epqslot))
+						{
+							*epqslot = NULL;
+							return false;
+						}
+					}
+					else
 					{
-						*epqslot = NULL;
+						if (tmresultp)
+							*tmresultp = TM_Updated;
 						return false;
 					}
 				}
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
new file mode 100644
index a0d1091..c230b66
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -1349,9 +1349,10 @@ ExecDeletePrologue(ModifyTableContext *c
 		if (context->estate->es_insert_pending_result_relations != NIL)
 			ExecPendingInserts(context->estate);
 
-		return ExecBRDeleteTriggers(context->estate, context->epqstate,
-									resultRelInfo, tupleid, oldtuple,
-									epqreturnslot, result, &context->tmfd);
+		return ExecBRDeleteTriggersNew(context->estate, context->epqstate,
+									   resultRelInfo, tupleid, oldtuple,
+									   epqreturnslot, result, &context->tmfd,
+									   context->mtstate->operation == CMD_MERGE);
 	}
 
 	return true;
@@ -1947,9 +1948,10 @@ ExecUpdatePrologue(ModifyTableContext *c
 		if (context->estate->es_insert_pending_result_relations != NIL)
 			ExecPendingInserts(context->estate);
 
-		return ExecBRUpdateTriggers(context->estate, context->epqstate,
-									resultRelInfo, tupleid, oldtuple, slot,
-									result, &context->tmfd);
+		return ExecBRUpdateTriggersNew(context->estate, context->epqstate,
+									   resultRelInfo, tupleid, oldtuple, slot,
+									   result, &context->tmfd,
+									   context->mtstate->operation == CMD_MERGE);
 	}
 
 	return true;
diff --git a/src/include/commands/trigger.h b/src/include/commands/trigger.h
new file mode 100644
index 8a5a9fe..f9e4dc4
--- a/src/include/commands/trigger.h
+++ b/src/include/commands/trigger.h
@@ -206,6 +206,15 @@ extern void ExecBSDeleteTriggers(EState
 extern void ExecASDeleteTriggers(EState *estate,
 								 ResultRelInfo *relinfo,
 								 TransitionCaptureState *transition_capture);
+extern bool ExecBRDeleteTriggersNew(EState *estate,
+									EPQState *epqstate,
+									ResultRelInfo *relinfo,
+									ItemPointer tupleid,
+									HeapTuple fdw_trigtuple,
+									TupleTableSlot **epqslot,
+									TM_Result *tmresult,
+									TM_FailureData *tmfd,
+									bool is_merge_delete);
 extern bool ExecBRDeleteTriggers(EState *estate,
 								 EPQState *epqstate,
 								 ResultRelInfo *relinfo,
@@ -228,6 +237,15 @@ extern void ExecBSUpdateTriggers(EState
 extern void ExecASUpdateTriggers(EState *estate,
 								 ResultRelInfo *relinfo,
 								 TransitionCaptureState *transition_capture);
+extern bool ExecBRUpdateTriggersNew(EState *estate,
+									EPQState *epqstate,
+									ResultRelInfo *relinfo,
+									ItemPointer tupleid,
+									HeapTuple fdw_trigtuple,
+									TupleTableSlot *newslot,
+									TM_Result *tmresult,
+									TM_FailureData *tmfd,
+									bool is_merge_update);
 extern bool ExecBRUpdateTriggers(EState *estate,
 								 EPQState *epqstate,
 								 ResultRelInfo *relinfo,
diff --git a/src/test/isolation/expected/merge-match-recheck.out b/src/test/isolation/expected/merge-match-recheck.out
new file mode 100644
index 9a44a59..90300f1
--- a/src/test/isolation/expected/merge-match-recheck.out
+++ b/src/test/isolation/expected/merge-match-recheck.out
@@ -241,19 +241,28 @@ starting permutation: update_bal1_tg mer
 s2: NOTICE:  Update: (1,160,s1,setup) -> (1,50,s1,"setup updated by update_bal1_tg")
 step update_bal1_tg: UPDATE target_tg t SET balance = 50, val = t.val || ' updated by update_bal1_tg' WHERE t.key = 1;
 step merge_bal_tg: 
-  MERGE INTO target_tg t
-  USING (SELECT 1 as key) s
-  ON s.key = t.key
-  WHEN MATCHED AND balance < 100 THEN
-	UPDATE SET balance = balance * 2, val = t.val || ' when1'
-  WHEN MATCHED AND balance < 200 THEN
-	UPDATE SET balance = balance * 4, val = t.val || ' when2'
-  WHEN MATCHED AND balance < 300 THEN
-	UPDATE SET balance = balance * 8, val = t.val || ' when3';
+  WITH t AS (
+    MERGE INTO target_tg t
+    USING (SELECT 1 as key) s
+    ON s.key = t.key
+    WHEN MATCHED AND balance < 100 THEN
+      UPDATE SET balance = balance * 2, val = t.val || ' when1'
+    WHEN MATCHED AND balance < 200 THEN
+      UPDATE SET balance = balance * 4, val = t.val || ' when2'
+    WHEN MATCHED AND balance < 300 THEN
+      UPDATE SET balance = balance * 8, val = t.val || ' when3'
+    RETURNING t.*
+  )
+  SELECT * FROM t;
  <waiting ...>
 step c2: COMMIT;
 s1: NOTICE:  Update: (1,50,s1,"setup updated by update_bal1_tg") -> (1,100,s1,"setup updated by update_bal1_tg when1")
 step merge_bal_tg: <... completed>
+key|balance|status|val                                  
+---+-------+------+-------------------------------------
+  1|    100|s1    |setup updated by update_bal1_tg when1
+(1 row)
+
 step select1_tg: SELECT * FROM target_tg;
 key|balance|status|val                                  
 ---+-------+------+-------------------------------------
diff --git a/src/test/isolation/specs/merge-match-recheck.spec b/src/test/isolation/specs/merge-match-recheck.spec
new file mode 100644
index 298b2bf..22688bb
--- a/src/test/isolation/specs/merge-match-recheck.spec
+++ b/src/test/isolation/specs/merge-match-recheck.spec
@@ -99,15 +99,19 @@ step "merge_bal_pa"
 }
 step "merge_bal_tg"
 {
-  MERGE INTO target_tg t
-  USING (SELECT 1 as key) s
-  ON s.key = t.key
-  WHEN MATCHED AND balance < 100 THEN
-	UPDATE SET balance = balance * 2, val = t.val || ' when1'
-  WHEN MATCHED AND balance < 200 THEN
-	UPDATE SET balance = balance * 4, val = t.val || ' when2'
-  WHEN MATCHED AND balance < 300 THEN
-	UPDATE SET balance = balance * 8, val = t.val || ' when3';
+  WITH t AS (
+    MERGE INTO target_tg t
+    USING (SELECT 1 as key) s
+    ON s.key = t.key
+    WHEN MATCHED AND balance < 100 THEN
+      UPDATE SET balance = balance * 2, val = t.val || ' when1'
+    WHEN MATCHED AND balance < 200 THEN
+      UPDATE SET balance = balance * 4, val = t.val || ' when2'
+    WHEN MATCHED AND balance < 300 THEN
+      UPDATE SET balance = balance * 8, val = t.val || ' when3'
+    RETURNING t.*
+  )
+  SELECT * FROM t;
 }
 
 step "merge_delete"


^ permalink  raw  reply  [nested|flat] 12+ messages in thread

* Re: BUG #18986: SIGSEGV in nodeModifyTable.c during Parallel Execution
@ 2025-07-17 05:44  Michael Paquier <michael@paquier.xyz>
  parent: Dean Rasheed <dean.a.rasheed@gmail.com>
  0 siblings, 1 reply; 12+ messages in thread

From: Michael Paquier @ 2025-07-17 05:44 UTC (permalink / raw)
  To: Dean Rasheed <dean.a.rasheed@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; y@lll.gd; pgsql-bugs@lists.postgresql.org

On Wed, Jul 16, 2025 at 02:05:06PM +0100, Dean Rasheed wrote:
> I decided to do this by adding an extra "is_merge_update" boolean
> parameter, rather than passing the commandType because that looked
> slightly neater. It was also necessary to update
> ExecBRDeleteTriggers(), since otherwise a concurrent MERGE DELETE
> could do the wrong thing (execute the wrong action, rather than
> seg-faulting). That was picked up by an existing isolation test case
> added by 9321c79, so no need for more tests.

Looks sensible here, for the HEAD and v17 flavors.

> I haven't tested this against the OP's reproducer.

Except waiting for the OP to actually test the fix, I'm not sure if
there is much that we can do.  Anyway, the stack that one gets with
the isolation test is very close to what the OP has reported, so I'm
ready to bet a coffee that you have been able to narrow this one down.

I have also raised an issue to timescale to make them aware of the
issue.  I have no idea if this is an issue for them, just acting as a
reporter:
https://github.com/timescale/timescaledb/issues/8392
--
Michael

Attachments:

  [application/pgp-signature] signature.asc (832B, ../../aHiN1aldYZfbAvMO@paquier.xyz/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 12+ messages in thread

* Re: BUG #18986: SIGSEGV in nodeModifyTable.c during Parallel Execution
@ 2025-07-17 06:46  Dean Rasheed <dean.a.rasheed@gmail.com>
  parent: Michael Paquier <michael@paquier.xyz>
  0 siblings, 1 reply; 12+ messages in thread

From: Dean Rasheed @ 2025-07-17 06:46 UTC (permalink / raw)
  To: Michael Paquier <michael@paquier.xyz>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; y@lll.gd; pgsql-bugs@lists.postgresql.org

On Thu, 17 Jul 2025 at 06:45, Michael Paquier <michael@paquier.xyz> wrote:
>
> On Wed, Jul 16, 2025 at 02:05:06PM +0100, Dean Rasheed wrote:
> > I decided to do this by adding an extra "is_merge_update" boolean
> > parameter, rather than passing the commandType because that looked
> > slightly neater. It was also necessary to update
> > ExecBRDeleteTriggers(), since otherwise a concurrent MERGE DELETE
> > could do the wrong thing (execute the wrong action, rather than
> > seg-faulting). That was picked up by an existing isolation test case
> > added by 9321c79, so no need for more tests.
>
> Looks sensible here, for the HEAD and v17 flavors.

Thanks for looking.

> > I haven't tested this against the OP's reproducer.
>
> Except waiting for the OP to actually test the fix, I'm not sure if
> there is much that we can do.

I'll push this in a day or so in any case, since it's clearly fixing
*an* issue, even if it doesn't entirely fix the OP's issue.

> Anyway, the stack that one gets with
> the isolation test is very close to what the OP has reported, so I'm
> ready to bet a coffee that you have been able to narrow this one down.

Yeah, that was my thinking too.

> I have also raised an issue to timescale to make them aware of the
> issue.  I have no idea if this is an issue for them, just acting as a
> reporter:
> https://github.com/timescale/timescaledb/issues/8392

Cool, thanks for doing that.

Regards,
Dean





^ permalink  raw  reply  [nested|flat] 12+ messages in thread

* Re: BUG #18986: SIGSEGV in nodeModifyTable.c during Parallel Execution
@ 2025-07-18 09:19  Dean Rasheed <dean.a.rasheed@gmail.com>
  parent: Dean Rasheed <dean.a.rasheed@gmail.com>
  0 siblings, 1 reply; 12+ messages in thread

From: Dean Rasheed @ 2025-07-18 09:19 UTC (permalink / raw)
  To: Michael Paquier <michael@paquier.xyz>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; y@lll.gd; pgsql-bugs@lists.postgresql.org

On Thu, 17 Jul 2025 at 07:46, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
>
> I'll push this in a day or so in any case, since it's clearly fixing
> *an* issue, even if it doesn't entirely fix the OP's issue.
>

Pushed.

Regards,
Dean





^ permalink  raw  reply  [nested|flat] 12+ messages in thread

* Re: BUG #18986: SIGSEGV in nodeModifyTable.c during Parallel Execution
@ 2025-07-18 10:36  Yaroslav Syrytsia <me@ys.lc>
  parent: Dean Rasheed <dean.a.rasheed@gmail.com>
  0 siblings, 0 replies; 12+ messages in thread

From: Yaroslav Syrytsia @ 2025-07-18 10:36 UTC (permalink / raw)
  To: Dean Rasheed <dean.a.rasheed@gmail.com>; +Cc: Michael Paquier <michael@paquier.xyz>; Tom Lane <tgl@sss.pgh.pa.us>; y@lll.gd <y@lll.gd>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>

Hello,

I just made a fresh build:
commit 91ad1bdef8e46983d82d2723c6e1c05e004f74c5 (HEAD -> 17_stable, origin/REL_17_STABLE)
Author: Dean Rasheed <dean.a.rasheed@gmail.com>
Date: Fri Jul 18 10:01:31 2025 +0100

Fix concurrent update trigger issues with MERGE in a CTE.
Can confirm that the issue is resolved now.
Thanks!
Yaroslav.
On Jul 18 2025, at 11:19 am, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
> On Thu, 17 Jul 2025 at 07:46, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
> >
> > I'll push this in a day or so in any case, since it's clearly fixing
> > *an* issue, even if it doesn't entirely fix the OP's issue.
> >
>
> Pushed.
> Regards,
> Dean
>

^ permalink  raw  reply  [nested|flat] 12+ messages in thread


end of thread, other threads:[~2025-07-18 10:36 UTC | newest]

Thread overview: 12+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2025-07-15 16:11 BUG #18986: SIGSEGV in nodeModifyTable.c during Parallel Execution PG Bug reporting form <noreply@postgresql.org>
2025-07-15 18:13 ` Tom Lane <tgl@sss.pgh.pa.us>
2025-07-16 00:10   ` Michael Paquier <michael@paquier.xyz>
2025-07-16 09:55     ` Dean Rasheed <dean.a.rasheed@gmail.com>
2025-07-16 10:42       ` Dean Rasheed <dean.a.rasheed@gmail.com>
2025-07-16 10:48         ` Michael Paquier <michael@paquier.xyz>
2025-07-16 13:05           ` Dean Rasheed <dean.a.rasheed@gmail.com>
2025-07-17 05:44             ` Michael Paquier <michael@paquier.xyz>
2025-07-17 06:46               ` Dean Rasheed <dean.a.rasheed@gmail.com>
2025-07-18 09:19                 ` Dean Rasheed <dean.a.rasheed@gmail.com>
2025-07-18 10:36                   ` Yaroslav Syrytsia <me@ys.lc>
2025-07-16 09:33   ` Yaroslav Syrytsia <me@ys.lc>

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox