Robert Haas <robertmhaas@gmail.com> writes:
> On Thu, Apr 30, 2015 at 5:21 PM, Kouhei Kaigai <kaigai@ak.jp.nec.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. 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. The business about
resjunk columns in that list also seems a bit half baked, or at least
underdocumented.
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.
regards, tom lane