Re: Concurrency bug in UPDATE of partition-key
Amit Kapila <amit.kapila16@gmail.com>
From: Amit Kapila <amit.kapila16@gmail.com>
To: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Dilip Kumar <dilipbalaut@gmail.com>,
pgsql-hackers <pgsql-hackers@postgresql.org>, Amit Khandekar <amitdkhan.pg@gmail.com>
Date: 2018-07-11T03:26:23Z
Lists: pgsql-hackers
On Wed, Jul 11, 2018 at 5:59 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
> On 2018-Jul-09, Amit Kapila wrote:
>
>> Alvaro,
>>
>> Can you please comment whether this addresses your concern?
>
> I was thinking that it would be a matter of passing the tuple slot to
> EvalPlanQual for it to fill, rather than requiring it to fill some other
> slot obtained from who-knows-where, but I realize now that that's nigh
> impossible.
>
Right, giving EvalPlanQual the responsibility to use the output slot
can easily turn into a huge work without much benefit.
> Thanks for the explanation and patience.
>
> What bothers me about this whole business is how ExecBRDeleteTriggers
> and ExecDelete are now completely different from their sibling routines,
> but maybe there's no helping that.
>
Yeah, I think the differences started appearing when we decide to
overload ExecDelete for the usage of update-partition key, however,
the alternative would have been to write a new equivalent function
which can create a lot of code duplication.
> Please move the output arguments at the end of argument lists;
make sense.
> also, it
> would be great if you add commentary about ExecDelete other undocumented
> arguments (tupleDeleted in particular) while you're in the vicinity.
>
We already have some commentary in the caller of ExecDelete ("For some
reason if DELETE didn't happen ..."), but I think it will be clear if
we can add some comments atop function ExecDelete. I will send the
updated patch shortly.
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
Commits
-
Allow using the updated tuple while moving it to a different partition.
- 40ca70ebcc9d 12.0 landed
- 0527df732beb 11.0 landed