On Thu, Nov 26, 2015 at 12:04 AM, Kouhei Kaigai <kai...@ak.jp.nec.com> wrote:
> The attached patch adds: Path *fdw_outerpath field to ForeignPath node.
> FDW driver can set arbitrary but one path-node here.
> After that, this path-node shall be transformed to plan-node by
> createplan.c, then passed to FDW driver using GetForeignPlan callback.
> We expect FDW driver set this plan-node on lefttree (a.k.a outerPlan).
> The Plan->outerPlan is a common field, so patch size become relatively
> small. FDW driver can initialize this plan at BeginForeignScan, then
> execute this sub-plan-tree on demand.
>
> Remaining portions are as previous version. ExecScanFetch is revised
> to call recheckMtd always when scanrelid==0, then FDW driver can get
> control using RecheckForeignScan callback.
> It allows FDW driver to handle (1) EPQ recheck on underlying scan nodes,
> (2) reconstruction of joined tuple, and (3) EPQ recheck on join clauses,
> by its preferable implementation - including execution of an alternative
> sub-plan.
>
>> There seems to be no changes to make_foreignscan. Is that OK?
>>
> create_foreignscan_path(), not only make_foreignscan().
>
> This patch is not tested by actual FDW extensions, so it is helpful
> to enhance postgres_fdw to run the alternative sub-plan on EPQ recheck.
I have done some editing and some small revisions on this patch.
Here's what I came up with. The revisions are mostly cosmetic, but I
revised it a bit so that the signature of GetForeignPlan need not
change. Also, I made nodeForeignScan.c do some of the outer plan
handling automatically, and I fixed the compile breaks in
contrib/file_fdw and contrib/postgres_fdw.
Comments/review/testing are very welcome.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
diff --git a/contrib/file_fdw/file_fdw.c b/contrib/file_fdw/file_fdw.c
index 5ce8f90..1966b51 100644
--- a/contrib/file_fdw/file_fdw.c
+++ b/contrib/file_fdw/file_fdw.c
@@ -525,6 +525,7 @@ fileGetForeignPaths(PlannerInfo *root,
total_cost,
NIL, /* no pathkeys */
NULL, /* no outer rel either */
+ NULL, /* no extra plan */
coptions));
/*
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index a6ba672..dd63159 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -535,6 +535,7 @@ postgresGetForeignPaths(PlannerInfo *root,
fpinfo->total_cost,
NIL, /* no pathkeys */
NULL, /* no outer rel either */
+ NULL, /* no extra plan */
NIL); /* no fdw_private list */
add_path(baserel, (Path *) path);
@@ -589,6 +590,7 @@ postgresGetForeignPaths(PlannerInfo *root,
total_cost,
usable_pathkeys,
NULL,
+ NULL,
NIL));
}
@@ -756,6 +758,7 @@ postgresGetForeignPaths(PlannerInfo *root,
total_cost,
NIL, /* no pathkeys */
param_info->ppi_req_outer,
+ NULL,
NIL); /* no fdw_private list */
add_path(baserel, (Path *) path);
}
diff --git a/doc/src/sgml/fdwhandler.sgml b/doc/src/sgml/fdwhandler.sgml
index 1533a6b..a646b2a 100644
--- a/doc/src/sgml/fdwhandler.sgml
+++ b/doc/src/sgml/fdwhandler.sgml
@@ -765,6 +765,35 @@ RefetchForeignRow (EState *estate,
See <xref linkend="fdw-row-locking"> for more information.
</para>
+ <para>
+<programlisting>
+bool
+RecheckForeignScan (ForeignScanState *node, TupleTableSlot *slot);
+</programlisting>
+ Recheck that a previously-returned tuple still matches the relevant
+ scan and join qualifiers, and possibly provide a modified version of
+ the tuple. For foreign data wrappers which do not perform join pushdown,
+ it will typically be more convenient to set this to <literal>NULL</> and
+ instead set <structfield>fdw_recheck_quals</structfield> appropriately.
+ When outer joins are pushed down, however, it isn't sufficient to
+ reapply the checks relevant to all the base tables to the result tuple,
+ even if all needed attributes are present, because failure to match some
+ qualifier might result in some attributes going to NULL, rather than in
+ no tuple being returned. <literal>RecheckForeignScan</> can recheck
+ qualifiers and return true if they are still satisfied and false
+ otherwise, but it can also store a replacement tuple into the supplied
+ slot.
+ </para>
+
+ <para>
+ To implement join pushdown, a foreign data wrapper will typically
+ construct an alternative local join plan which is used only for
+ rechecks; this will become the outer subplan of the
+ <literal>ForeignScan</>. When a recheck is required, this subplan
+ can be executed and the resulting tuple can be stored in the slot.
+ This plan need not be efficient since no base table will return more
+ that one row; for example, it may implement all joins as nested loops.
+ </para>
</sect2>
<sect2 id="fdw-callbacks-explain">
@@ -1137,11 +1166,17 @@ GetForeignServerByName(const char *name, bool missing_ok);
<para>
Any clauses removed from the plan node's qual list must instead be added
- to <literal>fdw_recheck_quals</> in order to ensure correct behavior
+ to <literal>fdw_recheck_quals</> or rechecked by
+ <literal>RecheckForeignScan</> in order to ensure correct behavior
at the <literal>READ COMMITTED</> isolation level. When a concurrent
update occurs for some other table involved in the query, the executor
may need to verify that all of the original quals are still satisfied for
- the tuple, possibly against a different set of parameter values.
+ the tuple, possibly against a different set of parameter values. Using
+ <literal>fdw_recheck_quals</> is typically easier than implementing checks
+ inside <literal>RecheckForeignScan</>, but this method will be
+ insufficient when outer joins have been pushed down, since the join tuples
+ in that case might have some fields go to NULL without rejecting the
+ tuple entirely.
</para>
<para>
diff --git a/src/backend/executor/execScan.c b/src/backend/executor/execScan.c
index a96e826..3faf7f9 100644
--- a/src/backend/executor/execScan.c
+++ b/src/backend/executor/execScan.c
@@ -49,8 +49,21 @@ ExecScanFetch(ScanState *node,
*/
Index scanrelid = ((Scan *) node->ps.plan)->scanrelid;
- Assert(scanrelid > 0);
- if (estate->es_epqTupleSet[scanrelid - 1])
+ if (scanrelid == 0)
+ {
+ TupleTableSlot *slot = node->ss_ScanTupleSlot;
+
+ /*
+ * This is a ForeignScan or CustomScan which has pushed down a
+ * join to the remote side. The recheck method is responsible not
+ * only for rechecking the scan/join quals but also for storing
+ * the correct tuple in the slot.
+ */
+ if (!(*recheckMtd) (node, slot))
+ ExecClearTuple(slot); /* would not be returned by scan */
+ return slot;
+ }
+ else if (estate->es_epqTupleSet[scanrelid - 1])
{
TupleTableSlot *slot = node->ss_ScanTupleSlot;
@@ -347,8 +360,31 @@ ExecScanReScan(ScanState *node)
{
Index scanrelid = ((Scan *) node->ps.plan)->scanrelid;
- Assert(scanrelid > 0);
+ if (scanrelid > 0)
+ estate->es_epqScanDone[scanrelid - 1] = false;
+ else
+ {
+ Bitmapset *relids;
+ int rtindex = -1;
- estate->es_epqScanDone[scanrelid - 1] = false;
+ /*
+ * If an FDW or custom scan provider has replaced the join with a
+ * scan, there are multiple RTIs; reset the epqScanDone flag for
+ * all of them.
+ */
+ if (IsA(node->ps.plan, ForeignScan))
+ relids = ((ForeignScan *) node->ps.plan)->fs_relids;
+ else if (IsA(node->ps.plan, CustomScan))
+ relids = ((CustomScan *) node->ps.plan)->custom_relids;
+ else
+ elog(ERROR, "unexpected scan node: %d",
+ (int) nodeTag(node->ps.plan));
+
+ while ((rtindex = bms_next_member(relids, rtindex)) >= 0)
+ {
+ Assert(rtindex > 0);
+ estate->es_epqScanDone[rtindex - 1] = false;
+ }
+ }
}
}
diff --git a/src/backend/executor/nodeForeignscan.c b/src/backend/executor/nodeForeignscan.c
index 6165e4a..fdf88ff 100644
--- a/src/backend/executor/nodeForeignscan.c
+++ b/src/backend/executor/nodeForeignscan.c
@@ -73,6 +73,7 @@ ForeignNext(ForeignScanState *node)
static bool
ForeignRecheck(ForeignScanState *node, TupleTableSlot *slot)
{
+ FdwRoutine *fdwroutine = node->fdwroutine;
ExprContext *econtext;
/*
@@ -85,6 +86,18 @@ ForeignRecheck(ForeignScanState *node, TupleTableSlot *slot)
ResetExprContext(econtext);
+ /*
+ * If an outer join is pushed down, RecheckForeignScan may need to store a
+ * different tuple in the slot, because a different set of columns may go
+ * to NULL upon recheck. Otherwise, it shouldn't need to change the slot
+ * contents, just return true or false to indicate whether the quals still
+ * pass. For simple cases, setting fdw_recheck_quals may be easier than
+ * providing this callback.
+ */
+ if (fdwroutine->RecheckForeignScan &&
+ !fdwroutine->RecheckForeignScan(node, slot))
+ return false;
+
return ExecQual(node->fdw_recheck_quals, econtext, false);
}
@@ -205,6 +218,11 @@ ExecInitForeignScan(ForeignScan *node, EState *estate, int eflags)
scanstate->fdwroutine = fdwroutine;
scanstate->fdw_state = NULL;
+ /* Initialize any outer plan. */
+ if (outerPlanState(scanstate))
+ outerPlanState(scanstate) =
+ ExecInitNode(outerPlan(node), estate, eflags);
+
/*
* Tell the FDW to initialize the scan.
*/
@@ -225,6 +243,10 @@ ExecEndForeignScan(ForeignScanState *node)
/* Let the FDW shut down */
node->fdwroutine->EndForeignScan(node);
+ /* Shut down any outer plan. */
+ if (outerPlanState(node))
+ ExecEndNode(outerPlanState(node));
+
/* Free the exprcontext */
ExecFreeExprContext(&node->ss.ps);
@@ -246,7 +268,17 @@ ExecEndForeignScan(ForeignScanState *node)
void
ExecReScanForeignScan(ForeignScanState *node)
{
+ PlanState *outerPlan = outerPlanState(node);
+
node->fdwroutine->ReScanForeignScan(node);
+ /*
+ * If chgParam of subnode is not null then plan will be re-scanned by
+ * first ExecProcNode. outerPlan may also be NULL, in which case there
+ * is nothing to rescan at all.
+ */
+ if (outerPlan != NULL && outerPlan->chgParam == NULL)
+ ExecReScan(outerPlan);
+
ExecScanReScan(&node->ss);
}
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index 012c14b..aff27ea 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -1683,6 +1683,7 @@ _outForeignPath(StringInfo str, const ForeignPath *node)
_outPathInfo(str, (const Path *) node);
+ WRITE_NODE_FIELD(fdw_outerpath);
WRITE_NODE_FIELD(fdw_private);
}
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 411b36c..7335579 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -2095,11 +2095,16 @@ create_foreignscan_plan(PlannerInfo *root, ForeignPath *best_path,
Index scan_relid = rel->relid;
Oid rel_oid = InvalidOid;
Bitmapset *attrs_used = NULL;
+ Plan *fdw_outerplan = NULL;
ListCell *lc;
int i;
Assert(rel->fdwroutine != NULL);
+ /* transform the child path if any */
+ if (best_path->fdw_outerpath)
+ fdw_outerplan = create_plan_recurse(root, best_path->fdw_outerpath);
+
/*
* If we're scanning a base relation, fetch its OID. (Irrelevant if
* scanning a join relation.)
@@ -2129,7 +2134,9 @@ create_foreignscan_plan(PlannerInfo *root, ForeignPath *best_path,
*/
scan_plan = rel->fdwroutine->GetForeignPlan(root, rel, rel_oid,
best_path,
- tlist, scan_clauses);
+ tlist,
+ scan_clauses);
+ outerPlan(scan_plan) = fdw_outerplan;
/* Copy cost data from Path to Plan; no need to make FDW do this */
copy_generic_path_info(&scan_plan->scan.plan, &best_path->path);
@@ -3755,7 +3762,6 @@ make_foreignscan(List *qptlist,
/* cost will be filled in by create_foreignscan_plan */
plan->targetlist = qptlist;
plan->qual = qpqual;
- plan->lefttree = NULL;
plan->righttree = NULL;
node->scan.scanrelid = scanrelid;
/* fs_server will be filled in by create_foreignscan_plan */
diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c
index 09c3244..ec0910d 100644
--- a/src/backend/optimizer/util/pathnode.c
+++ b/src/backend/optimizer/util/pathnode.c
@@ -1507,6 +1507,7 @@ create_foreignscan_path(PlannerInfo *root, RelOptInfo *rel,
double rows, Cost startup_cost, Cost total_cost,
List *pathkeys,
Relids required_outer,
+ Path *fdw_outerpath,
List *fdw_private)
{
ForeignPath *pathnode = makeNode(ForeignPath);
@@ -1521,6 +1522,7 @@ create_foreignscan_path(PlannerInfo *root, RelOptInfo *rel,
pathnode->path.total_cost = total_cost;
pathnode->path.pathkeys = pathkeys;
+ pathnode->fdw_outerpath = fdw_outerpath;
pathnode->fdw_private = fdw_private;
return pathnode;
diff --git a/src/include/foreign/fdwapi.h b/src/include/foreign/fdwapi.h
index 69b48b4..1a5e1fd 100644
--- a/src/include/foreign/fdwapi.h
+++ b/src/include/foreign/fdwapi.h
@@ -36,13 +36,16 @@ typedef ForeignScan *(*GetForeignPlan_function) (PlannerInfo *root,
Oid foreigntableid,
ForeignPath *best_path,
List *tlist,
- List *scan_clauses);
+ List *scan_clauses);
typedef void (*BeginForeignScan_function) (ForeignScanState *node,
int eflags);
typedef TupleTableSlot *(*IterateForeignScan_function) (ForeignScanState *node);
+typedef bool (*RecheckForeignScan_function) (ForeignScanState *node,
+ TupleTableSlot *slot);
+
typedef void (*ReScanForeignScan_function) (ForeignScanState *node);
typedef void (*EndForeignScan_function) (ForeignScanState *node);
@@ -162,6 +165,7 @@ typedef struct FdwRoutine
/* Functions for SELECT FOR UPDATE/SHARE row locking */
GetForeignRowMarkType_function GetForeignRowMarkType;
RefetchForeignRow_function RefetchForeignRow;
+ RecheckForeignScan_function RecheckForeignScan;
/* Support functions for EXPLAIN */
ExplainForeignScan_function ExplainForeignScan;
diff --git a/src/include/nodes/relation.h b/src/include/nodes/relation.h
index 9a0dd28..b072e1e 100644
--- a/src/include/nodes/relation.h
+++ b/src/include/nodes/relation.h
@@ -909,6 +909,7 @@ typedef struct TidPath
typedef struct ForeignPath
{
Path path;
+ Path *fdw_outerpath;
List *fdw_private;
} ForeignPath;
diff --git a/src/include/optimizer/pathnode.h b/src/include/optimizer/pathnode.h
index f28b4e2..35e17e7 100644
--- a/src/include/optimizer/pathnode.h
+++ b/src/include/optimizer/pathnode.h
@@ -86,6 +86,7 @@ extern ForeignPath *create_foreignscan_path(PlannerInfo *root, RelOptInfo *rel,
double rows, Cost startup_cost, Cost total_cost,
List *pathkeys,
Relids required_outer,
+ Path *fdw_outerpath,
List *fdw_private);
extern Relids calc_nestloop_required_outer(Path *outer_path, Path *inner_path);
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
