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"
