Re: allow trigger to get updated columns

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Peter Eisentraut <peter(at)eisentraut(dot)org>
Cc: Daniel Gustafsson <daniel(at)yesql(dot)se>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: allow trigger to get updated columns
Date: 2025-01-21 23:19:51
Message-ID: 3496294.1737501591@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com> writes:
> On 2020-03-05 13:53, Daniel Gustafsson wrote:
>> +1 on the patchset, marking this entry as Ready For Committer.

> and done

While looking at a pending patch, I happened to notice that commit
71d60e2aa added a field ats_modifiedcols to AfterTriggerSharedData,
but did not change the logic in afterTriggerAddEvent that decides
whether the new event's evtshared matches some existing one.
Thus, we could seize on a pre-existing entry that matches in
everything except ats_modifiedcols, resulting in ultimately
firing the trigger with the wrong modifiedcols information.

If this is not in fact completely broken, the reason must be that
ats_modifiedcols will always be the same for the same values of
ats_tgoid/ats_relid/ats_event/ats_table (within a given transaction).
That seems unlikely, although if it were true we could perhaps put
the data somewhere else instead of bloating AfterTriggerSharedData.

The obvious fix would involve adding a bms_equal() call to the
comparison loop in afterTriggerAddEvent, which makes me quite
sad on performance grounds. Maybe it hardly matters in the
big scheme of things, though.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jim Jones 2025-01-21 23:35:56 Re: XMLDocument (SQL/XML X030)
Previous Message Daniel Gustafsson 2025-01-21 22:57:55 Re: Replace current implementations in crypt() and gen_salt() to OpenSSL