Re: Push down more full joins in postgres_fdw

From: Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>
To: Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Push down more full joins in postgres_fdw
Date: 2017-01-05 12:11:26
Message-ID: CAFjFpRe=-iob09OHGCWo8k=LCTEWE-PSXzznJF7USVMvK7ZzTw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Jan 5, 2017 at 5:14 PM, Etsuro Fujita
<fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Ashutosh Sharma 2017-01-05 12:13:50 Re: pageinspect: Hash index support
Previous Message Amit Kapila 2017-01-05 11:51:00 Re: UNDO and in-place update