Re: partition routing layering in nodeModifyTable.c

Amit-san,

On Wed, Aug 7, 2019 at 4:28 PM Amit Langote <amitlangote09@gmail.com> wrote:
> On Wed, Aug 7, 2019 at 12:00 PM Etsuro Fujita <etsuro.fujita@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