Redesign PlanForeignScan API to allow multiple paths for a foreign table.

The original API specification only allowed an FDW to create a single
access path, which doesn't seem like a terribly good idea in hindsight.
Instead, move the responsibility for building the Path node and calling
add_path() into the FDW's PlanForeignScan function.  Now, it can do that
more than once if appropriate.  There is no longer any need for the
transient FdwPlan struct, so get rid of that.

Etsuro Fujita, Shigeru Hanada, Tom Lane

Author: tglsfdc

contrib/file_fdw/file_fdw.c

@@ -25,6 +25,7 @@
 #include "miscadmin.h"
 #include "nodes/makefuncs.h"
 #include "optimizer/cost.h"
+#include "optimizer/pathnode.h"
 #include "utils/rel.h"
 #include "utils/syscache.h"
 
@@ -93,7 +94,7 @@ PG_FUNCTION_INFO_V1(file_fdw_validator);
 /*
  * FDW callback routines
  */
-static FdwPlan *filePlanForeignScan(Oid foreigntableid,
+static void filePlanForeignScan(Oid foreigntableid,
 					PlannerInfo *root,
 					RelOptInfo *baserel);
 static void fileExplainForeignScan(ForeignScanState *node, ExplainState *es);
@@ -406,27 +407,44 @@ get_file_fdw_attribute_options(Oid relid)
 
 /*
  * filePlanForeignScan
- *		Create a FdwPlan for a scan on the foreign table
+ *		Create possible access paths for a scan on the foreign table
+ *
+ *		Currently we don't support any push-down feature, so there is only one
+ *		possible access path, which simply returns all records in the order in
+ *		the data file.
  */
-static FdwPlan *
+static void
 filePlanForeignScan(Oid foreigntableid,
 					PlannerInfo *root,
 					RelOptInfo *baserel)
 {
-	FdwPlan    *fdwplan;
 	char	   *filename;
 	List	   *options;
+	Cost		startup_cost;
+	Cost		total_cost;
 
 	/* Fetch options --- we only need filename at this point */
 	fileGetOptions(foreigntableid, &filename, &options);
 
-	/* Construct FdwPlan with cost estimates */
-	fdwplan = makeNode(FdwPlan);
+	/* Estimate costs and update baserel->rows */
 	estimate_costs(root, baserel, filename,
-				   &fdwplan->startup_cost, &fdwplan->total_cost);
-	fdwplan->fdw_private = NIL; /* not used */
+				   &startup_cost, &total_cost);
+
+	/* Create a ForeignPath node and add it as only possible path */
+	add_path(baserel, (Path *)
+			 create_foreignscan_path(root, baserel,
+									 baserel->rows,
+									 startup_cost,
+									 total_cost,
+									 NIL, /* no pathkeys */
+									 NULL, /* no outer rel either */
+									 NIL,
+									 NIL)); /* no fdw_private data */
 
-	return fdwplan;
+	/*
+	 * If data file was sorted, and we knew it somehow, we could insert
+	 * appropriate pathkeys into the ForeignPath node to tell the planner that.
+	 */
 }
 
 /*
@@ -576,6 +594,9 @@ fileReScanForeignScan(ForeignScanState *node)
 
 /*
  * Estimate costs of scanning a foreign table.
+ *
+ * In addition to setting *startup_cost and *total_cost, this should
+ * update baserel->rows.
  */
 static void
 estimate_costs(PlannerInfo *root, RelOptInfo *baserel,

doc/src/sgml/fdwhandler.sgml

@@ -88,21 +88,31 @@
 
     <para>
 <programlisting>
-FdwPlan *
+void
 PlanForeignScan (Oid foreigntableid,
                  PlannerInfo *root,
                  RelOptInfo *baserel);
 </programlisting>
 
-     Plan a scan on a foreign table. This is called when a query is planned.
+     Create possible access paths for a scan on a foreign table. This is
+     called when a query is planned.
      <literal>foreigntableid</> is the <structname>pg_class</> OID of the
      foreign table.  <literal>root</> is the planner's global information
      about the query, and <literal>baserel</> is the planner's information
      about this table.
-     The function must return a palloc'd struct that contains cost estimates
-     plus any FDW-private information that is needed to execute the foreign
-     scan at a later time.  (Note that the private information must be
-     represented in a form that <function>copyObject</> knows how to copy.)
+    </para>
+
+    <para>
+     The function must generate at least one access path (ForeignPath node)
+     for a scan on the foreign table and must call <function>add_path</> to
+     add the path to <literal>baserel-&gt;pathlist</>.  It's recommended to
+     use <function>create_foreignscan_path</> to build the ForeignPath node.
+     The function may generate multiple access paths, e.g., a path which has
+     valid <literal>pathkeys</> to represent a pre-sorted result.  Each access
+     path must contain cost estimates, and can contain any FDW-private
+     information that is needed to execute the foreign scan at a later time.
+     (Note that the private information must be represented in a form that
+     <function>copyObject</> knows how to copy.)
     </para>
 
     <para>
@@ -159,9 +169,8 @@ BeginForeignScan (ForeignScanState *node,
      its <structfield>fdw_state</> field is still NULL.  Information about
      the table to scan is accessible through the
      <structname>ForeignScanState</> node (in particular, from the underlying
-     <structname>ForeignScan</> plan node, which contains a pointer to the
-     <structname>FdwPlan</> structure returned by
-     <function>PlanForeignScan</>).
+     <structname>ForeignScan</> plan node, which contains any FDW-private
+     information provided by <function>PlanForeignScan</>).
     </para>
 
     <para>
@@ -228,9 +237,9 @@ EndForeignScan (ForeignScanState *node);
     </para>
 
     <para>
-     The <structname>FdwRoutine</> and <structname>FdwPlan</> struct types
-     are declared in <filename>src/include/foreign/fdwapi.h</>, which see
-     for additional details.
+     The <structname>FdwRoutine</> struct type is declared in
+     <filename>src/include/foreign/fdwapi.h</>, which see for additional
+     details.
     </para>
 
    </sect1>

src/backend/nodes/copyfuncs.c

@@ -23,7 +23,8 @@
 #include "postgres.h"
 
 #include "miscadmin.h"
-#include "foreign/fdwapi.h"
+#include "nodes/plannodes.h"
+#include "nodes/relation.h"
 #include "utils/datum.h"
 
 
@@ -591,21 +592,6 @@ _copyForeignScan(const ForeignScan *from)
 	 * copy remainder of node
 	 */
 	COPY_SCALAR_FIELD(fsSystemCol);
-	COPY_NODE_FIELD(fdwplan);
-
-	return newnode;
-}
-
-/*
- * _copyFdwPlan
- */
-static FdwPlan *
-_copyFdwPlan(const FdwPlan *from)
-{
-	FdwPlan    *newnode = makeNode(FdwPlan);
-
-	COPY_SCALAR_FIELD(startup_cost);
-	COPY_SCALAR_FIELD(total_cost);
 	COPY_NODE_FIELD(fdw_private);
 
 	return newnode;
@@ -3842,9 +3828,6 @@ copyObject(const void *from)
 		case T_ForeignScan:
 			retval = _copyForeignScan(from);
 			break;
-		case T_FdwPlan:
-			retval = _copyFdwPlan(from);
-			break;
 		case T_Join:
 			retval = _copyJoin(from);
 			break;

src/backend/nodes/outfuncs.c

@@ -23,7 +23,9 @@
 
 #include <ctype.h>
 
-#include "foreign/fdwapi.h"
+#include "lib/stringinfo.h"
+#include "nodes/plannodes.h"
+#include "nodes/relation.h"
 #include "utils/datum.h"
 
 
@@ -558,16 +560,6 @@ _outForeignScan(StringInfo str, const ForeignScan *node)
 	_outScanInfo(str, (const Scan *) node);
 
 	WRITE_BOOL_FIELD(fsSystemCol);
-	WRITE_NODE_FIELD(fdwplan);
-}
-
-static void
-_outFdwPlan(StringInfo str, const FdwPlan *node)
-{
-	WRITE_NODE_TYPE("FDWPLAN");
-
-	WRITE_FLOAT_FIELD(startup_cost, "%.2f");
-	WRITE_FLOAT_FIELD(total_cost, "%.2f");
 	WRITE_NODE_FIELD(fdw_private);
 }
 
