Re: partition routing layering in nodeModifyTable.c

[Amit Langote]

Fujita-san,

On Wed, Aug 7, 2019 at 6:00 PM Etsuro Fujita <etsuro.fuj...@gmail.com> wrote:
> On Wed, Aug 7, 2019 at 4:28 PM Amit Langote <amitlangot...@gmail.com> wrote:
> > I just noticed obsolete references to es_result_relation_info that
> > 0002 failed to remove.  One of them is in fdwhandler.sgml:
> >
> > <programlisting>
> > TupleTableSlot *
> > IterateDirectModify(ForeignScanState *node);
> > </programlisting>
> >
> >     ... The data that was actually inserted, updated
> >      or deleted must be stored in the
> >      
> > <literal>es_result_relation_info-&gt;ri_projectReturning-&gt;pi_exprContext-&gt;ecxt_scantuple</literal>
> >      of the node's <structname>EState</structname>.
> >
> > We will need to rewrite this without mentioning
> > es_result_relation_info.  How about as follows:
> >
> > -     
> > <literal>es_result_relation_info-&gt;ri_projectReturning-&gt;pi_exprContext-&gt;ecxt_scantuple</literal>
> > -     of the node's <structname>EState</structname>.
> > +     
> > <literal>ri_projectReturning-&gt;pi_exprContext-&gt;ecxt_scantuple</literal>
> > +     of the result relation's<structname>ResultRelInfo</structname> that 
> > has
> > +     been made available via node.
> >
> > I've updated 0001 with the above change.
>
> Good catch!

Thanks for the review.

> This would be nitpicking, but:
>
> * IIUC, we don't use the term "result relation" in fdwhandler.sgml.
> For consistency with your change to the doc for BeginDirectModify, how
> about using the term "target foreign table" instead of "result
> relation"?

Agreed, done.

> * ISTM that "<structname>ResultRelInfo</structname> that has been made
> available via node" would be a bit fuzzy to FDW authors.  To be more
> specific, how about changing it to
> "<structname>ResultRelInfo</structname> passed to
> <function>BeginDirectModify</function>" or something like that?

That works for me, although an FDW author reading this still has got
to make the connection.

Attached updated patches; only 0001 changed in this version.

Thanks,
Amit

v7-0001-Revise-BeginDirectModify-API-to-pass-ResultRelInf.patch
Description: Binary data

v7-0002-Remove-es_result_relation_info.patch
Description: Binary data

v7-0004-Refactor-transition-tuple-capture-code-a-bit.patch
Description: Binary data

v7-0003-Rearrange-partition-update-row-movement-code-a-bi.patch
Description: Binary data