On 2015/09/03 19:25, Etsuro Fujita wrote:
On 2015/09/03 14:22, Etsuro Fujita wrote:
On 2015/09/03 9:41, Robert Haas wrote:
That having been said, I don't entirely like Fujita-san's patch
either. Much of the new code is called immediately adjacent to an FDW
callback which could pretty trivially do the same thing itself, if
needed.
Another idea about that code is to call that code in eg, ExecProcNode,
instead of calling ExecForeignScan there. I think that that might be
much cleaner and resolve the naming problem below.
I gave it another thought; the following changes to ExecInitNode would
make the patch much simpler, ie, we would no longer need to call the new
code in ExecInitForeignScan, ExecForeignScan, ExecEndForeignScan, and
ExecReScanForeignScan. I think that would resolve the name problem also.
I'm attaching an updated version of the patch. The patch is based on
the SS_finalize_plan patch that has been recently committed. I'd be
happy if this helps people discuss more about how to fix this issue.
Best regards,
Etsuro Fujita
*** a/contrib/file_fdw/file_fdw.c
--- b/contrib/file_fdw/file_fdw.c
***************
*** 525,530 **** fileGetForeignPaths(PlannerInfo *root,
--- 525,531 ----
total_cost,
NIL, /* no pathkeys */
NULL, /* no outer rel either */
+ NULL, /* no alternative path */
coptions));
/*
***************
*** 563,569 **** fileGetForeignPlan(PlannerInfo *root,
scan_relid,
NIL, /* no expressions to evaluate */
best_path->fdw_private,
! NIL /* no custom tlist */ );
}
/*
--- 564,571 ----
scan_relid,
NIL, /* no expressions to evaluate */
best_path->fdw_private,
! NIL, /* no custom tlist */
! NIL /* no remote quals */ );
}
/*
*** a/contrib/postgres_fdw/postgres_fdw.c
--- b/contrib/postgres_fdw/postgres_fdw.c
***************
*** 560,565 **** postgresGetForeignPaths(PlannerInfo *root,
--- 560,566 ----
fpinfo->total_cost,
NIL, /* no pathkeys */
NULL, /* no outer rel either */
+ NULL, /* no alternative path */
NIL); /* no fdw_private list */
add_path(baserel, (Path *) path);
***************
*** 727,732 **** postgresGetForeignPaths(PlannerInfo *root,
--- 728,734 ----
total_cost,
NIL, /* no pathkeys */
param_info->ppi_req_outer,
+ NULL, /* no alternative path */
NIL); /* no fdw_private list */
add_path(baserel, (Path *) path);
}
***************
*** 748,753 **** postgresGetForeignPlan(PlannerInfo *root,
--- 750,756 ----
Index scan_relid = baserel->relid;
List *fdw_private;
List *remote_conds = NIL;
+ List *remote_exprs = NIL;
List *local_exprs = NIL;
List *params_list = NIL;
List *retrieved_attrs;
***************
*** 769,776 **** postgresGetForeignPlan(PlannerInfo *root,
*
* This code must match "extract_actual_clauses(scan_clauses, false)"
* except for the additional decision about remote versus local execution.
! * Note however that we only strip the RestrictInfo nodes from the
! * local_exprs list, since appendWhereClause expects a list of
* RestrictInfos.
*/
foreach(lc, scan_clauses)
--- 772,779 ----
*
* This code must match "extract_actual_clauses(scan_clauses, false)"
* except for the additional decision about remote versus local execution.
! * Note however that we don't strip the RestrictInfo nodes from the
! * remote_conds list, since appendWhereClause expects a list of
* RestrictInfos.
*/
foreach(lc, scan_clauses)
***************
*** 784,794 **** postgresGetForeignPlan(PlannerInfo *root,
--- 787,803 ----
continue;
if (list_member_ptr(fpinfo->remote_conds, rinfo))
+ {
remote_conds = lappend(remote_conds, rinfo);
+ remote_exprs = lappend(remote_exprs, rinfo->clause);
+ }
else if (list_member_ptr(fpinfo->local_conds, rinfo))
local_exprs = lappend(local_exprs, rinfo->clause);
else if (is_foreign_expr(root, baserel, rinfo->clause))
+ {
remote_conds = lappend(remote_conds, rinfo);
+ remote_exprs = lappend(remote_exprs, rinfo->clause);
+ }
else
local_exprs = lappend(local_exprs, rinfo->clause);
}
***************
*** 874,880 **** postgresGetForeignPlan(PlannerInfo *root,
scan_relid,
params_list,
fdw_private,
! NIL /* no custom tlist */ );
}
/*
--- 883,890 ----
scan_relid,
params_list,
fdw_private,
! NIL, /* no custom tlist */
! remote_exprs);
}
/*
*** a/doc/src/sgml/fdwhandler.sgml
--- b/doc/src/sgml/fdwhandler.sgml
***************
*** 333,339 **** GetForeignJoinPaths (PlannerInfo *root,
remote join cannot be found from the system catalogs, the FDW must
fill <structfield>fdw_scan_tlist</> with an appropriate list
of <structfield>TargetEntry</> nodes, representing the set of columns
! it will supply at runtime in the tuples it returns.
</para>
<para>
--- 333,343 ----
remote join cannot be found from the system catalogs, the FDW must
fill <structfield>fdw_scan_tlist</> with an appropriate list
of <structfield>TargetEntry</> nodes, representing the set of columns
! it will supply at runtime in the tuples it returns. Yet another
! difference is that, for possible use in the
! <productname>PostgreSQL</productname> executor, the FDW must fill
! <structfield>fs_subplan</> with a local join plan equivalent to
! the resulting <structname>ForeignScan</> plan.
</para>
<para>
***************
*** 1111,1117 **** GetForeignServerByName(const char *name, bool missing_ok);
clauses will be checked by the executor at run time. More complex FDWs
may be able to check some of the clauses internally, in which case those
clauses can be removed from the plan node's qual list so that the
! executor doesn't waste time rechecking them.
</para>
<para>
--- 1115,1124 ----
clauses will be checked by the executor at run time. More complex FDWs
may be able to check some of the clauses internally, in which case those
clauses can be removed from the plan node's qual list so that the
! executor doesn't waste time rechecking them. In the case of planning
! foreign table scans, removed clauses must be added to the
! <structfield>fdw_quals</> list of the <structname>ForeignScan</> node
! for possible use in the <productname>PostgreSQL</productname> executor.
</para>
<para>
*** a/src/backend/executor/execProcnode.c
--- b/src/backend/executor/execProcnode.c
***************
*** 247,254 **** ExecInitNode(Plan *node, EState *estate, int eflags)
break;
case T_ForeignScan:
! result = (PlanState *) ExecInitForeignScan((ForeignScan *) node,
! estate, eflags);
break;
case T_CustomScan:
--- 247,274 ----
break;
case T_ForeignScan:
! {
! Index scanrelid = ((ForeignScan *) node)->scan.scanrelid;
!
! if (estate->es_epqTuple != NULL && scanrelid == 0)
! {
! /*
! * We are in foreign join for EvalPlanQual testing.
! * Initialize the local join plan, instead.
! *
! * Note: initPlans attached to this PlanState node below
! * will be in line with those attached to the plan. See
! * SS_finalize_plan().
! */
! Plan *subplan = ((ForeignScan *) node)->fs_subplan;
!
! Assert(subplan != NULL);
! result = ExecInitNode(subplan, estate, eflags);
! }
! else
! result = (PlanState *) ExecInitForeignScan((ForeignScan *) node,
! estate, eflags);
! }
break;
case T_CustomScan:
*** a/src/backend/executor/nodeForeignscan.c
--- b/src/backend/executor/nodeForeignscan.c
***************
*** 72,79 **** ForeignNext(ForeignScanState *node)
static bool
ForeignRecheck(ForeignScanState *node, TupleTableSlot *slot)
{
! /* There are no access-method-specific conditions to recheck. */
! return true;
}
/* ----------------------------------------------------------------
--- 72,90 ----
static bool
ForeignRecheck(ForeignScanState *node, TupleTableSlot *slot)
{
! ExprContext *econtext;
!
! /*
! * extract necessary information from foreign scan node
! */
! econtext = node->ss.ps.ps_ExprContext;
!
! /* Does the tuple meet the remote qual condition? */
! econtext->ecxt_scantuple = slot;
!
! ResetExprContext(econtext);
!
! return ExecQual(node->fdw_quals, econtext, false);
}
/* ----------------------------------------------------------------
***************
*** 135,140 **** ExecInitForeignScan(ForeignScan *node, EState *estate, int eflags)
--- 146,154 ----
scanstate->ss.ps.qual = (List *)
ExecInitExpr((Expr *) node->scan.plan.qual,
(PlanState *) scanstate);
+ scanstate->fdw_quals = (List *)
+ ExecInitExpr((Expr *) node->fdw_quals,
+ (PlanState *) scanstate);
/*
* tuple table initialization
*** a/src/backend/nodes/copyfuncs.c
--- b/src/backend/nodes/copyfuncs.c
***************
*** 624,629 **** _copyForeignScan(const ForeignScan *from)
--- 624,631 ----
COPY_NODE_FIELD(fdw_exprs);
COPY_NODE_FIELD(fdw_private);
COPY_NODE_FIELD(fdw_scan_tlist);
+ COPY_NODE_FIELD(fdw_quals);
+ COPY_NODE_FIELD(fs_subplan);
COPY_BITMAPSET_FIELD(fs_relids);
COPY_SCALAR_FIELD(fsSystemCol);
*** a/src/backend/nodes/outfuncs.c
--- b/src/backend/nodes/outfuncs.c
***************
*** 579,584 **** _outForeignScan(StringInfo str, const ForeignScan *node)
--- 579,586 ----
WRITE_NODE_FIELD(fdw_exprs);
WRITE_NODE_FIELD(fdw_private);
WRITE_NODE_FIELD(fdw_scan_tlist);
+ WRITE_NODE_FIELD(fdw_quals);
+ WRITE_NODE_FIELD(fs_subplan);
WRITE_BITMAPSET_FIELD(fs_relids);
WRITE_BOOL_FIELD(fsSystemCol);
}
*** a/src/backend/optimizer/plan/createplan.c
--- b/src/backend/optimizer/plan/createplan.c
***************
*** 2117,2125 **** create_foreignscan_plan(PlannerInfo *root, ForeignPath *best_path,
--- 2117,2134 ----
replace_nestloop_params(root, (Node *) scan_plan->scan.plan.qual);
scan_plan->fdw_exprs = (List *)
replace_nestloop_params(root, (Node *) scan_plan->fdw_exprs);
+ scan_plan->fdw_quals = (List *)
+ replace_nestloop_params(root, (Node *) scan_plan->fdw_quals);
}
/*
+ * If we're scanning a join relation, generate the local join plan for
+ * EvalPlanQual support. (Irrelevant if scanning a base relation.)
+ */
+ if (scan_relid == 0)
+ scan_plan->fs_subplan = create_plan_recurse(root, best_path->subpath);
+
+ /*
* Detect whether any system columns are requested from rel. This is a
* bit of a kluge and might go away someday, so we intentionally leave it
* out of the API presented to FDWs.
***************
*** 3702,3708 **** make_foreignscan(List *qptlist,
Index scanrelid,
List *fdw_exprs,
List *fdw_private,
! List *fdw_scan_tlist)
{
ForeignScan *node = makeNode(ForeignScan);
Plan *plan = &node->scan.plan;
--- 3711,3718 ----
Index scanrelid,
List *fdw_exprs,
List *fdw_private,
! List *fdw_scan_tlist,
! List *fdw_quals)
{
ForeignScan *node = makeNode(ForeignScan);
Plan *plan = &node->scan.plan;
***************
*** 3718,3723 **** make_foreignscan(List *qptlist,
--- 3728,3736 ----
node->fdw_exprs = fdw_exprs;
node->fdw_private = fdw_private;
node->fdw_scan_tlist = fdw_scan_tlist;
+ node->fdw_quals = fdw_quals;
+ /* fs_subplan will be filled in by create_foreignscan_plan */
+ node->fs_subplan = NULL;
/* fs_relids will be filled in by create_foreignscan_plan */
node->fs_relids = NULL;
/* fsSystemCol will be filled in by create_foreignscan_plan */
*** a/src/backend/optimizer/plan/setrefs.c
--- b/src/backend/optimizer/plan/setrefs.c
***************
*** 1124,1139 **** set_foreignscan_references(PlannerInfo *root,
/* fdw_scan_tlist itself just needs fix_scan_list() adjustments */
fscan->fdw_scan_tlist =
fix_scan_list(root, fscan->fdw_scan_tlist, rtoffset);
}
else
{
! /* Adjust tlist, qual, fdw_exprs in the standard way */
fscan->scan.plan.targetlist =
fix_scan_list(root, fscan->scan.plan.targetlist, rtoffset);
fscan->scan.plan.qual =
fix_scan_list(root, fscan->scan.plan.qual, rtoffset);
fscan->fdw_exprs =
fix_scan_list(root, fscan->fdw_exprs, rtoffset);
}
/* Adjust fs_relids if needed */
--- 1124,1143 ----
/* fdw_scan_tlist itself just needs fix_scan_list() adjustments */
fscan->fdw_scan_tlist =
fix_scan_list(root, fscan->fdw_scan_tlist, rtoffset);
+ /* fs_subplan needs set_plan_refs() adjustments */
+ set_plan_refs(root, fscan->fs_subplan, rtoffset);
}
else
{
! /* Adjust tlist, qual, fdw_exprs, fdw_quals in the standard way */
fscan->scan.plan.targetlist =
fix_scan_list(root, fscan->scan.plan.targetlist, rtoffset);
fscan->scan.plan.qual =
fix_scan_list(root, fscan->scan.plan.qual, rtoffset);
fscan->fdw_exprs =
fix_scan_list(root, fscan->fdw_exprs, rtoffset);
+ fscan->fdw_quals =
+ fix_scan_list(root, fscan->fdw_quals, rtoffset);
}
/* Adjust fs_relids if needed */
*** a/src/backend/optimizer/plan/subselect.c
--- b/src/backend/optimizer/plan/subselect.c
***************
*** 2394,2403 **** finalize_plan(PlannerInfo *root, Plan *plan, Bitmapset *valid_params,
break;
case T_ForeignScan:
! finalize_primnode((Node *) ((ForeignScan *) plan)->fdw_exprs,
! &context);
! /* We assume fdw_scan_tlist cannot contain Params */
! context.paramids = bms_add_members(context.paramids, scan_params);
break;
case T_CustomScan:
--- 2394,2444 ----
break;
case T_ForeignScan:
! {
! ForeignScan *fscan = (ForeignScan *) plan;
!
! finalize_primnode((Node *) fscan->fdw_exprs, &context);
!
! /* We assume fdw_scan_tlist cannot contain Params */
! context.paramids =
! bms_add_members(context.paramids, scan_params);
!
! /*
! * We need not look at fdw_quals, since it will have the same
! * param references as fdw_exprs.
! */
!
! /* subplan node if foreign join */
! if (fscan->scan.scanrelid == 0)
! {
! /*
! * If the ForeignScan node was the topmost scan/join plan
! * node, grouping_planner() might have replaced the tlist
! * of the ForeignScan node. So, replace the tlist of the
! * subplan with that of the ForeignScan node.
! */
! fscan->fs_subplan->targetlist = plan->targetlist;
!
! /*
! * If the ForeignScan node was the topmost plan node for
! * the query level, it might have initplans. To match
! * the computed extParam/allParam sets for the subplan
! * with those for the ForeignScan node accurately, set
! * the initPlans of the subplan to the ForeignScan node's
! * initPlans. Not sure this is needed.
! */
! fscan->fs_subplan->initPlan = plan->initPlan;
!
! /*
! * We need not include the subplan's params. However,
! * the subplan itself needs finalize_plan() processing.
! */
! finalize_plan(root,
! fscan->fs_subplan,
! valid_params,
! scan_params);
! }
! }
break;
case T_CustomScan:
*** a/src/backend/optimizer/util/pathnode.c
--- b/src/backend/optimizer/util/pathnode.c
***************
*** 1462,1467 **** create_foreignscan_path(PlannerInfo *root, RelOptInfo *rel,
--- 1462,1468 ----
double rows, Cost startup_cost, Cost total_cost,
List *pathkeys,
Relids required_outer,
+ Path *subpath,
List *fdw_private)
{
ForeignPath *pathnode = makeNode(ForeignPath);
***************
*** 1475,1480 **** create_foreignscan_path(PlannerInfo *root, RelOptInfo *rel,
--- 1476,1482 ----
pathnode->path.total_cost = total_cost;
pathnode->path.pathkeys = pathkeys;
+ pathnode->subpath = subpath;
pathnode->fdw_private = fdw_private;
return pathnode;
*** a/src/include/nodes/execnodes.h
--- b/src/include/nodes/execnodes.h
***************
*** 1577,1582 **** typedef struct WorkTableScanState
--- 1577,1583 ----
typedef struct ForeignScanState
{
ScanState ss; /* its first field is NodeTag */
+ List *fdw_quals; /* remote quals if foreign table */
/* use struct pointer to avoid including fdwapi.h here */
struct FdwRoutine *fdwroutine;
void *fdw_state; /* foreign-data wrapper can keep state here */
*** a/src/include/nodes/plannodes.h
--- b/src/include/nodes/plannodes.h
***************
*** 521,526 **** typedef struct ForeignScan
--- 521,528 ----
List *fdw_exprs; /* expressions that FDW may evaluate */
List *fdw_private; /* private data for FDW */
List *fdw_scan_tlist; /* optional tlist describing scan tuple */
+ List *fdw_quals; /* remote quals if foreign table */
+ Plan *fs_subplan; /* local join plan if foreign join */
Bitmapset *fs_relids; /* RTIs generated by this scan */
bool fsSystemCol; /* true if any "system column" is needed */
} ForeignScan;
*** a/src/include/nodes/relation.h
--- b/src/include/nodes/relation.h
***************
*** 897,906 **** typedef struct TidPath
--- 897,914 ----
* 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().
+ *
+ * If a ForeignPath node represents a remote join of foreign tables, subpath
+ * is a local join of those tables with equivalent results that will be used
+ * for EvalPlanQual testing. The pathkeys and parameterization of subpath
+ * must be the same as that of the path's output. (The requirement for the
+ * pathkeys is unnecessary, since the testing can return at most one tuple
+ * for any particular set of scan tuples of those tables, but let's be safe.)
*/
typedef struct ForeignPath
{
Path path;
+ Path *subpath;
List *fdw_private;
} ForeignPath;
*** a/src/include/optimizer/pathnode.h
--- b/src/include/optimizer/pathnode.h
***************
*** 83,88 **** extern ForeignPath *create_foreignscan_path(PlannerInfo *root, RelOptInfo *rel,
--- 83,89 ----
double rows, Cost startup_cost, Cost total_cost,
List *pathkeys,
Relids required_outer,
+ Path *subpath,
List *fdw_private);
extern Relids calc_nestloop_required_outer(Path *outer_path, Path *inner_path);
*** a/src/include/optimizer/planmain.h
--- b/src/include/optimizer/planmain.h
***************
*** 45,51 **** extern SubqueryScan *make_subqueryscan(List *qptlist, List *qpqual,
Index scanrelid, Plan *subplan);
extern ForeignScan *make_foreignscan(List *qptlist, List *qpqual,
Index scanrelid, List *fdw_exprs, List *fdw_private,
! List *fdw_scan_tlist);
extern Append *make_append(List *appendplans, List *tlist);
extern RecursiveUnion *make_recursive_union(List *tlist,
Plan *lefttree, Plan *righttree, int wtParam,
--- 45,51 ----
Index scanrelid, Plan *subplan);
extern ForeignScan *make_foreignscan(List *qptlist, List *qpqual,
Index scanrelid, List *fdw_exprs, List *fdw_private,
! List *fdw_scan_tlist, List *fdw_quals);
extern Append *make_append(List *appendplans, List *tlist);
extern RecursiveUnion *make_recursive_union(List *tlist,
Plan *lefttree, Plan *righttree, int wtParam,
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