@@ -1572,7 +1564,7 @@ _outForeignPath(StringInfo str, const ForeignPath *node)
 
 	_outPathInfo(str, (const Path *) node);
 
-	WRITE_NODE_FIELD(fdwplan);
+	WRITE_NODE_FIELD(fdw_private);
 }
 
 static void
@@ -2745,9 +2737,6 @@ _outNode(StringInfo str, const void *obj)
 			case T_ForeignScan:
 				_outForeignScan(str, obj);
 				break;
-			case T_FdwPlan:
-				_outFdwPlan(str, obj);
-				break;
 			case T_Join:
 				_outJoin(str, obj);
 				break;

src/backend/optimizer/path/allpaths.c

@@ -18,6 +18,7 @@
 #include <math.h>
 
 #include "catalog/pg_class.h"
+#include "foreign/fdwapi.h"
 #include "nodes/nodeFuncs.h"
 #ifdef OPTIMIZER_DEBUG
 #include "nodes/print.h"
@@ -399,15 +400,18 @@ set_foreign_size(PlannerInfo *root, RelOptInfo *rel, RangeTblEntry *rte)
 
 /*
  * set_foreign_pathlist
- *		Build the (single) access path for a foreign table RTE
+ *		Build access paths for a foreign table RTE
  */
 static void
 set_foreign_pathlist(PlannerInfo *root, RelOptInfo *rel, RangeTblEntry *rte)
 {
-	/* Generate appropriate path */
-	add_path(rel, (Path *) create_foreignscan_path(root, rel));
+	FdwRoutine *fdwroutine;
 
-	/* Select cheapest path (pretty easy in this case...) */
+	/* Call the FDW's PlanForeignScan function to generate path(s) */
+	fdwroutine = GetFdwRoutineByRelId(rte->relid);
+	fdwroutine->PlanForeignScan(rte->relid, root, rel);
+
+	/* Select cheapest path */
 	set_cheapest(rel);
 }
 

