Peter Eisentraut <peter.eisentr...@2ndquadrant.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
