Push down join with PHVs (WAS: Re: [sqlsmith] Failed assertion in postgres_fdw/deparse.c:1116)

On 2016/06/17 21:45, Robert Haas wrote:
> On Thu, Jun 16, 2016 at 10:44 PM, Etsuro Fujita
> <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>> On 2016/06/16 22:00, Robert Haas wrote:
>>> On Thu, Jun 16, 2016 at 7:05 AM, Etsuro Fujita
>>> <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>>>>
>>>> ISTM that a robuster solution to this is to push down the ft1-ft2-ft3
>>>> join
>>>> with the PHV by extending deparseExplicitTargetList() and/or something
>>>> else
>>>> so that we can send the remote query like:
>>>>
>>>> select ft1.a, (ft3.a IS NOT NULL) from (ft1 inner join ft2 on ft1.a =
>>>> ft2.a)
>>>> left join ft3 on ft1.a = ft3.a

>>> I completely agree we should have that, but not for 9.6.

>> OK, I'll work on that for 10.0.

> Great, that will be a nice improvement.

Here is a patch for that.

* Modified deparseExplicitTargetList as well as is_foreign_expr and
deparseExpr so that a join is pushed down with PHVs if not only the join
but the PHVs are safe to execute remotely. The existing deparsing logic
can't build a remote query that involves subqueries, so the patch
currently gives up pushing down any upper joins involving a join (or
foreign table!) with PHVs in the tlist. But I think the patch would be
easily extended to support that, once we support deparsing subqueries.

* Besides that, simplified foreign_join_ok a bit, by adding a new flag,
allow_upper_foreign_join, to PgFdwRelationInfo, replacing the existing
pushdown_safe flag with it. See the comments in postgres_fdw.h.

Comments welcome.

Best regards,
Etsuro Fujita