Postgres Pro
Downloads
Home   >   mailing lists

Thread: FDW join pushdown and scanclauses

FDW join pushdown and scanclauses

From
Ashutosh Bapat
Date:
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]).

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] http://www.postgresql.org/message-id/CAFjFpRdHgeNOhM0AB6Gxz1eVx_yOqkYwuKddZeB5vPzfBaeCnQ@mail.gmail.com
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
Attachment

Re: FDW join pushdown and scanclauses

From
Etsuro Fujita
Date:
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

Re: FDW join pushdown and scanclauses

From
Ashutosh Bapat
Date:


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] that you have started, I understood that fsSystemCol should not be set for a join relation, so the bug doesn't exist.

[1] http://www.postgresql.org/message-id/559F9323.4080403@lab.ntt.co.jp

Thanks for your inputs.
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company