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_yoqkywukddzeb5vpzfbae...@mail.gmail.com
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 953aa62..bb1c058 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -327,22 +327,25 @@ create_scan_plan(PlannerInfo *root, Path *best_path)
}
}
else
{
tlist = build_path_tlist(root, best_path);
}
/*
* Extract the relevant restriction clauses from the parent relation. The
* executor must apply all these restrictions during the scan, except for
- * pseudoconstants which we'll take care of below.
+ * pseudoconstants which we'll take care of below. We may get here for a
+ * pushed down join between foreign relations. baserestrictinfo would be
+ * NULL in that case.
*/
+ Assert(rel->reloptkind != RELOPT_JOINREL || !rel->baserestrictinfo);
scan_clauses = rel->baserestrictinfo;
/*
* If this is a parameterized scan, we also need to enforce all the join
* clauses available from the outer relation(s).
*
* For paranoia's sake, don't modify the stored baserestrictinfo list.
*/
if (best_path->param_info)
scan_clauses = list_concat(list_copy(scan_clauses),
@@ -2114,20 +2117,26 @@ create_foreignscan_plan(PlannerInfo *root, ForeignPath *best_path,
*/
if (scan_relid > 0)
{
RangeTblEntry *rte;
Assert(rel->rtekind == RTE_RELATION);
rte = planner_rt_fetch(scan_relid, root);
Assert(rte->rtekind == RTE_RELATION);
rel_oid = rte->relid;
}
+ else
+ {
+ Assert(rel->reloptkind == RELOPT_JOINREL);
+ Assert(!scan_clauses);
+ scan_clauses = best_path->joinrestrictinfo;
+ }
/*
* Sort clauses into best execution order. We do this first since the FDW
* might have more info than we do and wish to adjust the ordering.
*/
scan_clauses = order_qual_clauses(root, scan_clauses);
/*
* Let the FDW perform its processing on the restriction clauses and
* generate the plan node. Note that the FDW might remove restriction
@@ -2173,21 +2182,21 @@ create_foreignscan_plan(PlannerInfo *root, ForeignPath *best_path,
* bit of a kluge and might go away someday, so we intentionally leave it
* out of the API presented to FDWs.
*
* First, examine all the attributes needed for joins or final output.
* Note: we must look at reltargetlist, not the attr_needed data, because
* attr_needed isn't computed for inheritance child rels.
*/
pull_varattnos((Node *) rel->reltargetlist, rel->relid, &attrs_used);
/* Add all the attributes used by restriction clauses. */
- foreach(lc, rel->baserestrictinfo)
+ foreach(lc, scan_clauses)
{
RestrictInfo *rinfo = (RestrictInfo *) lfirst(lc);
pull_varattnos((Node *) rinfo->clause, rel->relid, &attrs_used);
}
/* Now, are any system columns requested from rel? */
scan_plan->fsSystemCol = false;
for (i = FirstLowInvalidHeapAttributeNumber + 1; i < 0; i++)
{
diff --git a/src/include/nodes/relation.h b/src/include/nodes/relation.h
index 19752ca..c951e24 100644
--- a/src/include/nodes/relation.h
+++ b/src/include/nodes/relation.h
@@ -896,33 +896,37 @@ typedef struct BitmapOrPath
* "CTID = pseudoconstant" or "CTID = ANY(pseudoconstant_array)".
* Note they are bare expressions, not RestrictInfos.
*/
typedef struct TidPath
{
Path path;
List *tidquals; /* qual(s) involving CTID = something */
} TidPath;
/*
- * ForeignPath represents a potential scan of a foreign table
+ * ForeignPath represents a potential scan of a foreign table or join
*
* fdw_private stores FDW private data about the scan. While fdw_private is
* not actually touched by the core code during normal operations, it's
* generally a good idea to use a representation that can be dumped by
* nodeToString(), so that you can examine the structure during debugging
* with tools like pprint().
+ *
+ * For pushed down joins joinrestrictinfo should contain list of RestrictInfo to
+ * be applied to the join.
*/
typedef struct ForeignPath
{
Path path;
Path *fdw_outerpath;
List *fdw_private;
+ List *joinrestrictinfo;
} ForeignPath;
/*
* CustomPath represents a table scan done by some out-of-core extension.
*
* We provide a set of hooks here - which the provider must take care to set
* up correctly - to allow extensions to supply their own methods of scanning
* a relation. For example, a provider might provide GPU acceleration, a
* cache-based scan, or some other kind of logic we haven't dreamed up yet.
*
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
