(2012/02/21 20:25), Etsuro Fujita wrote:
> Please find attached an updated version of the patch.
This v2 patch can be applied on HEAD cleanly. Compile completed with
only one expected warning of scan.c, and all regression tests for both
core and contrib modules passed.
This patch allows FDWs to return multiple ForeignPath nodes per a
PlanForeignScan call. It also get rid of FdwPlan, FDW-private
information container, by replacing with simple List.
I've reviewed the patch closely, and have some comments about its design.
Basically a create_foo_path is responsible for creating a node object
with a particular Path-derived type, but this patch changes
create_foreignscan_path to just call PlanForeignScan and return void.
This change seems breaking module design. IMO create_foreignscan_path
should return just one ForeignPath node per a call, so calling add_path
multiple times should be done in somewhere else. I think
set_foreign_pathlist suites for it, because set_foo_pathlist functions
are responsible for building possible paths for a RangeTblEntry, as
comment of set_foreign_pathlist says.
/*
* set_foreign_pathlist
* Build one or more access paths for a foreign table RTE
*/
In this design, FDW authors can implement PlanForeignScan by repeating
steps below for each possible scan path for a foreign table:
(1) create a template ForeignPath node with create_foreignscan_path
(2) customize the path as FDW wants, e.g. push down WHERE clause
(3) store FDW-private info
(4) estimate costs of the path
(5) call add_path to add the path to RelOptInfo
Current design doesn't allow FDWs to provide multiple paths which have
different local filtering from each other, because all paths share a
RelOptInfo and baserestrictinfo in it. I think this restriction
wouldn't be a serious problem.
Please find attached a patch implementing the design above.
--
Shigeru Hanada
diff --git a/contrib/file_fdw/file_fdw.c b/contrib/file_fdw/file_fdw.c
index 46394a8..bb541e3 100644
*** a/contrib/file_fdw/file_fdw.c
--- b/contrib/file_fdw/file_fdw.c
***************
*** 25,30 ****
--- 25,31 ----
#include "miscadmin.h"
#include "nodes/makefuncs.h"
#include "optimizer/cost.h"
+ #include "optimizer/pathnode.h"
#include "utils/rel.h"
#include "utils/syscache.h"
*************** PG_FUNCTION_INFO_V1(file_fdw_validator);
*** 93,99 ****
/*
* FDW callback routines
*/
! static FdwPlan *filePlanForeignScan(Oid foreigntableid,
PlannerInfo *root,
RelOptInfo *baserel);
static void fileExplainForeignScan(ForeignScanState *node, ExplainState *es);
--- 94,100 ----
/*
* FDW callback routines
*/
! static void filePlanForeignScan(Oid foreigntableid,
PlannerInfo *root,
RelOptInfo *baserel);
static void fileExplainForeignScan(ForeignScanState *node, ExplainState *es);
*************** get_file_fdw_attribute_options(Oid relid
*** 406,432 ****
/*
* filePlanForeignScan
! * Create a FdwPlan for a scan on the foreign table
*/
! static FdwPlan *
filePlanForeignScan(Oid foreigntableid,
PlannerInfo *root,
RelOptInfo *baserel)
{
! FdwPlan *fdwplan;
char *filename;
List *options;
/* Fetch options --- we only need filename at this point */
fileGetOptions(foreigntableid, &filename, &options);
! /* Construct FdwPlan with cost estimates */
! fdwplan = makeNode(FdwPlan);
estimate_costs(root, baserel, filename,
! &fdwplan->startup_cost,
&fdwplan->total_cost);
! fdwplan->fdw_private = NIL; /* not used */
! return fdwplan;
}
/*
--- 407,443 ----
/*
* filePlanForeignScan
! * 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 void
filePlanForeignScan(Oid foreigntableid,
PlannerInfo *root,
RelOptInfo *baserel)
{
! ForeignPath *pathnode = makeNode(ForeignPath);
char *filename;
List *options;
/* Fetch options --- we only need filename at this point */
fileGetOptions(foreigntableid, &filename, &options);
! /* Create a ForeignPath node and add it as only one possible path. */
! pathnode = create_foreignscan_path(root, baserel);
! pathnode->fdw_private = NIL;
estimate_costs(root, baserel, filename,
! &pathnode->path.startup_cost,
&pathnode->path.total_cost);
! pathnode->path.rows = baserel->rows;
! add_path(baserel, (Path *) pathnode);
! /*
! * If data file was sorted, and we knew it somehow, we can create
another
! * ForeignPath node with valid pathkeys to tell planner that sorting
this
! * file by such key is not expensive.
! */
}
/*
diff --git a/doc/src/sgml/fdwhandler.sgml b/doc/src/sgml/fdwhandler.sgml
index 76ff243..63fe8bd 100644
*** a/doc/src/sgml/fdwhandler.sgml
--- b/doc/src/sgml/fdwhandler.sgml
***************
*** 88,108 ****
<para>
<programlisting>
! FdwPlan *
PlanForeignScan (Oid foreigntableid,
PlannerInfo *root,
RelOptInfo *baserel);
</programlisting>
! Plan 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>
--- 88,117 ----
<para>
<programlisting>
! void
PlanForeignScan (Oid foreigntableid,
PlannerInfo *root,
RelOptInfo *baserel);
</programlisting>
! 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.
! </para>
!
! <para>
! The function must generate at least one access path for a scan on the
! foreign table and call <function>add_path</> to add it to
! <literal>baserel->pathlist</>. The function may generate extra
! access paths, e.g. a path which has valid <literal>pathkeys</> for sorted
! result. Each access path should 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>
*************** BeginForeignScan (ForeignScanState *node
*** 159,167 ****
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</>).
</para>
<para>
--- 168,175 ----
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 FDW-private
! information about it provided by <function>PlanForeignScan</>).
</para>
<para>
*************** EndForeignScan (ForeignScanState *node);
*** 228,236 ****
</para>
<para>
! The <structname>FdwRoutine</> and <structname>FdwPlan</> struct types
! are declared in <filename>src/include/foreign/fdwapi.h</>, which see
! for additional details.
</para>
</sect1>
--- 236,244 ----
</para>
<para>
! The <structname>FdwRoutine</> struct type is declared
! in <filename>src/include/foreign/fdwapi.h</>, which see for additional
! details.
</para>
</sect1>
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 7fec4db..9b2f688 100644
*** a/src/backend/nodes/copyfuncs.c
--- b/src/backend/nodes/copyfuncs.c
*************** _copyForeignScan(const ForeignScan *from
*** 591,611 ****
* 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;
--- 591,596 ----
*************** copyObject(const void *from)
*** 3842,3850 ****
case T_ForeignScan:
retval = _copyForeignScan(from);
break;
- case T_FdwPlan:
- retval = _copyFdwPlan(from);
- break;
case T_Join:
retval = _copyJoin(from);
break;
--- 3827,3832 ----
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index 25a215e..08b74fb 100644
*** a/src/backend/nodes/outfuncs.c
--- b/src/backend/nodes/outfuncs.c
*************** _outForeignScan(StringInfo str, const Fo
*** 558,573 ****
_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);
}
--- 558,563 ----
*************** _outForeignPath(StringInfo str, const Fo
*** 1572,1578 ****
_outPathInfo(str, (const Path *) node);
! WRITE_NODE_FIELD(fdwplan);
}
static void
--- 1562,1568 ----
_outPathInfo(str, (const Path *) node);
! WRITE_NODE_FIELD(fdw_private);
}
static void
*************** _outNode(StringInfo str, const void *obj
*** 2745,2753 ****
case T_ForeignScan:
_outForeignScan(str, obj);
break;
- case T_FdwPlan:
- _outFdwPlan(str, obj);
- break;
case T_Join:
_outJoin(str, obj);
break;
--- 2735,2740 ----
diff --git a/src/backend/optimizer/path/allpaths.c
b/src/backend/optimizer/path/allpaths.c
index 8f03417..060d49d 100644
*** a/src/backend/optimizer/path/allpaths.c
--- b/src/backend/optimizer/path/allpaths.c
***************
*** 18,23 ****
--- 18,24 ----
#include <math.h>
#include "catalog/pg_class.h"
+ #include "foreign/fdwapi.h"
#include "nodes/nodeFuncs.h"
#ifdef OPTIMIZER_DEBUG
#include "nodes/print.h"
*************** set_foreign_size(PlannerInfo *root, RelO
*** 399,411 ****
/*
* set_foreign_pathlist
! * Build the (single) access path 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));
/* Select cheapest path (pretty easy in this case...) */
set_cheapest(rel);
--- 400,414 ----
/*
* set_foreign_pathlist
! * Build access paths for a foreign table RTE
*/
static void
set_foreign_pathlist(PlannerInfo *root, RelOptInfo *rel, RangeTblEntry *rte)
{
! FdwRoutine *fdwroutine;
!
! fdwroutine = GetFdwRoutineByRelId(rte->relid);
! fdwroutine->PlanForeignScan(rte->relid, root, rel);
/* Select cheapest path (pretty easy in this case...) */
set_cheapest(rel);
diff --git a/src/backend/optimizer/plan/createplan.c
b/src/backend/optimizer/plan/createplan.c
index 9ac0c99..c348fd6 100644
*** a/src/backend/optimizer/plan/createplan.c
--- b/src/backend/optimizer/plan/createplan.c
*************** static CteScan *make_ctescan(List *qptli
*** 121,127 ****
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);
static BitmapAnd *make_bitmap_and(List *bitmapplans);
static BitmapOr *make_bitmap_or(List *bitmapplans);
static NestLoop *make_nestloop(List *tlist,
--- 121,127 ----
static WorkTableScan *make_worktablescan(List *qptlist, List *qpqual,
Index scanrelid, int wtParam);
static ForeignScan *make_foreignscan(List *qptlist, List *qpqual,
! 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,
*************** create_foreignscan_plan(PlannerInfo *roo
*** 1847,1853 ****
scan_clauses,
scan_relid,
fsSystemCol,
!
best_path->fdwplan);
copy_path_costsize(&scan_plan->scan.plan, &best_path->path);
--- 1847,1853 ----
scan_clauses,
scan_relid,
fsSystemCol,
!
best_path->fdw_private);
copy_path_costsize(&scan_plan->scan.plan, &best_path->path);
*************** make_foreignscan(List *qptlist,
*** 3189,3195 ****
List *qpqual,
Index scanrelid,
bool fsSystemCol,
! FdwPlan *fdwplan)
{
ForeignScan *node = makeNode(ForeignScan);
Plan *plan = &node->scan.plan;
--- 3189,3195 ----
List *qpqual,
Index scanrelid,
bool fsSystemCol,
! List *fdw_private)
{
ForeignScan *node = makeNode(ForeignScan);
Plan *plan = &node->scan.plan;
*************** make_foreignscan(List *qptlist,
*** 3201,3207 ****
plan->righttree = NULL;
node->scan.scanrelid = scanrelid;
node->fsSystemCol = fsSystemCol;
! node->fdwplan = fdwplan;
return node;
}
--- 3201,3207 ----
plan->righttree = NULL;
node->scan.scanrelid = scanrelid;
node->fsSystemCol = fsSystemCol;
! node->fdw_private = fdw_private;
return node;
}
diff --git a/src/backend/optimizer/util/pathnode.c
b/src/backend/optimizer/util/pathnode.c
index d29b454..9c285a9 100644
*** a/src/backend/optimizer/util/pathnode.c
--- b/src/backend/optimizer/util/pathnode.c
*************** create_worktablescan_path(PlannerInfo *r
*** 1764,1801 ****
/*
* create_foreignscan_path
! * Creates a path corresponding to a scan of a foreign table,
! * returning the pathnode.
*/
ForeignPath *
create_foreignscan_path(PlannerInfo *root, RelOptInfo *rel)
{
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;
! /* 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;
return pathnode;
}
--- 1764,1795 ----
/*
* create_foreignscan_path
! * Creates a path corresponding to a scan of a foreign table, and returns
! * the pathnode.
! *
! * It's assumed that this function is called from PlanForeignScan
! * imeplementation of FDWs, but not from planner directly; planner never
! * calls this function directly. Generated ForeignPath node has valid
! * values for only requried fields, so each FDW should set optional
fields
! * as they want.
! *
! * If FDW needs to pass FDW-specific information from planner to
executor,
! * fdw_private field is available. Its type is List *, so you can store
! * Node subclass objects. Planner will copy fdw_private of cheapest
! * ForeignPath to ForeignScan plan node.
*/
ForeignPath *
create_foreignscan_path(PlannerInfo *root, RelOptInfo *rel)
{
ForeignPath *pathnode = makeNode(ForeignPath);
pathnode->path.pathtype = T_ForeignScan;
pathnode->path.parent = rel;
! pathnode->path.pathkeys = NIL;
pathnode->path.required_outer = NULL;
pathnode->path.param_clauses = NIL;
! pathnode->fdw_private = NIL;
return pathnode;
}
diff --git a/src/include/foreign/fdwapi.h b/src/include/foreign/fdwapi.h
index 3696623..4831e7c 100644
*** a/src/include/foreign/fdwapi.h
--- b/src/include/foreign/fdwapi.h
*************** struct ExplainState;
*** 20,58 ****
/*
- * FdwPlan is the information returned to the planner by PlanForeignScan.
- */
- typedef struct FdwPlan
- {
- NodeTag type;
-
- /*
- * Cost estimation info. The startup_cost is time before retrieving the
- * first row, so it should include costs of connecting to the remote
host,
- * sending over the query, etc. Note that PlanForeignScan also ought to
- * set baserel->rows and baserel->width if it can produce any usable
- * estimates of those values.
- */
- Cost startup_cost; /* cost expended before fetching any
tuples */
- Cost total_cost; /* total cost (assuming all
tuples fetched) */
-
- /*
- * FDW private data, which will be available at execution time.
- *
- * Note that everything in this list must be copiable by copyObject().
One
- * way to store an arbitrary blob of bytes is to represent it as a bytea
- * Const. Usually, though, you'll be better off choosing a
representation
- * that can be dumped usefully by nodeToString().
- */
- List *fdw_private;
- } FdwPlan;
-
-
- /*
* Callback function signatures --- see fdwhandler.sgml for more info.
*/
! typedef FdwPlan *(*PlanForeignScan_function) (Oid foreigntableid,
PlannerInfo *root,
RelOptInfo *baserel);
--- 20,29 ----
/*
* Callback function signatures --- see fdwhandler.sgml for more info.
*/
! typedef void (*PlanForeignScan_function) (Oid foreigntableid,
PlannerInfo *root,
RelOptInfo *baserel);
diff --git a/src/include/nodes/nodes.h b/src/include/nodes/nodes.h
index 0e7d184..905458f 100644
*** a/src/include/nodes/nodes.h
--- b/src/include/nodes/nodes.h
*************** typedef enum NodeTag
*** 62,68 ****
T_CteScan,
T_WorkTableScan,
T_ForeignScan,
- T_FdwPlan,
T_Join,
T_NestLoop,
T_MergeJoin,
--- 62,67 ----
diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index 7d90b91..23d9256 100644
*** a/src/include/nodes/plannodes.h
--- b/src/include/nodes/plannodes.h
*************** typedef struct ForeignScan
*** 468,475 ****
{
Scan scan;
bool fsSystemCol; /* true if any "system column" is
needed */
! /* use struct pointer to avoid including fdwapi.h here */
! struct FdwPlan *fdwplan;
} ForeignScan;
--- 468,474 ----
{
Scan scan;
bool fsSystemCol; /* true if any "system column" is
needed */
! List *fdw_private;
} ForeignScan;
diff --git a/src/include/nodes/relation.h b/src/include/nodes/relation.h
index 6ba920a..1acc4f8 100644
*** a/src/include/nodes/relation.h
--- b/src/include/nodes/relation.h
*************** typedef struct TidPath
*** 794,805 ****
/*
* ForeignPath represents a scan of a foreign table
*/
typedef struct ForeignPath
{
Path path;
! /* use struct pointer to avoid including fdwapi.h here */
! struct FdwPlan *fdwplan;
} ForeignPath;
/*
--- 794,810 ----
/*
* ForeignPath represents a scan of a foreign table
+ *
+ * fdw_private is a list of FDW private data, which will be available at
+ * execution time. Note that everything in this list must be copiable
+ * by copyObject(). One way to store an arbitrary blob of bytes is to
+ * represent it as a bytea Const. Usually, though, you'll be better off
+ * choosing a representation that can be dumped usefully by nodeToString().
*/
typedef struct ForeignPath
{
Path path;
! List *fdw_private;
} ForeignPath;
/*
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
