On 11 June 2018 at 15:29, Amit Kapila <amit.kapil...@gmail.com> wrote: > On Mon, Jun 11, 2018 at 3:02 PM, Amit Kapila <amit.kapil...@gmail.com> wrote: >> On Thu, Jun 7, 2018 at 1:53 PM, Amit Khandekar <amitdkhan...@gmail.com> >> wrote: >>> On 7 June 2018 at 11:44, Amit Kapila <amit.kapil...@gmail.com> wrote: >>>> On Tue, Jun 5, 2018 at 8:03 PM, Amit Khandekar <amitdkhan...@gmail.com> >>>> wrote: >>>> >>>> I think this will allow before row delete triggers to be fired more than >>>> once. Normally, if the EvalPlanQual testing generates a new tuple, we >>>> don't >>>> fire the triggers again. >>> >>> If there are BR delete triggers, the tuple will be locked using >>> GetTupleForTrigger(). So the subsequent EvalPlanQual testing won't be >>> run, since the tuple is already locked due to triggers having run. >>> >>> But that leads me to think : The same concurrency issue can occur in >>> GetTupleForTrigger() also. Say, a concurrent session has already >>> locked the tuple, and GetTupleForTrigger() would wait and then return >>> the updated tuple in its last parameter newSlot. In that case, we need >>> to pass this slot back through ExecBRDeleteTriggers(), and further >>> through epqslot parameter of ExecDelete(). But yes, in this case, we >>> should avoid calling this trigger function the second time. >>> >>> If you agree on the above, I will send an updated patch. >>> >> >> Sounds reasonable to me.
Attached is v2 version of the patch. It contains the above trigger-related issue fixed. The updated tuple is passed back using the existing newslot parameter of GetTupleForTrigger(). When ExecBRDeleteTriggers() is called using a new epqslot parameter, it means caller wants to skip the trigger execution, because the updated tuple needs to be again checked for constraints. I have added comments of this behaviour in the ExecBRDeleteTriggers() function header. Because the trigger execution is skipped the first time ExecDelete() is called, I didn't have to explicitly add any changes for preventing trigger execution the second time. > Try to add a test case which covers BR trigger code path where you are > planning to update. Added the test cases. Also added cases where the concurrent session causes the EvalPlanQual() test to fail, so the partition key update does not happen. -- Thanks, -Amit Khandekar EnterpriseDB Corporation The Postgres Database Company
fix_concurrency_bug_v2.patch
Description: Binary data
