Re: Concurrency bug in UPDATE of partition-key

On 30 June 2018 at 19:20, Amit Kapila <amit.kapila16@gmail.com> wrote:
> On Fri, Jun 29, 2018 at 11:22 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
>> I was a bit surprised by the new epqslot output argument being added,
>> and now I think I know why: we already have es_trig_tuple_slot, so
>> shouldn't we be using that here instead?  Seems like it'd all be simpler ...

es_trig_tuple_slot is already allocated in ExecInitModifyTable(). And
the slot returned by EvalPlanQual is a separately allocated tuple
slot. I didn't get how we can assign the epqslot to
estate->es_trig_tuple_slot. That would mean throwing away the already
allocated es_trig_tuple_slot. I believe, the es_trig_tuple_slot
variable is not used for assigning already allocated slots to it.

>>
>
> Hmm, maybe, but not sure if it will be simpler.  The point is that we
> don't need to always return the epqslot, it will only be returned for
> the special case, so you might need to use an additional boolean
> variable to indicate when to fill the epqslot or someway indicate the
> same via es_trig_tuple_slot.  I think even if we somehow do that, we
> need to do something additional like taking tuple from epqslot and
> store it in es_trig_tuple_slot as I am not sure if we can directly
> assign the slot returned by EvalPlanQual to es_trig_tuple_slot.

Right, I think making use of es_trig_tuple_slot will cause redundancy
in our case, because epqslot is a separately allocated slot; so it
makes sense to pass it back separately.

> OTOH, the approach used by Amit Khandekar seems somewhat better as you
> can directly return the slot returned by EvalPlanQual in the output
> parameter.  IIUC, the same technique is already used by
> GetTupleForTrigger where it returns the epqslot in an additional
> parameter.
>



-- 
Thanks,
-Amit Khandekar
EnterpriseDB Corporation
The Postgres Database Company