FDW join pushdown and scanclauses
Hi,
In add_paths_to_joinrel(), the FDW specific hook GetForeignJoinPaths() is
called. This hook if implemented should add ForeignPaths for pushed down
joins. create_plan_recurse() calls create_scan_plan() on seeing these paths.
create_scan_plan() generates a list of clauses to be applied on scan from
rel->baserestrictinfo and parameterization clauses. This list is passed to
create_*scan_plan routine as scanclauses. This code is very specific for
single relations scans. Now that we are using create_scan_plan() for
creating plan for join relations, it needs some changes so that quals
relevant to a join can be passed to create_foreignscan_plan(). A related
problem is in create_foreignscan_plan(), which sets
ForeignScan::fsSystemCol if a system column is being used in the targetlist
or quals. Right now it only checks rel->baserestrictinfo, which is NULL for
a joinrel. Thus in case a system column appears in the joinclauses it will
not be considered.
Here are possible ways to fix the problem
1. Add joinrestrictinfo field in ForeignPath and use that as scan_clauses
in create_foreignscan_plan(). Something like patch attached. FDWs anyway
need to examine the join clauses to check whether they are pushable or not.
So mostly FDWs have already sorted the clauses (joinclauses and
otherclauses as well as pushable and nonpushable) and stored in their
private structures. So they probably don't need to look at the scan_clauses
passed in. (See WIP patch in [1]/messages/by-id/CAFjFpRdHgeNOhM0AB6Gxz1eVx_yOqkYwuKddZeB5vPzfBaeCnQ@mail.gmail.com -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company).
2. Create a separate ForeignJoinPath node with JoinPath as member (like
MergePath). This node then takes entirely different create_*_plan route,
ultimately producing a ForeignScan node. This path though clean will have a
lot of new and duplicate code.
Any other suggestions welcome.
[1]: /messages/by-id/CAFjFpRdHgeNOhM0AB6Gxz1eVx_yOqkYwuKddZeB5vPzfBaeCnQ@mail.gmail.com -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company
/messages/by-id/CAFjFpRdHgeNOhM0AB6Gxz1eVx_yOqkYwuKddZeB5vPzfBaeCnQ@mail.gmail.com
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
Attachments:
fdw_join_clauses.patchtext/x-patch; charset=US-ASCII; name=fdw_join_clauses.patchDownload+16-3
On 2016/01/08 22:05, Ashutosh Bapat wrote:
In add_paths_to_joinrel(), the FDW specific hook GetForeignJoinPaths()
is called. This hook if implemented should add ForeignPaths for pushed
down joins. create_plan_recurse() calls create_scan_plan() on seeing
these paths.
create_scan_plan() generates a list of clauses to be applied on scan
from rel->baserestrictinfo and parameterization clauses. This list is
passed to create_*scan_plan routine as scanclauses. This code is very
specific for single relations scans. Now that we are using
create_scan_plan() for creating plan for join relations, it needs some
changes so that quals relevant to a join can be passed to
create_foreignscan_plan().
Do we really need that? The relevant join quals are passed to
GetForeignJoinPaths as extra->restrictlist, so I think we can get those
quals during GetForeignPlan, by looking at the selected ForeignPath that
is passed to that function as a parameter.
A related problem is in
create_foreignscan_plan(), which sets ForeignScan::fsSystemCol if a
system column is being used in the targetlist or quals. Right now it
only checks rel->baserestrictinfo, which is NULL for a joinrel. Thus in
case a system column appears in the joinclauses it will not be considered.
IIUC, we assume that such system columns are assumed to be contained in
fdw_scan_tlist in the joinrel case.
Best regards,
Etsuro Fujita
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Jan 14, 2016 at 9:31 AM, Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>
wrote:
On 2016/01/08 22:05, Ashutosh Bapat wrote:
In add_paths_to_joinrel(), the FDW specific hook GetForeignJoinPaths()
is called. This hook if implemented should add ForeignPaths for pushed
down joins. create_plan_recurse() calls create_scan_plan() on seeing
these paths.create_scan_plan() generates a list of clauses to be applied on scan
from rel->baserestrictinfo and parameterization clauses. This list is
passed to create_*scan_plan routine as scanclauses. This code is very
specific for single relations scans. Now that we are using
create_scan_plan() for creating plan for join relations, it needs some
changes so that quals relevant to a join can be passed to
create_foreignscan_plan().Do we really need that? The relevant join quals are passed to
GetForeignJoinPaths as extra->restrictlist, so I think we can get those
quals during GetForeignPlan, by looking at the selected ForeignPath that is
passed to that function as a parameter.
Right! You are suggesting that an FDW should just ignore scan_clauses for
join relation and manage those by itself. That looks fine.
A related problem is in
create_foreignscan_plan(), which sets ForeignScan::fsSystemCol if a
system column is being used in the targetlist or quals. Right now it
only checks rel->baserestrictinfo, which is NULL for a joinrel. Thus in
case a system column appears in the joinclauses it will not be considered.IIUC, we assume that such system columns are assumed to be contained in
fdw_scan_tlist in the joinrel case.
From another mail thread [1]/messages/by-id/559F9323.4080403@lab.ntt.co.jp that you have started, I understood that
fsSystemCol should not be set for a join relation, so the bug doesn't exist.
[1]: /messages/by-id/559F9323.4080403@lab.ntt.co.jp
Thanks for your inputs.
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company