public inbox for [email protected]
help / color / mirror / Atom feedFrom: Dean Rasheed <[email protected]>
To: [email protected]
To: [email protected]
Subject: Re: BUG #19380: Transition table in AFTER INSERT trigger misses rows from MERGE when used with INSERT in a CTE
Date: Tue, 20 Jan 2026 15:59:12 +0000
Message-ID: <CAEZATCVtbX5+qxbHAW=KGQUVv3hJCAC3pKZppy46Gac2t6gAsg@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
On Tue, 20 Jan 2026 at 08:58, PG Bug reporting form
<[email protected]> wrote:
>
> The following bug has been logged on the website:
>
> In a CTE that inserts rows with both MERGE and INSERT, the transition table
> will not contain the rows from the MERGE.
>
Thanks for the report.
I took a look at this and was able to reproduce it with a simpler
example, not using MERGE ... RETURNING:
WITH ins AS (
INSERT INTO merge_bug_test (id, val) values (1, 'a')
)
MERGE INTO merge_bug_test t
USING (VALUES (2, 'b')) s(id, val) ON t.id = s.id
WHEN NOT MATCHED THEN
INSERT (id, val) VALUES (s.id, s.val);
2 rows are inserted, but the transition table only includes the
(1,'a') row added by the INSERT command.
With this example, I can confirm that the bug goes all the way back to
PG15 (MERGE ... RETURNING was only added in PG17).
It looks like the problem stems from this code in
MakeTransitionCaptureState() in commands/trigger.c:
table = GetAfterTriggersTableData(relid, cmdType);
where cmdType might be CMD_MERGE.
The problem is that MERGE really needs to use the same
AfterTriggersTableData structs as INSERT, UPDATE, and DELETE, so that
any captured tuples get added to the same tuplestores.
This means that the TransitionCaptureState needs pointers to 3
separate AfterTriggersTableData structs, one for each of INSERT,
UPDATE, and DELETE. It then follows that an AfterTriggersTableData
struct only needs 2 tuplestores (old and new), rather than 4, because
it never has cmdType == CMD_MERGE.
Attached is a rough patch doing that.
I wonder if it would be possible to get rid of
ModifyTableState.mt_oc_transition_capture and just have INSERT ... ON
CONFLICT DO UPDATE use a single TransitionCaptureState in a similar
way to MERGE? Anyway, that's a separate idea, not relevant to this bug
fix.
Regards,
Dean
Attachments:
[text/x-patch] fix-transition-table-capture-in-merge-in-cte.patch (12.1K, 2-fix-transition-table-capture-in-merge-in-cte.patch)
download | inline diff:
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
new file mode 100644
index d30fda6..149bd0d
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -3919,21 +3919,10 @@ struct AfterTriggersTableData
bool after_trig_done; /* did we already queue AS triggers? */
AfterTriggerEventList after_trig_events; /* if so, saved list pointer */
- /*
- * We maintain separate transition tables for UPDATE/INSERT/DELETE since
- * MERGE can run all three actions in a single statement. Note that UPDATE
- * needs both old and new transition tables whereas INSERT needs only new,
- * and DELETE needs only old.
- */
-
- /* "old" transition table for UPDATE, if any */
- Tuplestorestate *old_upd_tuplestore;
- /* "new" transition table for UPDATE, if any */
- Tuplestorestate *new_upd_tuplestore;
- /* "old" transition table for DELETE, if any */
- Tuplestorestate *old_del_tuplestore;
- /* "new" transition table for INSERT, if any */
- Tuplestorestate *new_ins_tuplestore;
+ /* "old" transition table for UPDATE/DELETE, if any */
+ Tuplestorestate *old_tuplestore;
+ /* "new" transition table for INSERT/UPDATE, if any */
+ Tuplestorestate *new_tuplestore;
TupleTableSlot *storeslot; /* for converting to tuplestore's format */
};
@@ -3960,6 +3949,7 @@ static Tuplestorestate *GetAfterTriggers
TupleTableSlot *newslot,
TransitionCaptureState *transition_capture);
static void TransitionTableAddTuple(EState *estate,
+ int event,
TransitionCaptureState *transition_capture,
ResultRelInfo *relinfo,
TupleTableSlot *slot,
@@ -4527,19 +4517,13 @@ AfterTriggerExecute(EState *estate,
{
if (LocTriggerData.tg_trigger->tgoldtable)
{
- if (TRIGGER_FIRED_BY_UPDATE(evtshared->ats_event))
- LocTriggerData.tg_oldtable = evtshared->ats_table->old_upd_tuplestore;
- else
- LocTriggerData.tg_oldtable = evtshared->ats_table->old_del_tuplestore;
+ LocTriggerData.tg_oldtable = evtshared->ats_table->old_tuplestore;
evtshared->ats_table->closed = true;
}
if (LocTriggerData.tg_trigger->tgnewtable)
{
- if (TRIGGER_FIRED_BY_INSERT(evtshared->ats_event))
- LocTriggerData.tg_newtable = evtshared->ats_table->new_ins_tuplestore;
- else
- LocTriggerData.tg_newtable = evtshared->ats_table->new_upd_tuplestore;
+ LocTriggerData.tg_newtable = evtshared->ats_table->new_tuplestore;
evtshared->ats_table->closed = true;
}
}
@@ -4972,7 +4956,9 @@ MakeTransitionCaptureState(TriggerDesc *
need_new_upd,
need_old_del,
need_new_ins;
- AfterTriggersTableData *table;
+ AfterTriggersTableData *ins_table;
+ AfterTriggersTableData *upd_table;
+ AfterTriggersTableData *del_table;
MemoryContext oldcxt;
ResourceOwner saveResourceOwner;
@@ -5019,10 +5005,15 @@ MakeTransitionCaptureState(TriggerDesc *
AfterTriggerEnlargeQueryState();
/*
- * Find or create an AfterTriggersTableData struct to hold the
+ * Find or create an AfterTriggersTableData structs to hold the
* tuplestore(s). If there's a matching struct but it's marked closed,
* ignore it; we need a newer one.
*
+ * Note: MERGE must use the same AfterTriggersTableData structs as INSERT,
+ * UPDATE, and DELETE, so that any MERGE'd tuples are added to the same
+ * tuplestores as tuples from any INSERT, UPDATE, or DELETE commands
+ * running in the same top-level command (e.g., in a writable CTE).
+ *
* Note: the AfterTriggersTableData list, as well as the tuplestores, are
* allocated in the current (sub)transaction's CurTransactionContext, and
* the tuplestores are managed by the (sub)transaction's resource owner.
@@ -5030,21 +5021,34 @@ MakeTransitionCaptureState(TriggerDesc *
* transition tables to be deferrable; they will be fired during
* AfterTriggerEndQuery, after which it's okay to delete the data.
*/
- table = GetAfterTriggersTableData(relid, cmdType);
+ if (need_new_ins)
+ ins_table = GetAfterTriggersTableData(relid, CMD_INSERT);
+ else
+ ins_table = NULL;
+
+ if (need_old_upd || need_new_upd)
+ upd_table = GetAfterTriggersTableData(relid, CMD_UPDATE);
+ else
+ upd_table = NULL;
+
+ if (need_old_del)
+ del_table = GetAfterTriggersTableData(relid, CMD_DELETE);
+ else
+ del_table = NULL;
/* Now create required tuplestore(s), if we don't have them already. */
oldcxt = MemoryContextSwitchTo(CurTransactionContext);
saveResourceOwner = CurrentResourceOwner;
CurrentResourceOwner = CurTransactionResourceOwner;
- if (need_old_upd && table->old_upd_tuplestore == NULL)
- table->old_upd_tuplestore = tuplestore_begin_heap(false, false, work_mem);
- if (need_new_upd && table->new_upd_tuplestore == NULL)
- table->new_upd_tuplestore = tuplestore_begin_heap(false, false, work_mem);
- if (need_old_del && table->old_del_tuplestore == NULL)
- table->old_del_tuplestore = tuplestore_begin_heap(false, false, work_mem);
- if (need_new_ins && table->new_ins_tuplestore == NULL)
- table->new_ins_tuplestore = tuplestore_begin_heap(false, false, work_mem);
+ if (need_old_upd && upd_table->old_tuplestore == NULL)
+ upd_table->old_tuplestore = tuplestore_begin_heap(false, false, work_mem);
+ if (need_new_upd && upd_table->new_tuplestore == NULL)
+ upd_table->new_tuplestore = tuplestore_begin_heap(false, false, work_mem);
+ if (need_old_del && del_table->old_tuplestore == NULL)
+ del_table->old_tuplestore = tuplestore_begin_heap(false, false, work_mem);
+ if (need_new_ins && ins_table->new_tuplestore == NULL)
+ ins_table->new_tuplestore = tuplestore_begin_heap(false, false, work_mem);
CurrentResourceOwner = saveResourceOwner;
MemoryContextSwitchTo(oldcxt);
@@ -5055,7 +5059,9 @@ MakeTransitionCaptureState(TriggerDesc *
state->tcs_update_old_table = need_old_upd;
state->tcs_update_new_table = need_new_upd;
state->tcs_insert_new_table = need_new_ins;
- state->tcs_private = table;
+ state->tcs_insert_private = ins_table;
+ state->tcs_update_private = upd_table;
+ state->tcs_delete_private = del_table;
return state;
}
@@ -5233,20 +5239,12 @@ AfterTriggerFreeQuery(AfterTriggersQuery
{
AfterTriggersTableData *table = (AfterTriggersTableData *) lfirst(lc);
- ts = table->old_upd_tuplestore;
- table->old_upd_tuplestore = NULL;
- if (ts)
- tuplestore_end(ts);
- ts = table->new_upd_tuplestore;
- table->new_upd_tuplestore = NULL;
- if (ts)
- tuplestore_end(ts);
- ts = table->old_del_tuplestore;
- table->old_del_tuplestore = NULL;
+ ts = table->old_tuplestore;
+ table->old_tuplestore = NULL;
if (ts)
tuplestore_end(ts);
- ts = table->new_ins_tuplestore;
- table->new_ins_tuplestore = NULL;
+ ts = table->new_tuplestore;
+ table->new_tuplestore = NULL;
if (ts)
tuplestore_end(ts);
if (table->storeslot)
@@ -5557,17 +5555,17 @@ GetAfterTriggersTransitionTable(int even
{
Assert(TupIsNull(newslot));
if (event == TRIGGER_EVENT_DELETE && delete_old_table)
- tuplestore = transition_capture->tcs_private->old_del_tuplestore;
+ tuplestore = transition_capture->tcs_delete_private->old_tuplestore;
else if (event == TRIGGER_EVENT_UPDATE && update_old_table)
- tuplestore = transition_capture->tcs_private->old_upd_tuplestore;
+ tuplestore = transition_capture->tcs_update_private->old_tuplestore;
}
else if (!TupIsNull(newslot))
{
Assert(TupIsNull(oldslot));
if (event == TRIGGER_EVENT_INSERT && insert_new_table)
- tuplestore = transition_capture->tcs_private->new_ins_tuplestore;
+ tuplestore = transition_capture->tcs_insert_private->new_tuplestore;
else if (event == TRIGGER_EVENT_UPDATE && update_new_table)
- tuplestore = transition_capture->tcs_private->new_upd_tuplestore;
+ tuplestore = transition_capture->tcs_update_private->new_tuplestore;
}
return tuplestore;
@@ -5581,6 +5579,7 @@ GetAfterTriggersTransitionTable(int even
*/
static void
TransitionTableAddTuple(EState *estate,
+ int event,
TransitionCaptureState *transition_capture,
ResultRelInfo *relinfo,
TupleTableSlot *slot,
@@ -5599,9 +5598,26 @@ TransitionTableAddTuple(EState *estate,
tuplestore_puttupleslot(tuplestore, original_insert_tuple);
else if ((map = ExecGetChildToRootMap(relinfo)) != NULL)
{
- AfterTriggersTableData *table = transition_capture->tcs_private;
+ AfterTriggersTableData *table;
TupleTableSlot *storeslot;
+ switch (event)
+ {
+ case TRIGGER_EVENT_INSERT:
+ table = transition_capture->tcs_insert_private;
+ break;
+ case TRIGGER_EVENT_UPDATE:
+ table = transition_capture->tcs_update_private;
+ break;
+ case TRIGGER_EVENT_DELETE:
+ table = transition_capture->tcs_delete_private;
+ break;
+ default:
+ elog(ERROR, "invalid after-trigger event code: %d", event);
+ table = NULL; /* keep compiler quiet */
+ break;
+ }
+
storeslot = GetAfterTriggersStoreSlot(table, map->outdesc);
execute_attr_map_slot(map->attrMap, slot, storeslot);
tuplestore_puttupleslot(tuplestore, storeslot);
@@ -6195,7 +6211,7 @@ AfterTriggerSaveEvent(EState *estate, Re
oldslot,
NULL,
transition_capture);
- TransitionTableAddTuple(estate, transition_capture, relinfo,
+ TransitionTableAddTuple(estate, event, transition_capture, relinfo,
oldslot, NULL, old_tuplestore);
}
@@ -6211,7 +6227,7 @@ AfterTriggerSaveEvent(EState *estate, Re
NULL,
newslot,
transition_capture);
- TransitionTableAddTuple(estate, transition_capture, relinfo,
+ TransitionTableAddTuple(estate, event, transition_capture, relinfo,
newslot, original_insert_tuple, new_tuplestore);
}
@@ -6514,7 +6530,23 @@ AfterTriggerSaveEvent(EState *estate, Re
new_shared.ats_firing_id = 0;
if ((trigger->tgoldtable || trigger->tgnewtable) &&
transition_capture != NULL)
- new_shared.ats_table = transition_capture->tcs_private;
+ {
+ switch (event)
+ {
+ case TRIGGER_EVENT_INSERT:
+ new_shared.ats_table = transition_capture->tcs_insert_private;
+ break;
+ case TRIGGER_EVENT_UPDATE:
+ new_shared.ats_table = transition_capture->tcs_update_private;
+ break;
+ case TRIGGER_EVENT_DELETE:
+ new_shared.ats_table = transition_capture->tcs_delete_private;
+ break;
+ default:
+ new_shared.ats_table = NULL;
+ break;
+ }
+ }
else
new_shared.ats_table = NULL;
new_shared.ats_modifiedcols = modifiedCols;
diff --git a/src/include/commands/trigger.h b/src/include/commands/trigger.h
new file mode 100644
index b60317c..556c86b
--- a/src/include/commands/trigger.h
+++ b/src/include/commands/trigger.h
@@ -78,7 +78,9 @@ typedef struct TransitionCaptureState
/*
* Private data including the tuplestore(s) into which to insert tuples.
*/
- struct AfterTriggersTableData *tcs_private;
+ struct AfterTriggersTableData *tcs_insert_private;
+ struct AfterTriggersTableData *tcs_update_private;
+ struct AfterTriggersTableData *tcs_delete_private;
} TransitionCaptureState;
/*
diff --git a/src/test/regress/expected/triggers.out b/src/test/regress/expected/triggers.out
new file mode 100644
index 1eb8fba..1acdd12
--- a/src/test/regress/expected/triggers.out
+++ b/src/test/regress/expected/triggers.out
@@ -3033,13 +3033,19 @@ NOTICE: trigger = table1_trig, new tabl
with wcte as (insert into table1 values (43))
insert into table1 values (44);
NOTICE: trigger = table1_trig, new table = (43), (44)
+with wcte as (insert into table1 values (45))
+ merge into table1 using (values (46)) as v(a) on table1.a = v.a
+ when not matched then insert values (v.a);
+NOTICE: trigger = table1_trig, new table = (45), (46)
select * from table1;
a
----
42
44
43
-(3 rows)
+ 46
+ 45
+(5 rows)
select * from table2;
a
diff --git a/src/test/regress/sql/triggers.sql b/src/test/regress/sql/triggers.sql
new file mode 100644
index 5f7f75d..cc87845
--- a/src/test/regress/sql/triggers.sql
+++ b/src/test/regress/sql/triggers.sql
@@ -2228,6 +2228,10 @@ with wcte as (insert into table1 values
with wcte as (insert into table1 values (43))
insert into table1 values (44);
+with wcte as (insert into table1 values (45))
+ merge into table1 using (values (46)) as v(a) on table1.a = v.a
+ when not matched then insert values (v.a);
+
select * from table1;
select * from table2;
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]
Subject: Re: BUG #19380: Transition table in AFTER INSERT trigger misses rows from MERGE when used with INSERT in a CTE
In-Reply-To: <CAEZATCVtbX5+qxbHAW=KGQUVv3hJCAC3pKZppy46Gac2t6gAsg@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