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
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 |