Hi, On Thu, Aug 8, 2019 at 10:10 AM Amit Langote <amitlangot...@gmail.com> wrote: > 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->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.
> > 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 for the updated version, Amit-san! I updated the 0001 patch a bit further: * Tweaked comments in plannodes.h, createplan.c, and nodeForeignscan.c. * Made cosmetic changes to postgres_fdw.c. * Adjusted doc changes a bit, mainly not to produce unnecessary diff. * Modified the commit message. Attached is an updated version of the 0001 patch. Does that make sense? Best regards, Etsuro Fujita
v7-0001-Remove-dependency-on-estate-es_result_relation_info-efujita.patch
Description: Binary data
