(2012/03/09 14:00), Tom Lane wrote:
I wrote:
There are a couple of other points that make me think we need to revisit
the PlanForeignScan API definition some more, too. ...
So we need to break down what PlanForeignScan currently does into three
separate steps. The first idea that comes to mind is to call them
GetForeignRelSize, GetForeignPaths, GetForeignPlan; but maybe somebody
has a better idea for names?
Attached is a draft patch for that.
1. FilefdwPlanState.pages and FileFdwPlanState.ntuples seems redundant.
Why not use RelOptInfo.pages and RelOptInfo.tuples?
2. IMHO RelOptInfo.fdw_private seems confusing. How about renaming it
to e.g., RelOptInfo.fdw_state?
Attached is a patch for the draft patch.
Best regards,
Etsuro Fujita
*** a/contrib/file_fdw/file_fdw.c
--- b/contrib/file_fdw/file_fdw.c
***************
*** 74,87 **** static const struct FileFdwOption valid_options[] = {
};
/*
! * FDW-specific information for RelOptInfo.fdw_private.
*/
typedef struct FileFdwPlanState
{
char *filename; /* file to read */
List *options; /* merged COPY options, excluding
filename */
- BlockNumber pages; /* estimate of file's physical
size */
- double ntuples; /* estimate of number of rows
in file */
} FileFdwPlanState;
/*
--- 74,85 ----
};
/*
! * FDW-specific information for RelOptInfo.fdw_state.
*/
typedef struct FileFdwPlanState
{
char *filename; /* file to read */
List *options; /* merged COPY options, excluding
filename */
} FileFdwPlanState;
/*
***************
*** 132,140 **** static void fileGetOptions(Oid foreigntableid,
char **filename, List **other_options);
static List *get_file_fdw_attribute_options(Oid relid);
static void estimate_size(PlannerInfo *root, RelOptInfo *baserel,
! FileFdwPlanState *fdw_private);
static void estimate_costs(PlannerInfo *root, RelOptInfo *baserel,
- FileFdwPlanState *fdw_private,
Cost *startup_cost, Cost *total_cost);
--- 130,137 ----
char **filename, List **other_options);
static List *get_file_fdw_attribute_options(Oid relid);
static void estimate_size(PlannerInfo *root, RelOptInfo *baserel,
! FileFdwPlanState *fpstate);
static void estimate_costs(PlannerInfo *root, RelOptInfo *baserel,
Cost *startup_cost, Cost *total_cost);
***************
*** 415,433 **** fileGetForeignRelSize(PlannerInfo *root,
RelOptInfo *baserel,
Oid foreigntableid)
{
! FileFdwPlanState *fdw_private;
/*
* Fetch options. We only need filename at this point, but we might
* as well get everything and not need to re-fetch it later in planning.
*/
! fdw_private = (FileFdwPlanState *) palloc(sizeof(FileFdwPlanState));
! fileGetOptions(foreigntableid,
! &fdw_private->filename,
&fdw_private->options);
! baserel->fdw_private = (void *) fdw_private;
/* Estimate relation size */
! estimate_size(root, baserel, fdw_private);
}
/*
--- 412,429 ----
RelOptInfo *baserel,
Oid foreigntableid)
{
! FileFdwPlanState *fpstate;
/*
* Fetch options. We only need filename at this point, but we might
* as well get everything and not need to re-fetch it later in planning.
*/
! fpstate = (FileFdwPlanState *) palloc(sizeof(FileFdwPlanState));
! fileGetOptions(foreigntableid, &fpstate->filename, &fpstate->options);
! baserel->fdw_state = (void *) fpstate;
/* Estimate relation size */
! estimate_size(root, baserel, fpstate);
}
/*
***************
*** 443,455 **** fileGetForeignPaths(PlannerInfo *root,
RelOptInfo *baserel,
Oid foreigntableid)
{
- FileFdwPlanState *fdw_private = (FileFdwPlanState *)
baserel->fdw_private;
Cost startup_cost;
Cost total_cost;
/* Estimate costs */
! estimate_costs(root, baserel, fdw_private,
! &startup_cost, &total_cost);
/* Create a ForeignPath node and add it as only possible path */
add_path(baserel, (Path *)
--- 439,449 ----
RelOptInfo *baserel,
Oid foreigntableid)
{
Cost startup_cost;
Cost total_cost;
/* Estimate costs */
! estimate_costs(root, baserel, &startup_cost, &total_cost);
/* Create a ForeignPath node and add it as only possible path */
add_path(baserel, (Path *)
***************
*** 647,659 **** fileReScanForeignScan(ForeignScanState *node)
/*
* Estimate size of a foreign table.
*
! * The main result is returned in baserel->rows. We also set
! * fdw_private->pages and fdw_private->ntuples for later use in the cost
! * calculation.
*/
static void
estimate_size(PlannerInfo *root, RelOptInfo *baserel,
! FileFdwPlanState *fdw_private)
{
struct stat stat_buf;
BlockNumber pages;
--- 641,652 ----
/*
* Estimate size of a foreign table.
*
! * The main result is returned in baserel->rows. We also set baserel->pages
! * and baserel->tuples for later use in the cost calculation.
*/
static void
estimate_size(PlannerInfo *root, RelOptInfo *baserel,
! FileFdwPlanState *fpstate)
{
struct stat stat_buf;
BlockNumber pages;
***************
*** 665,671 **** estimate_size(PlannerInfo *root, RelOptInfo *baserel,
* Get size of the file. It might not be there at plan time, though, in
* which case we have to use a default estimate.
*/
! if (stat(fdw_private->filename, &stat_buf) < 0)
stat_buf.st_size = 10 * BLCKSZ;
/*
--- 658,664 ----
* Get size of the file. It might not be there at plan time, though, in
* which case we have to use a default estimate.
*/
! if (stat(fpstate->filename, &stat_buf) < 0)
stat_buf.st_size = 10 * BLCKSZ;
/*
***************
*** 675,681 **** estimate_size(PlannerInfo *root, RelOptInfo *baserel,
if (pages < 1)
pages = 1;
! fdw_private->pages = pages;
/*
* Estimate the number of tuples in the file. We back into this
estimate
--- 668,674 ----
if (pages < 1)
pages = 1;
! baserel->pages = pages;
/*
* Estimate the number of tuples in the file. We back into this
estimate
***************
*** 688,694 **** estimate_size(PlannerInfo *root, RelOptInfo *baserel,
ntuples = clamp_row_est((double) stat_buf.st_size / (double)
tuple_width);
! fdw_private->ntuples = ntuples;
/*
* Now estimate the number of rows returned by the scan after applying
the
--- 681,687 ----
ntuples = clamp_row_est((double) stat_buf.st_size / (double)
tuple_width);
! baserel->tuples = ntuples;
/*
* Now estimate the number of rows returned by the scan after applying
the
***************
*** 715,725 **** estimate_size(PlannerInfo *root, RelOptInfo *baserel,
*/
static void
estimate_costs(PlannerInfo *root, RelOptInfo *baserel,
- FileFdwPlanState *fdw_private,
Cost *startup_cost, Cost *total_cost)
{
! BlockNumber pages = fdw_private->pages;
! double ntuples = fdw_private->ntuples;
Cost run_cost = 0;
Cost cpu_per_tuple;
--- 708,717 ----
*/
static void
estimate_costs(PlannerInfo *root, RelOptInfo *baserel,
Cost *startup_cost, Cost *total_cost)
{
! BlockNumber pages = baserel->pages;
! double ntuples = baserel->tuples;
Cost run_cost = 0;
Cost cpu_per_tuple;
*** a/doc/src/sgml/fdwhandler.sgml
--- b/doc/src/sgml/fdwhandler.sgml
***************
*** 126,132 **** GetForeignRelSize (PlannerInfo *root,
</para>
<para>
! <literal>baserel->fdw_private</> is a <type>void</> pointer that is
available for use by FDW planning functions. It can be used to pass
information forward from <function>GetForeignRelSize</> to
<function>GetForeignPaths</> and/or <function>GetForeignPaths</> to
--- 126,132 ----
</para>
<para>
! <literal>baserel->fdw_state</> is a <type>void</> pointer that is
available for use by FDW planning functions. It can be used to pass
information forward from <function>GetForeignRelSize</> to
<function>GetForeignPaths</> and/or <function>GetForeignPaths</> to
*** a/src/backend/optimizer/util/relnode.c
--- b/src/backend/optimizer/util/relnode.c
***************
*** 114,120 **** build_simple_rel(PlannerInfo *root, int relid, RelOptKind
reloptkind)
rel->subplan = NULL;
rel->subroot = NULL;
rel->fdwroutine = NULL;
! rel->fdw_private = NULL;
rel->baserestrictinfo = NIL;
rel->baserestrictcost.startup = 0;
rel->baserestrictcost.per_tuple = 0;
--- 114,120 ----
rel->subplan = NULL;
rel->subroot = NULL;
rel->fdwroutine = NULL;
! rel->fdw_state = NULL;
rel->baserestrictinfo = NIL;
rel->baserestrictcost.startup = 0;
rel->baserestrictcost.per_tuple = 0;
***************
*** 369,375 **** build_join_rel(PlannerInfo *root,
joinrel->subplan = NULL;
joinrel->subroot = NULL;
joinrel->fdwroutine = NULL;
! joinrel->fdw_private = NULL;
joinrel->baserestrictinfo = NIL;
joinrel->baserestrictcost.startup = 0;
joinrel->baserestrictcost.per_tuple = 0;
--- 369,375 ----
joinrel->subplan = NULL;
joinrel->subroot = NULL;
joinrel->fdwroutine = NULL;
! joinrel->fdw_state = NULL;
joinrel->baserestrictinfo = NIL;
joinrel->baserestrictcost.startup = 0;
joinrel->baserestrictcost.per_tuple = 0;
*** a/src/include/nodes/relation.h
--- b/src/include/nodes/relation.h
***************
*** 422,428 **** typedef struct RelOptInfo
PlannerInfo *subroot; /* if subquery */
/* use "struct FdwRoutine" to avoid including fdwapi.h here */
struct FdwRoutine *fdwroutine; /* if foreign table */
! void *fdw_private; /* if foreign table */
/* used by various scans and joins: */
List *baserestrictinfo; /* RestrictInfo structures (if
base
--- 422,428 ----
PlannerInfo *subroot; /* if subquery */
/* use "struct FdwRoutine" to avoid including fdwapi.h here */
struct FdwRoutine *fdwroutine; /* if foreign table */
! void *fdw_state; /* if foreign table */
/* used by various scans and joins: */
List *baserestrictinfo; /* RestrictInfo structures (if
base
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
