On Thu, Dec 3, 2015 at 4:34 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> triggers.out is telling that this may be intended to work this way:
> WARNING: after update (old): (5,"updated green trig modified")
> WARNING: after update (new): (5,"updated green trig modified")
> Though I agree that it is not instinctive...
>
> Btw, the patch provided fails on an assertion with regression tests:
> 2426 /* Determine lock mode to use */
> 2427 lockmode = ExecUpdateLockMode(estate, relinfo);
> 2428
> -> 2429 Assert(HeapTupleIsValid(fdw_trigtuple) ^
> ItemPointerIsValid(tupleid));
> 2430 if (fdw_trigtuple == NULL)
>
> Peter, Andres, thoughts?
I agree with Stanislav that the behavior is wrong. The fix is more
complicated, though. As it says at the top of ExecUpdate():
* When updating a table, tupleid identifies the tuple to
* update and oldtuple is NULL. When updating a view, oldtuple
Since the ON CONFLICT DO UPDATE variant is always updating a table, it
is always supposed to pass oldtuple = NULL. The problem is that unlike
a regular update, it cannot reconstruct the original/target tuple for
an AFTER UPDATE trigger *correctly* -- the GetTupleForTrigger() logic
depends on executor state, which is not consistent with a regular
update.
I'll need to think about a fix.
--
Peter Geoghegan