Horton

Thread

Commits

  1. Update .abi-compliance-history for change to TransitionCaptureState.

  2. Fix trigger transition table capture for MERGE in CTE queries.

  1. BUG #19380: Transition table in AFTER INSERT trigger misses rows from MERGE when used with INSERT in a CTE

    The Post Office <noreply@postgresql.org> — 2026-01-19T19:26:32Z

    The following bug has been logged on the website:

Bug reference:      19380
Logged by:          Daniel Woelfel
Email address:      dwwoelfel@gmail.com
PostgreSQL version: 17.7
Operating system:   macOS (aarch64)
Description:        

In a CTE that inserts rows with both MERGE and INSERT, the transition table
will not contain the rows from the MERGE.

I have included a small reproduction script, which inserts 2 rows with a
merge and one row with an insert. On my machine, the trigger outputs: `Row
count: 1, Rows: [{"id":3,"val":"c"}]`, but I would expect it to output: `Row
count: 3, Rows: [{"id": 1, "val": "a"}, {"id": 2, "val": "b"},
{"id":3,"val":"c"}]`

```
CREATE TEMP TABLE merge_bug_test (id INT PRIMARY KEY, val TEXT);

-- Create trigger functions that list the IDs they see
CREATE OR REPLACE FUNCTION report_insert_rows()
RETURNS TRIGGER AS $$
BEGIN
    RAISE WARNING '[AFTER INSERT TRIGGER] Row count: %, Rows: %', (SELECT
COUNT(*) FROM newrows), (SELECT
COALESCE(json_agg(row_to_json(newrows))::text, 'EMPTY') FROM newrows);
    RETURN NULL;
END;
$$ LANGUAGE plpgsql;

-- Create statement-level triggers for INSERT
CREATE TRIGGER insert_trigger
    AFTER INSERT ON merge_bug_test
    REFERENCING NEW TABLE AS newrows
    FOR EACH STATEMENT
    EXECUTE FUNCTION report_insert_rows();

-- MERGE inserts rows, but INSERT CTE inserts nothing
WITH
  input_triples (id, val) AS (
    VALUES (1, 'a'), (2, 'b')
  ),
  insert_cte AS (
    INSERT INTO merge_bug_test (id, val) values (3, 'c')
    RETURNING id
  ),
  -- Insert two row with merge
  merge_cte AS (
    MERGE INTO merge_bug_test t
    USING input_triples s
    ON t.id = s.id
    WHEN NOT MATCHED THEN
        INSERT (id, val) VALUES (s.id, s.val)
    RETURNING t.id
  )
  -- Insert one row with a regular insert

SELECT id FROM merge_cte
UNION ALL
SELECT id FROM insert_cte;

DROP TABLE merge_bug_test;
DROP FUNCTION report_insert_rows;
```

  2. Re: BUG #19380: Transition table in AFTER INSERT trigger misses rows from MERGE when used with INSERT in a CTE

    Dean Rasheed <dean.a.rasheed@gmail.com> — 2026-01-20T15:59:12Z

    On Tue, 20 Jan 2026 at 08:58, PG Bug reporting form
<noreply@postgresql.org> 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

  3. Re: BUG #19380: Transition table in AFTER INSERT trigger misses rows from MERGE when used with INSERT in a CTE

    Tom Lane <tgl@sss.pgh.pa.us> — 2026-01-20T17:21:20Z

    Dean Rasheed <dean.a.rasheed@gmail.com> writes:
> On Tue, 20 Jan 2026 at 08:58, PG Bug reporting form
> <noreply@postgresql.org> wrote:
>> In a CTE that inserts rows with both MERGE and INSERT, the transition table
>> will not contain the rows from the MERGE.

> 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.

Yeah, I had just come to the same conclusion when I saw your email.
Using CMD_MERGE as the AfterTriggersTableData lookup key could be
correct if MERGE were supposed to have separate transition tables,
but AFAICT from the spec it isn't.

> Attached is a rough patch doing that.

I haven't read this in detail, but it seems like one issue to think
about is whether it's okay to add fields to struct
TransitionCaptureState in released branches?  Although it's nominally
an ABI break, I can't think of a reason why any extension would be
manufacturing its own TransitionCaptureState structs rather than
calling MakeTransitionCaptureState(), nor should an extension be
touching the stated-to-be-private tcs_private field.  So it seems
like we should be able to get away with it.

> 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.

Yeah, that would have to be HEAD-only in any case; we're not going
to change ModifyTableState in released branches.

			regards, tom lane

  4. Re: BUG #19380: Transition table in AFTER INSERT trigger misses rows from MERGE when used with INSERT in a CTE

    Tom Lane <tgl@sss.pgh.pa.us> — 2026-01-20T18:00:56Z

    I wrote:
