On Thu, Mar 22, 2018 at 3:01 PM, Amit Langote <langote_amit...@lab.ntt.co.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
