Re: [HACKERS] Useless code in ExecInitModifyTable

Fujita-san, Amit,

* Amit Langote (Langote_Amit_f8@lab.ntt.co.jp) wrote:
> On 2017/06/21 16:59, Etsuro Fujita wrote:
> > Commit d3cc37f1d801a6b5cad9bf179274a8d767f1ee50 added this to
> > ExecInitModifyTable:
> > 
> > +   /* The root table RT index is at the head of the partitioned_rels list */
> > +   if (node->partitioned_rels)
> > +   {
> > +       Index   root_rti;
> > +       Oid     root_oid;
> > +
> > +       root_rti = linitial_int(node->partitioned_rels);
> > +       root_oid = getrelid(root_rti, estate->es_range_table);
> > +       rel = heap_open(root_oid, NoLock);  /* locked by InitPlan */
> > +   }
> > 
> > but I noticed that that function doesn't use the relation descriptor at
> > all.  Since partitioned_rels is given in case of an UPDATE/DELETE on a
> > partitioned table, the relation is opened in that case, but the relation
> > descriptor isn't referenced at all in initializing WITH CHECK OPTION
> > constraints and/or RETURNING projections.  (The mtstate->resultRelinfo
> > array is referenced in those initialization, instead.)  So, I'd like to
> > propose to remove this from that function.  Attached is a patch for that.
> 
> Thanks for cleaning that up.  I cannot see any problem in applying the patch.

Seems like this has gotten a review (and quite a bit of down-stream
discussion that seemed pretty positive), and the latest patch still
applies cleanly and passes the regression tests- is there some reason
it's still marked as Needs Review?  Seems like it should really be in
Ready for Committer state.

Amit, if you agree, would you mind going ahead and changing it?

Thanks!

Stephen