On 2015/12/23 2:47, Robert Haas wrote:
> On Tue, Dec 22, 2015 at 7:32 AM, Michael Paquier
> <michael.paquier@gmail.com> wrote:
>> Moved to next CF because of a lack of reviews.
Thanks, Michael!
> I just took a look at this. I think the basic idea of this patch is
> good, but the comments need some work, because they don't really
> explain why this should be skipped in the join case. Maybe something
> like this:
Thanks for the review, Robert!
> If rel is a base relation, detect whether any system columns were
> requested. (If rel is a join relation, rel->relid will be 0, but
> there can be no Var in the target list with relid 0, so we skip this
> in that case.) This is a bit of a kluge and might go away someday, so
> we intentionally leave it out of the API presented to FDWs.
> And the rest as it is currently written.
Agreed.
> It might be good, also, to say something about why we never need
> fsSystemCol to be true in the joinrel case.
+1 for that. How about adding something like this:
Note that any such system columns are assumed to be contained in
fdw_scan_tlist, so we never need fsSystemCol to be true in the joinrel case.
Attached is an updated version of the patch.
Best regards,
Etsuro Fujita
