(2012/02/15 20:50), Etsuro Fujita wrote:
> (2012/02/14 23:50), Tom Lane wrote:
>>> (2012/02/14 17:40), Etsuro Fujita wrote:
>>>> As discussed at
>>>> that thread, it would have to change the PlanForeignScan API to let the
>>>> FDW generate multiple paths and dump them all to add_path instead of
>>>> returning a FdwPlan struct.
>> I would really like to see that happen in 9.2, because the longer we let
>> that mistake live, the harder it will be to change. More and more FDWs
>> are getting written. I don't think it's that hard to do: we just have
>> to agree that PlanForeignScan should return void and call add_path for
>> itself, possibly more than once.
>
> Agreed. I fixed the PlanForeignScan API. Please find attached a patch.
>
>> If we do that, I'm inclined to think
>> we cou;d get rid of the separate Node type FdwPlan, and just incorporate
>> "List *fdw_private" into ForeignPath and ForeignScan.
>
> +1 While the patch retains the struct FdwPlan, I would like to get rid
> of it at next version of the patch.
Please find attached an updated version of the patch.
Best regards,
Etsuro Fujita
*** 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"
***************
*** 93,99 **** PG_FUNCTION_INFO_V1(file_fdw_validator);
/*
* 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);
***************
*** 406,432 **** get_file_fdw_attribute_options(Oid relid)
/*
* 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,439 ----
/*
* filePlanForeignScan
! * Create the (single) path for a scan on the foreign table
*/
! static void
filePlanForeignScan(Oid foreigntableid,
PlannerInfo *root,
RelOptInfo *baserel)
{
! ForeignPath *pathnode = makeNode(ForeignPath);
char *filename;
List *options;
+ pathnode->path.pathtype = T_ForeignScan;
+ pathnode->path.parent = baserel;
+ pathnode->path.pathkeys = NIL; /* result is always unordered */
+ pathnode->path.required_outer = NULL;
+ pathnode->path.param_clauses = NIL;
+ pathnode->fdw_private = NIL; /* not used */
+
/* Fetch options --- we only need filename at this point */
fileGetOptions(foreigntableid, &filename, &options);
! /* Estimate costs of scanning the foreign table */
estimate_costs(root, baserel, filename,
! &pathnode->path.startup_cost,
&pathnode->path.total_cost);
! pathnode->path.rows = baserel->rows;
! add_path(baserel, (Path *) pathnode);
}
/*
*** 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,114 ----
<para>
<programlisting>
! void
PlanForeignScan (Oid foreigntableid,
PlannerInfo *root,
RelOptInfo *baserel);
</programlisting>
! Create the 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 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 multiple
! access paths for a scan on the foreign table and add them to
! <literal>baserel->pathlist</> using <function>add_path</>. Each
! access path generated should contain 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>
***************
*** 159,167 **** 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</>).
</para>
<para>
--- 165,172 ----
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>
***************
*** 228,236 **** 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.
</para>
</sect1>
--- 233,241 ----
</para>
<para>
! The <structname>FdwRoutine</> struct type is declared
! in <filename>src/include/foreign/fdwapi.h</>, which see for additional
! details.
</para>
</sect1>
*** a/src/backend/nodes/copyfuncs.c
--- b/src/backend/nodes/copyfuncs.c
***************
*** 591,611 **** _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;
--- 591,596 ----
***************
*** 3841,3849 **** 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;
--- 3826,3831 ----
*** a/src/backend/nodes/outfuncs.c
--- b/src/backend/nodes/outfuncs.c
***************
*** 558,573 **** _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);
}
--- 558,563 ----
***************
*** 1572,1578 **** _outForeignPath(StringInfo str, const ForeignPath *node)
_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
***************
*** 2744,2752 **** _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;
--- 2734,2739 ----
*** a/src/backend/optimizer/path/allpaths.c
--- b/src/backend/optimizer/path/allpaths.c
***************
*** 399,411 **** set_foreign_size(PlannerInfo *root, RelOptInfo *rel,
RangeTblEntry *rte)
/*
* 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);
--- 399,411 ----
/*
* set_foreign_pathlist
! * Build one or more access paths for a foreign table RTE
*/
static void
set_foreign_pathlist(PlannerInfo *root, RelOptInfo *rel, RangeTblEntry *rte)
{
! /* Generate appropriate paths */
! create_foreignscan_path(root, rel);
/* Select cheapest path (pretty easy in this case...) */
set_cheapest(rel);
*** a/src/backend/optimizer/plan/createplan.c
--- b/src/backend/optimizer/plan/createplan.c
***************
*** 121,127 **** 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);
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,
***************
*** 1847,1853 **** create_foreignscan_plan(PlannerInfo *root, ForeignPath
*best_path,
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);
***************
*** 3189,3195 **** make_foreignscan(List *qptlist,
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;
***************
*** 3201,3207 **** make_foreignscan(List *qptlist,
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;
}
*** a/src/backend/optimizer/util/pathnode.c
--- b/src/backend/optimizer/util/pathnode.c
***************
*** 1764,1803 **** create_worktablescan_path(PlannerInfo *root, RelOptInfo *rel)
/*
* 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,1786 ----
/*
* create_foreignscan_path
! * Creates one or more paths corresponding to a scan of a foreign table,
! * returning void.
*/
! void
create_foreignscan_path(PlannerInfo *root, RelOptInfo *rel)
{
RangeTblEntry *rte;
FdwRoutine *fdwroutine;
/* Get FDW's callback info */
rte = planner_rt_fetch(rel->relid, root);
fdwroutine = GetFdwRoutineByRelId(rte->relid);
/* Let the FDW do its planning */
! fdwroutine->PlanForeignScan(rte->relid, root, rel);
! if (rel->pathlist == NIL)
! elog(ERROR, "could not devise a query plan for the given
query");
}
/*
*** a/src/include/foreign/fdwapi.h
--- b/src/include/foreign/fdwapi.h
***************
*** 20,58 **** struct ExplainState;
/*
- * 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);
*** a/src/include/nodes/nodes.h
--- b/src/include/nodes/nodes.h
***************
*** 62,68 **** typedef enum NodeTag
T_CteScan,
T_WorkTableScan,
T_ForeignScan,
- T_FdwPlan,
T_Join,
T_NestLoop,
T_MergeJoin,
--- 62,67 ----
*** a/src/include/nodes/plannodes.h
--- b/src/include/nodes/plannodes.h
***************
*** 468,475 **** typedef struct ForeignScan
{
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;
*** a/src/include/nodes/relation.h
--- b/src/include/nodes/relation.h
***************
*** 794,805 **** typedef struct TidPath
/*
* 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;
/*
*** a/src/include/optimizer/pathnode.h
--- b/src/include/optimizer/pathnode.h
***************
*** 68,74 **** extern Path *create_functionscan_path(PlannerInfo *root,
RelOptInfo *rel);
extern Path *create_valuesscan_path(PlannerInfo *root, RelOptInfo *rel);
extern Path *create_ctescan_path(PlannerInfo *root, RelOptInfo *rel);
extern Path *create_worktablescan_path(PlannerInfo *root, RelOptInfo *rel);
! extern ForeignPath *create_foreignscan_path(PlannerInfo *root, RelOptInfo
*rel);
extern Relids calc_nestloop_required_outer(Path *outer_path, Path
*inner_path);
extern Relids calc_non_nestloop_required_outer(Path *outer_path, Path
*inner_path);
--- 68,74 ----
extern Path *create_valuesscan_path(PlannerInfo *root, RelOptInfo *rel);
extern Path *create_ctescan_path(PlannerInfo *root, RelOptInfo *rel);
extern Path *create_worktablescan_path(PlannerInfo *root, RelOptInfo *rel);
! extern void create_foreignscan_path(PlannerInfo *root, RelOptInfo *rel);
extern Relids calc_nestloop_required_outer(Path *outer_path, Path
*inner_path);
extern Relids calc_non_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