> Dean Rasheed <dean.a.rasheed@gmail.com> writes:
>> Attached is a rough patch doing that.

> I haven't read this in detail,

Okay, I've now been through it more carefully, and it looks good
except for these nitpicks (addressed in v2 patch attached):

* I think GetAfterTriggersTableData() ought to have an Assert that
the cmdType used as lookup key is INSERT/UPDATE/DELETE and nothing
else.

* Corrected grammar in comment in MakeTransitionCaptureState.

* Added comment in new switch in AfterTriggerSaveEvent.

One could argue that that new switch should have an explicit
case for TRIGGER_EVENT_TRUNCATE and then the default: should
be elog(ERROR).  That seems excessive to me, given that the
switch earlier in the same function vetted the event value.
But others might see it differently.

			regards, tom lane

  5. Re: BUG #19380: Transition table in AFTER INSERT trigger misses rows from MERGE when used with INSERT in a CTE

    Dean Rasheed <dean.a.rasheed@gmail.com> — 2026-01-21T11:20:37Z

    On Tue, 20 Jan 2026 at 17:21, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Dean Rasheed <dean.a.rasheed@gmail.com> writes:
>
> > 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.
>
> Yeah, I had just come to the same conclusion when I saw your email.
> Using CMD_MERGE as the AfterTriggersTableData lookup key could be
> correct if MERGE were supposed to have separate transition tables,
> but AFAICT from the spec it isn't.

Yeah, I think that's right. The spec is clear that the effects of a
MERGE action should be the same as the effects of an
INSERT/UPDATE/DELETE command, and that includes triggers. However, I
don't think the spec says anything about what should happen if there
are multiple INSERT/UPDATE/DELETE/MERGE sub-commands within a single
top-level command, because including INSERT/UPDATE/DELETE/MERGE
statements inside a WITH statement is our own extension of the
standard.

So I think that we were free to choose whether to execute
statement-level triggers once per sub-command, or once for the entire
WITH query. Since we have chosen the latter, it follows that each
MERGE sub-command must add cumulatively to the set of rows in the
INSERT/UPDATE/DELETE transition tables, just as each
INSERT/UPDATE/DELETE sub-command does.

> > Attached is a rough patch doing that.
>
> I haven't read this in detail, but it seems like one issue to think
> about is whether it's okay to add fields to struct
> TransitionCaptureState in released branches?  Although it's nominally
> an ABI break, I can't think of a reason why any extension would be
> manufacturing its own TransitionCaptureState structs rather than
> calling MakeTransitionCaptureState(), nor should an extension be
> touching the stated-to-be-private tcs_private field.  So it seems
> like we should be able to get away with it.

Yeah, I think that's probably right. FWIW, I grepped for
TransitionCaptureState in a clone of PGXN, and didn't find any uses.

Regards,
Dean

  6. Re: BUG #19380: Transition table in AFTER INSERT trigger misses rows from MERGE when used with INSERT in a CTE

    Dean Rasheed <dean.a.rasheed@gmail.com> — 2026-01-24T11:48:41Z

    On Tue, 20 Jan 2026 at 18:01, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Okay, I've now been through it more carefully, and it looks good
> except for these nitpicks (addressed in v2 patch attached):
>

OK, those make sense. Thanks for reviewing.

I've pushed this now. Expecting to have to push updates to
.abi-compliance-history soon.

Regards,
Dean

  7. Re: BUG #19380: Transition table in AFTER INSERT trigger misses rows from MERGE when used with INSERT in a CTE

    Tom Lane <tgl@sss.pgh.pa.us> — 2026-01-24T15:58:40Z

    Dean Rasheed <dean.a.rasheed@gmail.com> writes:
> I've pushed this now. Expecting to have to push updates to
> .abi-compliance-history soon.

crake has reported in [1]:

Leaf changes summary: 2 artifacts changed
Changed leaf types summary: 2 leaf types changed
Removed/Changed/Added functions summary: 0 Removed, 0 Changed, 0 Added function (2 filtered out)
Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable

'struct AfterTriggersTableData' changed:
  type size changed from 80 to 64 (in bytes)
  2 data member deletions:
    'Tuplestorestate* old_del_tuplestore', at offset 56 (in bytes)
    'Tuplestorestate* new_ins_tuplestore', at offset 64 (in bytes)
  there are data member changes:
    'TupleTableSlot* storeslot' offset changed from 72 to 56 (in bytes) (by -16 bytes)

'struct TransitionCaptureState' changed:
  type size changed from 24 to 40 (in bytes)
  2 data member insertions:
    'AfterTriggersTableData* tcs_update_private', at offset 24 (in bytes)
    'AfterTriggersTableData* tcs_delete_private', at offset 32 (in bytes)
  there are data member changes:
    name of 'TransitionCaptureState::tcs_private' changed to 'TransitionCaptureState::tcs_insert_private'


