Re: Push down more full joins in postgres_fdw

From: Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>
To: Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Push down more full joins in postgres_fdw
Date: 2016-11-22 06:55:14
Message-ID: 63480889-acdb-c080-ec3a-bb9be140db2b@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

On 2016/11/21 22:02, Ashutosh Bapat wrote:

You wrote:
>>>> Instead we should calculate tlist, the
>>>> first time we need it and then add it to the fpinfo.

I wrote:
>> Having said that, I agree on that point. I'd like to propose (1) adding a
>> new member to fpinfo, to store a list of output Vars from the subquery, and
>> (2) creating a tlist from it in deparseRangeTblRef, then, which would allow
>> us to calculate the tlist only when we need it. The member added to fpinfo
>> would be also useful to address the comments on the DML/UPDATE pushdown
>> patch. See the attached patch in [1]. I named the member a bit differently
>> in that patch, though.

> Again the list of Vars will be wasted if we don't choose that path for
> final planning. So, I don't see the point of adding list of Vars
> there.

> If you think that the member is useful for DML/UDPATE pushdown,
> you may want to add it in the other patch.

OK, I'd like to propose referencing to foreignrel->reltarget->exprs
directly in deparseRangeTblRef and get_subselect_alias_id, then, which
is the same as what I proposed in the first version of the patch, but
I'd also like to change deparseRangeTblRef to use add_to_flat_tlist, not
make_tlist_from_pathtarget, to create a tlist of the subquery, as you
proposed.

>> You modified the comments I added to deparseLockingClause into this:
>>
>> /*
>> + * Ignore relation if it appears in a lower subquery. Locking clause
>> + * for such a relation is included in the subquery.
>> + */
>>
>> I don't think the second sentence is 100% correct because a locking clause
>> isn't always required for such a relation, so I modified the sentence a bit.

> I guess, "if required" part was implicit in construct "such a
> relation". Your version seems to make it explicit. I am fine with it.

OK, let's leave that for the committer's judge.

Please find attached an updated version of the patch.

Best regards,
Etsuro Fujita

Attachment Content-Type Size
postgres-fdw-subquery-support-v10.patch application/x-patch 28.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tsunakawa, Takayuki 2016-11-22 06:56:29 Re: [RFC] Should we fix postmaster to avoid slow shutdown?
Previous Message Craig Ringer 2016-11-22 06:49:46 Re: Logical decoding on standby