From: | Amit Langote <amitlangote09(at)gmail(dot)com> |
---|---|
To: | Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com> |
Cc: | Andres Freund <andres(at)anarazel(dot)de>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, amitdkhan(dot)pg(at)gmail(dot)com |
Subject: | Re: partition routing layering in nodeModifyTable.c |
Date: | 2019-08-08 01:10:11 |
Message-ID: | CA+HiwqGs6BfypYdZxrbDxf02wptxkG7h1GbfcGonQzbtB8iDNA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Fujita-san,
On Wed, Aug 7, 2019 at 6:00 PM Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com> wrote:
> On Wed, Aug 7, 2019 at 4:28 PM Amit Langote <amitlangote09(at)gmail(dot)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.
>
> 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
Attachment | Content-Type | Size |
---|---|---|
v7-0001-Revise-BeginDirectModify-API-to-pass-ResultRelInf.patch | application/octet-stream | 11.7 KB |
v7-0002-Remove-es_result_relation_info.patch | application/octet-stream | 37.9 KB |
v7-0004-Refactor-transition-tuple-capture-code-a-bit.patch | application/octet-stream | 20.8 KB |
v7-0003-Rearrange-partition-update-row-movement-code-a-bi.patch | application/octet-stream | 16.0 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Justin Pryzby | 2019-08-08 01:24:36 | Re: crash 11.5~ (and 11.4) |
Previous Message | Amit Langote | 2019-08-08 01:01:38 | Re: no default hash partition |