Re: Custom/Foreign-Join-APIs (Re: [v9.5] Custom Plan API)

> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> > On Thu, Apr 30, 2015 at 5:21 PM, Kouhei Kaigai <kaigai(at)ak(dot)jp(dot)nec(dot)com> wrote:
> >> I wanted to submit the v14 after the above items get clarified.
> >> The attached patch (v14) includes all what you suggested in the previous
> >> message.
>
> > Committed, after heavily working over the documentation, and with some
> > more revisions to the comments as well.
>
> I've been trying to code-review this patch, because the documentation
> seemed several bricks shy of a load, and I find myself entirely confused
> by the fdw_ps_tlist and custom_ps_tlist fields. The names, along with
> some of the comments, imply that these are just targetlists for the join
> nodes; but if that is the case then we don't need them, because surely
> scan.targetlist would serve the purpose. There is some other, utterly
> uncommented, code in setrefs.c and ruleutils.c that suggests these fields
> are supposed to serve a purpose more like IndexOnlyScan.indextlist; but
> if that's what they are the comments are woefully inadequate/misleading,
> and I'm really unsure that the associated code actually works.
>
Main-point of your concern is lack of documentation/comments to introduce
how does the pseudo-scan targetlist works here, isn't it??

> Also,
> if that is what they're for (ie, to allow the FDW to redefine the scan
> tuple contents) it would likely be better to decouple that feature from
> whether the plan node is for a simple scan or a join.
>
In this version, we don't intend FDW/CSP to redefine the contents of
scan tuples, even though I want off-loads heavy targetlist calculation
workloads to external computing resources in *the future version*.

> The business about
> resjunk columns in that list also seems a bit half baked, or at least
> underdocumented.
>
I'll add source code comments to introduce how does it works any when
does it have resjunk=true. It will be a bit too deep to be introduced
in the SGML file.

> I do not think that this should have gotten committed without an attendant
> proof-of-concept patch to postgres_fdw, so that the logic could be tested.
>
Hanada-san is now working according to the comments from Robert.
Overall design was already discussed in the upthread and the latest
implementation follows the people's consensus.

Thanks,
--
NEC Business Creation Division / PG-Strom Project
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>