src/backend/optimizer/plan/createplan.c

@@ -20,7 +20,6 @@
 #include <math.h>
 
 #include "access/skey.h"
-#include "foreign/fdwapi.h"
 #include "miscadmin.h"
 #include "nodes/makefuncs.h"
 #include "nodes/nodeFuncs.h"
@@ -121,7 +120,7 @@ static CteScan *make_ctescan(List *qptlist, List *qpqual,
 static WorkTableScan *make_worktablescan(List *qptlist, List *qpqual,
 				   Index scanrelid, int wtParam);
 static ForeignScan *make_foreignscan(List *qptlist, List *qpqual,
-				 Index scanrelid, bool fsSystemCol, FdwPlan *fdwplan);
+				 Index scanrelid, bool fsSystemCol, List *fdw_private);
 static BitmapAnd *make_bitmap_and(List *bitmapplans);
 static BitmapOr *make_bitmap_or(List *bitmapplans);
 static NestLoop *make_nestloop(List *tlist,
@@ -1847,7 +1846,7 @@ create_foreignscan_plan(PlannerInfo *root, ForeignPath *best_path,
 								 scan_clauses,
 								 scan_relid,
 								 fsSystemCol,
-								 best_path->fdwplan);
+								 best_path->fdw_private);
 
 	copy_path_costsize(&scan_plan->scan.plan, &best_path->path);
 
@@ -3189,7 +3188,7 @@ make_foreignscan(List *qptlist,
 				 List *qpqual,
 				 Index scanrelid,
 				 bool fsSystemCol,
-				 FdwPlan *fdwplan)
+				 List *fdw_private)
 {
 	ForeignScan *node = makeNode(ForeignScan);
 	Plan	   *plan = &node->scan.plan;
@@ -3201,7 +3200,7 @@ make_foreignscan(List *qptlist,
 	plan->righttree = NULL;
 	node->scan.scanrelid = scanrelid;
 	node->fsSystemCol = fsSystemCol;
-	node->fdwplan = fdwplan;
+	node->fdw_private = fdw_private;
 
 	return node;
 }

src/backend/optimizer/util/pathnode.c

@@ -16,7 +16,6 @@
 
 #include <math.h>
 
-#include "foreign/fdwapi.h"
 #include "miscadmin.h"
 #include "nodes/nodeFuncs.h"
 #include "optimizer/clauses.h"
@@ -1766,36 +1765,31 @@ create_worktablescan_path(PlannerInfo *root, RelOptInfo *rel)
  * create_foreignscan_path
  *	  Creates a path corresponding to a scan of a foreign table,
  *	  returning the pathnode.
+ *
+ * This function is never called from core Postgres; rather, it's expected
+ * to be called by the PlanForeignScan function of a foreign data wrapper.
+ * We make the FDW supply all fields of the path, since we do not have any
+ * way to calculate them in core.
  */
 ForeignPath *
-create_foreignscan_path(PlannerInfo *root, RelOptInfo *rel)
+create_foreignscan_path(PlannerInfo *root, RelOptInfo *rel,
+						double rows, Cost startup_cost, Cost total_cost,
+						List *pathkeys,
+						Relids required_outer, List *param_clauses,
+						List *fdw_private)
 {
 	ForeignPath *pathnode = makeNode(ForeignPath);
-	RangeTblEntry *rte;
-	FdwRoutine *fdwroutine;
-	FdwPlan    *fdwplan;
 
 	pathnode->path.pathtype = T_ForeignScan;
 	pathnode->path.parent = rel;
-	pathnode->path.pathkeys = NIL;		/* result is always unordered */
-	pathnode->path.required_outer = NULL;
-	pathnode->path.param_clauses = NIL;
+	pathnode->path.rows = rows;
+	pathnode->path.startup_cost = startup_cost;
+	pathnode->path.total_cost = total_cost;
+	pathnode->path.pathkeys = pathkeys;
+	pathnode->path.required_outer = required_outer;
+	pathnode->path.param_clauses = param_clauses;
 
-	/* Get FDW's callback info */
-	rte = planner_rt_fetch(rel->relid, root);
-	fdwroutine = GetFdwRoutineByRelId(rte->relid);
-
-	/* Let the FDW do its planning */
-	fdwplan = fdwroutine->PlanForeignScan(rte->relid, root, rel);
-	if (fdwplan == NULL || !IsA(fdwplan, FdwPlan))
-		elog(ERROR, "foreign-data wrapper PlanForeignScan function for relation %u did not return an FdwPlan struct",
-			 rte->relid);
-	pathnode->fdwplan = fdwplan;
-
-	/* use costs estimated by FDW */
-	pathnode->path.rows = rel->rows;
-	pathnode->path.startup_cost = fdwplan->startup_cost;
-	pathnode->path.total_cost = fdwplan->total_cost;
+	pathnode->fdw_private = fdw_private;
 
 	return pathnode;
 }