Re: ON CONFLICT DO UPDATE for partitioned tables

On Thu, Mar 22, 2018 at 3:01 PM, Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp
> wrote:

>
> >
> > Why do we need to pin the descriptor? If we do need, why don't we need
> > corresponding ReleaseTupDesc() call?
>
> PinTupleDesc was added in the patch as Alvaro had submitted it upthread,
> which it wasn't clear to me either why it was needed. Looking at it
> closely, it seems we can get rid of it if for the lack of corresponding
> ReleaseTupleDesc(). ExecSetSlotDescriptor() seems to take care of pinning
> and releasing tuple descriptors that are passed to it. If some
> partition's tupDesc remains pinned because it was the last one that was
> passed to it, the final ExecResetTupleTable will take care of releasing it.
>
> I have removed the instances of PinTupleDesc in the updated patch, but I'm
> not sure why the loose PinTupleDesc() in the previous version of the patch
> didn't cause reference leak warnings or some such.
>

Yeah, it wasn't clear to me as well. But I did not investigate. May be
Alvaro knows better.

> > I am still confused if the partition_conflproj_tdescs really belongs to
> > PartitionTupleRouting or should it be a part of the ResultRelInfo. FWIW
> for
> > the
> > MERGE patch, I moved everything to a new struct and made it part of the
> > ResultRelInfo. If no re-mapping is necessary, we can just point to the
> > struct
> > in the root relation's ResultRelInfo. Otherwise create and populate a new
> > one
> > in the partition relation's ResultRelInfo.
> >
> > + leaf_part_rri->ri_onConflictSetWhere =
> > + ExecInitQual(onconflwhere, &mtstate->ps);
> > + }
> >
> > So ri_onConflictSetWhere and ri_onConflictSetProj are part of the
> > ResultRelInfo. Why not move mt_conflproj_tupdesc,
> > partition_conflproj_tdescs,
> > partition_arbiter_indexes etc to the ResultRelInfo as well?
>
> I have moved both the projection tupdesc and the arbiter indexes list into
> ResultRelInfo as I wrote above.
>
>
Thanks. It's looking much better now. I think we can possibly move all ON
CONFLICT related members to a separate structure and just copy the pointer
to the structure if (map == NULL). That might make the code a bit more tidy.

Is there anything that needs to be done for transition tables? I checked
and didn't see anything, but please check.

Thanks,
Pavan

--
Pavan Deolasee http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services