| From: | Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com> |
|---|---|
| To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
| Cc: | dwwoelfel(at)gmail(dot)com, pgsql-bugs(at)lists(dot)postgresql(dot)org |
| Subject: | Re: BUG #19380: Transition table in AFTER INSERT trigger misses rows from MERGE when used with INSERT in a CTE |
| Date: | 2026-01-21 11:20:37 |
| Message-ID: | CAEZATCV6CAmfsB48gN7Okd37bQ2pQ1k0TX_6fBeCJ9Hcm9u=yw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-bugs |
On Tue, 20 Jan 2026 at 17:21, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)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
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Pierre Forstmann | 2026-01-21 11:42:50 | Re: BUG #19386: Unnecessary Sort in query plan for SELECT literal with ORDER BY |
| Previous Message | Andrei Lepikhov | 2026-01-21 11:11:05 | Re: BUG #19386: Unnecessary Sort in query plan for SELECT literal with ORDER BY |