(2012/02/14 23:50), Tom Lane wrote:
> Shigeru Hanada<shigeru.han...@gmail.com> writes:
>> (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.
>
>> Multiple valuable Paths for a scan of a foreign table by FDW, but
>> changing PlanForeignScan to return list of FdwPlan in 9.2 seems too
>> hasty.
>
> 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.
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,447 ----
/*
* 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);
FdwPlan *fdwplan;
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;
+
/* Fetch options --- we only need filename at this point */
fileGetOptions(foreigntableid, &filename, &options);
/* Construct FdwPlan with cost estimates */
fdwplan = makeNode(FdwPlan);
+ fdwplan->fdw_private = NIL; /* not used */
estimate_costs(root, baserel, filename,
&fdwplan->startup_cost,
&fdwplan->total_cost);
! pathnode->fdwplan = fdwplan;
!
! /* Use costs estimated by FDW */
! pathnode->path.rows = baserel->rows;
! pathnode->path.startup_cost = fdwplan->startup_cost;
! pathnode->path.total_cost = fdwplan->total_cost;
!
! 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 the access path 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</>. 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>
*** 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/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,1784 ----
/*
* 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);
}
/*
*** a/src/include/foreign/fdwapi.h
--- b/src/include/foreign/fdwapi.h
***************
*** 52,58 **** typedef struct FdwPlan
* Callback function signatures --- see fdwhandler.sgml for more info.
*/
! typedef FdwPlan *(*PlanForeignScan_function) (Oid foreigntableid,
PlannerInfo *root,
RelOptInfo *baserel);
--- 52,58 ----
* Callback function signatures --- see fdwhandler.sgml for more info.
*/
! typedef void (*PlanForeignScan_function) (Oid foreigntableid,
PlannerInfo *root,
RelOptInfo *baserel);
*** 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
