Michael Fuhr <mike@fuhr.org> writes:
> Thanks. Alvaro made the following suggestion but didn't copy the
> list -- shall I do what he suggested and submit the changes as
> another patch?
> Alvaro Herrera wrote:
>> I'd add an Assert() on the second hunk to make sure newtuple is only set
>> in UPDATE. And also a comment on top of the "if" to explain why.
Can't get excited about that. Will you also have asserts to complain
if the wrong combinations of tuples are supplied for the other cases?
Is this really likely to catch anything? It's not like this function
is called from a variety of places.
While I was applying the patch I considered changing the
if (LocTriggerData.tg_trigtuple != NULL)
to
if ((event->ate_event & TRIGGER_EVENT_OPMASK) == TRIGGER_EVENT_UPDATE)
but this didn't seem to be an improvement on the whole, as it effectively
provides two ways to get it wrong (wrong tuple args OR wrong event)
instead of only one. I think driving the setup of the tuple fields
entirely off the provided tuple args is logically cleaner.
regards, tom lane