Amit-san, On Wed, Aug 7, 2019 at 4:28 PM Amit Langote <amitlangot...@gmail.com> wrote: > On Wed, Aug 7, 2019 at 12:00 PM Etsuro Fujita <etsuro.fuj...@gmail.com> wrote: > > IIUC, I think we reached a consensus at least on the 0001 patch. > > Andres, would you mind if I commit that patch? > > 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->ri_projectReturning->pi_exprContext->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->ri_projectReturning->pi_exprContext->ecxt_scantuple</literal> > - of the node's <structname>EState</structname>. > + > <literal>ri_projectReturning->pi_exprContext->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! 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"? * 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? Best regards, Etsuro Fujita