I find it unhelpful that it reported the AfterTriggersTableData
change as an ABI change, even though that struct is not globally
accessible.  I wonder if there's some libabigail switches we
should be tweaking to refine that.

			regards, tom lane

[1] https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=crake&dt=2026-01-24%2013%3A06%3A41&stg=abi-compliance-check

  8. Re: BUG #19380: Transition table in AFTER INSERT trigger misses rows from MERGE when used with INSERT in a CTE

    Andres Freund <andres@anarazel.de> — 2026-01-24T17:03:49Z

    Hi,

On 2026-01-24 10:58:40 -0500, Tom Lane wrote:
> crake has reported in [1]:
>
> Leaf changes summary: 2 artifacts changed
> Changed leaf types summary: 2 leaf types changed
> Removed/Changed/Added functions summary: 0 Removed, 0 Changed, 0 Added function (2 filtered out)
> Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
>
> 'struct AfterTriggersTableData' changed:
>   type size changed from 80 to 64 (in bytes)
>   2 data member deletions:
>     'Tuplestorestate* old_del_tuplestore', at offset 56 (in bytes)
>     'Tuplestorestate* new_ins_tuplestore', at offset 64 (in bytes)
>   there are data member changes:
>     'TupleTableSlot* storeslot' offset changed from 72 to 56 (in bytes) (by -16 bytes)
>
> 'struct TransitionCaptureState' changed:
>   type size changed from 24 to 40 (in bytes)
>   2 data member insertions:
>     'AfterTriggersTableData* tcs_update_private', at offset 24 (in bytes)
>     'AfterTriggersTableData* tcs_delete_private', at offset 32 (in bytes)
>   there are data member changes:
>     name of 'TransitionCaptureState::tcs_private' changed to 'TransitionCaptureState::tcs_insert_private'
>
>
> I find it unhelpful that it reported the AfterTriggersTableData
> change as an ABI change, even though that struct is not globally
> accessible.  I wonder if there's some libabigail switches we
> should be tweaking to refine that.

Specifying --headers-dir ... during abidw seems to do the trick.

Without --headers-dir:

'struct AfterTriggersTableData at trigger.c:3912:1' changed:
  type size changed from 80 to 64 (in bytes)
  2 data member deletions:
    'Tuplestorestate* old_del_tuplestore', at offset 56 (in bytes) at trigger.c:3934:1
    'Tuplestorestate* new_ins_tuplestore', at offset 64 (in bytes) at trigger.c:3936:1
  there are data member changes:
    'TupleTableSlot* storeslot' offset changed from 72 to 56 (in bytes) (by -16 bytes)

'struct TransitionCaptureState at trigger.h:56:1' changed:
  type size changed from 24 to 40 (in bytes)
  2 data member insertions:
    'AfterTriggersTableData* tcs_update_private', at offset 24 (in bytes) at trigger.h:82:1
    'AfterTriggersTableData* tcs_delete_private', at offset 32 (in bytes) at trigger.h:83:1
  there are data member changes:
    name of 'TransitionCaptureState::tcs_private' changed to 'TransitionCaptureState::tcs_insert_private' at trigger.h:81:1


With --headers-dir:

'struct TransitionCaptureState at trigger.h:56:1' changed:
  type size changed from 24 to 40 (in bytes)
  2 data member insertions:
    'AfterTriggersTableData* tcs_update_private', at offset 24 (in bytes) at trigger.h:82:1
    'AfterTriggersTableData* tcs_delete_private', at offset 32 (in bytes) at trigger.h:83:1

(the reason --headers-dir matters is that it implies dropping private types)


So it seems like that'd improve things...  I did check the PR adding this to
the buildfarm [1] and it doesn't use --headers-dir.


It also has the nice side-benefit of making the diffing a bit faster (65s to
48s on my older workstation).

Greetings,

Andres Freund

[1] https://github.com/PGBuildFarm/client-code/pull/38/changes

  9. Re: BUG #19380: Transition table in AFTER INSERT trigger misses rows from MERGE when used with INSERT in a CTE

    Tom Lane <tgl@sss.pgh.pa.us> — 2026-01-24T17:27:35Z

    Andres Freund <andres@anarazel.de> writes:
> On 2026-01-24 10:58:40 -0500, Tom Lane wrote:
>> I find it unhelpful that it reported the AfterTriggersTableData
>> change as an ABI change, even though that struct is not globally
>> accessible.  I wonder if there's some libabigail switches we
>> should be tweaking to refine that.

> Specifying --headers-dir ... during abidw seems to do the trick.

Yeah, I was just experimenting with that.  It's pretty unclear
from the abigail docs whether you have to name all the header
directories explicitly or it's enough to point at the top one ...
but experimentation says the top one is enough.

I'll send Andrew a patch.

			regards, tom lane