Re: Concurrency bug in UPDATE of partition-key

On Mon, Jun 25, 2018 at 3:06 PM, Amit Khandekar <amitdkhan(dot)pg(at)gmail(dot)com> wrote:
> On 23 June 2018 at 15:46, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>>>
>>
>> Why do you need to update when newslot is NULL? Already *epqslot is
>> initialized as NULL in the caller (ExecUpdate). I think we only want
>> to update it when trigtuple is not null.
>
> But GetTupleForTrigger() updates the epqslot to NULL even when it
> returns NULL. So to be consistent with it, we want to do the same
> thing for ExecBRDeleteTriggers() : Update the epqslot even when
> GetTupleForTrigger() returns NULL.
>
> I think the reason we are doing "*newSlot = NULL" as the very first
> thing in the GetTupleForTrigger() code, is so that callers don't have
> to initialise newSlot to NULL before calling GetTupleForTrigger. And
> so ExecBRUpdateTriggers() does not initialize newSlot with NULL. We
> can follow the same approach while calling ExecDelete().
>

Yeah, we can do that if it is required. I see your point, but I feel
that is making code bit less readable.

> I understand that before calling ExecDelete() epqslot is initialized
> to NULL, so it is again not required to do the same inside
> GetTupleForTrigger(), but as per my above explanation, it is actually
> not necessary to initialize to NULL before calling ExecDelete(). So I
> can even remove that initialization.
>

If you remove that initialization, then won't you need to do something
in ExecDelete() as well because there the patch assigns a value to
epqslot only if EvalPlanQual returns non-null value?

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com