|From:||Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>|
|To:||Robert Haas <robertmhaas(at)gmail(dot)com>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>|
|Cc:||PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>|
|Subject:||Re: Minor code improvements to create_foreignscan_plan/ExecInitForeignScan|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
On 2015/12/23 2:47, Robert Haas wrote:
> On Tue, Dec 22, 2015 at 7:32 AM, Michael Paquier
> <michael(dot)paquier(at)gmail(dot)com> wrote:
>> Moved to next CF because of a lack of reviews.
> 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.
> 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.
|Next Message||Feng Tian||2015-12-28 07:01:08||Re: Remove Windows crash dump support?|
|Previous Message||Amit Kapila||2015-12-28 06:22:16||Re: Remove Windows crash dump support?|