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

On Mon, 27 Feb 2023 at 08:06, PG Bug reporting form
<noreply(at)postgresql(dot)org> wrote:
>
> When executing the following script I get a server crash with the following stack trace:
> Program terminated with signal SIGSEGV, Segmentation fault.
>

Confirmed. I can reliably reproduce this using 2 psql sessions as follows:

-- Setup
DROP TABLE IF EXISTS target;
CREATE TABLE target (tid integer, val integer);
INSERT INTO target VALUES (1, 0);
CREATE OR REPLACE FUNCTION brut_func() RETURNS trigger LANGUAGE plpgsql AS
$$ BEGIN RETURN NEW; END; $$;
CREATE TRIGGER brut BEFORE UPDATE ON target
FOR EACH ROW EXECUTE PROCEDURE brut_func();

-- Session 1
BEGIN;
UPDATE TARGET SET val = 0;

-- Session 2
MERGE INTO target t1 USING target t2 ON t1.tid = t2.tid
WHEN MATCHED THEN UPDATE SET val = 0;

-- Session 1
COMMIT;

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.

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.

Thoughts?

Regards,
Dean