Re: BUG #17809: MERGE ... UPDATE fails with BEFORE ROW UPDATE trigger when target row updated concurrently

From: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
To: Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>
Cc: exclusion(at)gmail(dot)com, pgsql-bugs(at)lists(dot)postgresql(dot)org
Subject: Re: BUG #17809: MERGE ... UPDATE fails with BEFORE ROW UPDATE trigger when target row updated concurrently
Date: 2023-03-07 11:50:52
Message-ID: 20230307115052.6f4t672n5ssfigv2@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

On 2023-Feb-27, Dean Rasheed wrote:

> What's happening is that (in session 2) the trigger code in
> ExecBRUpdateTriggers() detects the concurrent update and calls
> ExecGetUpdateNewTuple() to get a new tuple. However,
> ExecGetUpdateNewTuple() just calls internalGetUpdateNewTuple(), when
> really it ought to call mergeGetUpdateNewTuple(), since we're in a
> MERGE. Unfortunately there's no way for ExecGetUpdateNewTuple() to
> know that, because it doesn't have the required context information.

Hmm, right.

> A quick hack that fixes the problem is to pass the current
> MergeActionState to ExecGetUpdateNewTuple(), which also then requires
> that it be passed to ExecBRUpdateTriggers(). Changing the signature of
> 2 public functions in that way in back branches doesn't seem very nice
> though.
>
> OTOH, it's difficult to see how it can be fixed any other way.
>
> I don't really like this change to ExecGetUpdateNewTuple(), but if we
> are going to change it like this, then I think we should go all the
> way and get rid of the GetUpdateNewTuple callback, and the separate
> internalGetUpdateNewTuple() and mergeGetUpdateNewTuple() functions,
> and just do it all in ExecGetUpdateNewTuple(), so that all the code is
> in one place, making it easier to follow.

I'm pretty sure that I split things this way (using a callback) because
the MERGE code was at arms-length, being in a different file, and there
was no reasonable way, without duplicating a lot of other code, to
implement the different behavior other than using a callback.

I then moved the MERGE code from a separate file into nodeModifyTable.c
and could have put it back in one piece, but didn't remember that it was
there. It even looks like doing that may even make the code simpler, so
perhaps we should just do that -- nodeModifyTable.c if fully aware of
MERGE now, so it's not a problem.

--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"Puedes vivir sólo una vez, pero si lo haces bien, una vez es suficiente"

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Tom Lane 2023-03-07 18:43:05 Re: BUG #17811: Replacing an underlying view breaks OLD/NEW tuple when accessing it via upper-level view
Previous Message Alvaro Herrera 2023-03-07 11:25:47 Re: 'CLUSTER' in one database prevents running it in two others on the same database cluster (PG15.2)