Incorrect UPDATE trigger invocation in the UPDATE clause of an UPSERT statement.

Hello,

I'd like to report a bug in the UPDATE trigger invocation when using the
new INSERT ON CONFLICT UPDATE (UPSERT) functionality.

In short, if an UPDATE trigger is invoked by the ON CONFLICT DO UPDATE
clause of an UPSERT statement - it receives the new values in both the OLD
and NEW variables. Whereas if invoked by a normal UPDATE statement - it
correctly gets the respective values in OLD and NEW.

Here's a short test case to reproduce it:

test=# CREATE OR REPLACE FUNCTION public.test_trigger() RETURNS trigger AS
$$
BEGIN
RAISE NOTICE '%:%: old: %, new: %', TG_NAME, TG_OP, OLD, NEW;
IF OLD.value IS DISTINCT FROM NEW.value THEN
RAISE WARNING '%:%: Values are different!', TG_NAME, TG_OP;
ELSE
RAISE NOTICE '%:%: Values are the same', TG_NAME, TG_OP;
END IF;
RETURN NEW;
END;
$$ LANGUAGE plpgsql;

test=# CREATE table test(id serial primary key, value varchar(32));

test=# CREATE TRIGGER test_trigger after UPDATE on test for each row
execute procedure test_trigger();

test=# INSERT INTO test (value) VALUES('initial value');

test=# SELECT * FROM test;

┌────┬───────────────┐
│ id │ value │
├────┼───────────────┤
│ 1 │ initial value │
└────┴───────────────┘
(1 row)

Now, if we do an UPDATE, everything is as expected:

test=# UPDATE test SET value='plain update value' WHERE id=1;
NOTICE: 00000: test_trigger:UPDATE: old: (1,"initial value"), new:
(1,"plain update value")
LOCATION: exec_stmt_raise, pl_exec.c:3216
WARNING: 01000: test_trigger:UPDATE: Values are different!
LOCATION: exec_stmt_raise, pl_exec.c:3216
UPDATE 1

If we do an UPSERT instead, watch how OLD and NEW are the same (NEW):

test=# INSERT INTO test (id, value) VALUES(1, 'upserted value') ON CONFLICT
ON CONSTRAINT test_pkey DO UPDATE SET value='upserted value';
NOTICE: 00000: test_trigger:UPDATE: old: (1,"upserted value"), new:
(1,"upserted value")
LOCATION: exec_stmt_raise, pl_exec.c:3216
NOTICE: 00000: test_trigger:UPDATE: Values are the same
LOCATION: exec_stmt_raise, pl_exec.c:3216
INSERT 0 1

We have traced the problem to be in the
src/backend/executor/nodeModifyTable.c file, more specifically in the
ExecOnConflictUpdate function. The attached simple patch appears to fix the
issue. Now, the UPSERT behaves correctly (at least what we think should be
correct):

test=# INSERT INTO test (id, value) VALUES(1, 'upserted value') ON CONFLICT
ON CONSTRAINT test_pkey DO UPDATE SET value='upserted value';
NOTICE: 00000: test_trigger:UPDATE: old: (1,"initial value"), new:
(1,"upserted value")
LOCATION: exec_stmt_raise, pl_exec.c:3216
WARNING: 01000: test_trigger:UPDATE: Values are different!
LOCATION: exec_stmt_raise, pl_exec.c:3216
INSERT 0 1

We have verified this behaviour with PostgreSQL 9.5 beta1, beta2 and Git
head.

Thanks.
--

-S