Re: postgres_fdw join pushdown (was Re: Custom/Foreign-Join-APIs)

On 2016/02/16 16:40, Etsuro Fujita wrote:
> On 2016/02/16 16:02, Ashutosh Bapat wrote:
>> On Tue, Feb 16, 2016 at 12:26 PM, Etsuro Fujita
>> <fujita.etsuro@lab.ntt.co.jp <mailto:fujita.etsuro@lab.ntt.co.jp>> wrote:

>>     On 2016/02/16 15:22, Ashutosh Bapat wrote:

>>         During join planning, the planner tries multiple combinations of
>>         joining
>>         relations, thus the same base or join relation can be part of
>>         multiple
>>         of combination. Hence remote_conds or joinclauses will get linked
>>         multiple times as they are bidirectional lists, thus breaking
>>         linkages
>>         of previous join combinations tried. E.g. while planning A join
>>         B join C
>>         join D planner will come up with combinations like A(B(CD)) or
>>         (AB)(CD)
>>         or ((AB)C)D etc. and remote_conds from A will first be linked
>> into
>>         A(B(CD)), then AB breaking the first linkages.

>>     Exactly, but I don't think that that needs to be considered because
>>     we have this at the beginning of postgresGetGForeignJoinPaths:
>>
>>          /*
>>           * Skip if this join combination has been considered already.
>>           */
>>          if (joinrel->fdw_private)
>>              return;

>> There will be different joinrels for A(B(CD)) and (AB) where A's
>> remote_conds need to be pulled up.

> Agreed.

>> The check you have mentioned above
>> only protects us from adding paths multiple times to (AB) when we
>> encounter it for (AB)(CD) and ((AB)C)D.

> Sorry, I don't understand this fully.

Another thing I don't really understand is why list_copy is needed in 
the second list_concat for the case of INNER/FULL JOIN or in both 
list_concats for the case of LEFT/RIGHT JOIN, in your patch.  Since 
list_concat is nondestructive of its second argument, I don't think 
list_copy is needed in any such list_concat.  Maybe I'm missing 
something, though.

Best regards,
Etsuro Fujita