On Thu, Jan 5, 2017 at 5:14 PM, Etsuro Fujita <fujita.ets...@lab.ntt.co.jp> wrote: > On 2017/01/03 17:28, Ashutosh Bapat wrote: > > I wrote: >>> >>> I updated the patch a bit further: simplified the function name >>> (s/build_subquery_rel_tlists/build_subquery_tlists/), and revised >>> comments a >>> little bit. Attached is an updated version >>> (postgres-fdw-subquery-support-v14.patch). > > >> Few comments > > > Thanks for the comments! > >> In build_subquery_tlists(), why don't we handle base relations? >> + if (foreignrel->reloptkind != RELOPT_JOINREL) >> + return; > > > The reason for that is we don't need to handle the baserel cases; the tlist > for a base relation, if needed, would be created while recursing into a join > relation that joins the base relation to other base/join relation.
Right. Sorry, I misunderstood the code. May be a comment would help. > >> Also, in this function, if fpinfo->tlist is already set, why do we want to >> build it again? > > > When this function gets called, fpinfo->tlist isn't set for any base or join > relation that needs to build the tlist, so we always need to build it for > each such relation. IIUC, for a relation with use_remote_estimates we will deparse the query twice and will build the targetlist twice. > >> In build_tlist_to_deparse(), if fpinfo->tlist for the given relation is >> set, we >> should just return it rather than constructing it again. > > > In that function we wouldn't have such cases for base or join relations > needing the tlist. Same explanation as above. -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
