Re: postgres_fdw join pushdown (was Re: Custom/Foreign-Join-APIs) - Mailing list pgsql-hackers

From Ashutosh Bapat
Subject Re: postgres_fdw join pushdown (was Re: Custom/Foreign-Join-APIs)
Date
Msg-id CAFjFpRdaX85B1qPFuLhrwdVsm5Mp=d8nbt+0SjHcA9jcDWe8GQ@mail.gmail.com
Whole thread Raw
In response to Re: postgres_fdw join pushdown (was Re: Custom/Foreign-Join-APIs)  (Robert Haas <robertmhaas@gmail.com>)
Responses Re: postgres_fdw join pushdown (was Re: Custom/Foreign-Join-APIs)
List pgsql-hackers



Would it be nuts to set fdw_scan_tlist in all cases?  Would the code
come out simpler than what we have now?


PFA the patch that can be applied on v9 patches.

If there is a whole-row reference for base relation, instead of adding that as an additional column deparseTargetList() creates a list of all the attributes of that foreign table . The whole-row reference gets constructed during projection. This saves some network bandwidth while transferring the data from the foreign server. If we build the target list for base relation, we should include Vars for all the columns (like deparseTargetList). Thus we borrow some code from deparseTargetList to get all the attributes of a relation. I included this logic in function build_tlist_from_attrs_used(), which is being called by build_tlist_to_deparse(). So, before calling deparseSelectStmtForRel() the callers can just call build_tlist_to_deparse() and pass the targetlist to deparseSelectStmtForRel() and use the same to be handed over to the core code as fdw_scan_tlist. build_tlist_to_deparse() also constructs retrieved_attrs list, so that doesn't need to be passed around in deparse routines.

But we now have similar code in three places deparseTargetList(), deparseAnalyzeSql() and build_tlist_from_attrs_used(). So, I re-wrote deparseTargetList() (which is now used to deparse returning list) to call build_tlist_from_attrs_used() followed by deparseExplicitTargetList(). The later and its minion deparseVar requires a deparse_expr_cxt to be passed. deparse_expr_cxt has a member to store RelOptInfo of the relation being deparsed. The callers of deparseReturningList() do not have it and thus deparse_expr_cxt required changes, which in turn required changes in other places. All in all, a larger refactoring. It touches more places than necessary for foreign join push down. So, I think, we should try that as a separate refactoring work. We may just do the work described in the first paragraph for join pushdown, but we will then be left with duplicate code, which I don't think is worth the output.

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

pgsql-hackers by date:

Previous
From: Robert Haas
Date:
Subject: Re: Comment typo in slot.c
Next
From: Filip Rembiałkowski
Date:
Subject: proposal: make NOTIFY list de-duplication optional