On Fri, Jun 2, 2017 at 4:37 PM, Amit Khandekar <amitdkhan.pg@gmail.com> wrote:
> On 2 June 2017 at 01:17, Robert Haas <robertmhaas@gmail.com> wrote:
>> On Thu, Jun 1, 2017 at 7:41 AM, Amit Khandekar <amitdkhan.pg@gmail.com> wrote:
>>>> Regarding the trigger issue, I can't claim to have a terribly strong
>>>> opinion on this. I think that practically anything we do here might
>>>> upset somebody, but probably any halfway-reasonable thing we choose to
>>>> do will be OK for most people. However, there seems to be a
>>>> discrepancy between the approach that got the most votes and the one
>>>> that is implemented by the v8 patch, so that seems like something to
>>>> fix.
>>>
>>> Yes, I have started working on updating the patch to use that approach
>>> (BR and AR update triggers on source and destination partition
>>> respectively, instead of delete+insert) The approach taken by the
>>> patch (BR update + delete+insert triggers) didn't require any changes
>>> in the way ExecDelete() and ExecInsert() were called. Now we would
>>> require to skip the delete/insert triggers, so some flags need to be
>>> passed to these functions,
>>>
I thought you already need to pass an additional flag for special
handling of ctid in Delete case. For Insert, a new flag needs to be
passed and need to have a check for that in few places.
> or else have stripped down versions of
>>> ExecDelete() and ExecInsert() which don't do other things like
>>> RETURNING handling and firing triggers.
>>
>> See, that strikes me as a pretty good argument for firing the
>> DELETE+INSERT triggers...
>>
>> I'm not wedded to that approach, but "what makes the code simplest?"
>> is not a bad tiebreak, other things being equal.
>
> Yes, that sounds good to me.
>
I am okay if we want to go ahead with firing BR UPDATE + DELETE +
INSERT triggers for an Update statement (when row movement happens) on
the argument of code simplicity, but it sounds slightly odd behavior.
> But I think we want to wait for other's
> opinion because it is quite understandable that two triggers firing on
> the same partition sounds odd.
>
Yeah, but I think we have to rely on docs in this case as behavior is
not intuitive.
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com