On Thu, Jan 5, 2017 at 5:14 PM, Etsuro Fujita
<fujita.etsuro@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