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

On Thu, Feb 18, 2016 at 3:48 PM, Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>
wrote:

> 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(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp <mailto:fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)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.
>

If the list in the joining relation changes (may be because we appended
parameterized conditions), we would be breaking links on all the upper
relations in the join tree in an unpredictable manner. The problem may not
show up now, but it's an avenue for unrecognizable bugs. So, it's safer to
copy the lists in the state that we want them.

--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company