From 1c9ffcd0c62f751e6d71fea868deefc8d9053f4b Mon Sep 17 00:00:00 2001 From: Shigeru Hanada Date: Wed, 23 Feb 2011 20:16:02 +0900 Subject: [PATCH] Fix postgresql_fdw to fit to new FDW API. --- contrib/postgresql_fdw/postgresql_fdw.c | 164 ++++-------------- contrib/postgresql_fdw/sql/postgresql_fdw.sql | 6 +- src/backend/foreign/foreign.c | 32 +++- 3 files changed, 69 insertions(+), 133 deletions(-) diff --git a/contrib/postgresql_fdw/postgresql_fdw.c b/contrib/postgresql_fdw/postgresql_fdw.c index e1dcd97185..547538ab4c 100644 --- a/contrib/postgresql_fdw/postgresql_fdw.c +++ b/contrib/postgresql_fdw/postgresql_fdw.c @@ -26,6 +26,7 @@ #include "nodes/relation.h" #include "optimizer/clauses.h" #include "optimizer/cost.h" +#include "optimizer/plancat.h" #include "parser/parsetree.h" #include "parser/scansup.h" #include "utils/builtins.h" @@ -53,10 +54,10 @@ static void pgReScanForeignScan(ForeignScanState *node); static void pgEndForeignScan(ForeignScanState *node); /* helper for deparsing a request into SQL statement */ +static void estimate_costs(PlannerInfo *root, RelOptInfo *baserel, Cost *startup_cost, Cost *total_cost); static bool is_foreign_qual(PlannerInfo *root, RelOptInfo *baserel, Expr *expr); static bool foreign_qual_walker(Node *node, void *context); -static void deparseSelectClause(StringInfo sql, ForeignTable *table, TupleDesc tupdesc, const char *aliasname, bool prefix); static char *deparseSql(Oid foreigntableid, PlannerInfo *root, RelOptInfo *baserel); static void storeResult(TupleTableSlot *slot, PGresult *res, int rowno); @@ -65,7 +66,9 @@ static void storeResult(TupleTableSlot *slot, PGresult *res, int rowno); * Connection management */ static PGconn *GetConnection(ForeignServer *server, UserMapping *user); +#ifdef NOT_USED static void ReleaseConnection(PGconn *conn); +#endif static void check_conn_params(const char **keywords, const char **values); static PGconn *connect_pg_server(ForeignServer *server, UserMapping *user); static void cleanup_connection(ResourceReleasePhase phase, @@ -114,6 +117,9 @@ pgPlanForeignScan(Oid foreigntableid, PlannerInfo *root, RelOptInfo *baserel) sql = deparseSql(foreigntableid, root, baserel); + estimate_costs(root, baserel, &fdwplan->startup_cost, + &fdwplan->total_cost); + fdwplan->fdw_private = lappend(NIL, makeString(sql)); return fdwplan; @@ -175,79 +181,6 @@ foreign_qual_walker(Node *node, void *context) return expression_tree_walker(node, foreign_qual_walker, context); } -/* - * Deparse the passed TupleDesc into SELECT clauses and append to the buffer - * 'sql'. - */ -static void -deparseSelectClause(StringInfo sql, ForeignTable *table, TupleDesc tupdesc, - const char *aliasname, bool prefix) -{ - bool first; - int i; - const char *aliasname_q; - - /* The alias of relation is used in both SELECT clause and FROM clause. */ - aliasname_q = quote_identifier(aliasname); - - /* deparse SELECT clause */ - appendStringInfoString(sql, "SELECT "); - - /* - * TODO: omit (deparse to "NULL") columns which are not used in the - * original SQL. - * - * We must parse nodes parents of this ForeignScan node to determine unused - * columns because some columns may be used only in parent Sort/Agg/Limit - * nodes. - */ - first = true; - for (i = 0; i < tupdesc->natts; i++) - { -#ifdef NOT_USED - List *options; - ListCell *lc; -#endif - char *colname = NULL; - - /* skip dropped attributes */ - if (tupdesc->attrs[i]->attisdropped) - continue; - - /* Determine column name to be used */ -#ifdef NOT_USED /* XXX: What was this all about? */ - options = GetGenericOptionsPerColumn(table->relid, i + 1); - foreach (lc, options) - { - DefElem *def = (DefElem *) lfirst(lc); - if (strcmp(def->defname, "colname") == 0) - { - colname = strVal(def->arg); - break; - } - } -#endif - if (!colname) - colname = tupdesc->attrs[i]->attname.data; - - if (!first) - appendStringInfoString(sql, ", "); - - if (prefix) - appendStringInfo(sql, "%s.%s", aliasname_q, colname); - else - appendStringInfo(sql, "%s", colname); - - first = false; - } - - /* if target list is composed only of system attributes, add dummy column */ - if (first) - appendStringInfoString(sql, "NULL"); - - if (aliasname_q != aliasname) - pfree((char *) aliasname_q); -} static void deparse_var(PlannerInfo *root, StringInfo buf, Var *var) @@ -426,6 +359,8 @@ deparseSql(Oid foreigntableid, PlannerInfo *root, RelOptInfo *baserel) first = false; deparse_var(root, &sql, var); } + if (strcmp(sql.data, "SELECT ") == 0) + appendStringInfo(&sql, "*"); /* * Deparse FROM @@ -552,7 +487,7 @@ pgBeginForeignScan(ForeignScanState *node, int eflags) relid = RelationGetRelid(node->ss.ss_currentRelation); table = GetForeignTable(relid); server = GetForeignServer(table->serverid); - user = GetUserMapping(table->serverid, GetOuterUserId()); + user = GetUserMapping(GetOuterUserId(), table->serverid); conn = GetConnection(server, user); festate->conn = conn; @@ -729,46 +664,6 @@ storeResult(TupleTableSlot *slot, PGresult *res, int rowno) pfree(values); } -#ifdef NOT_USED -/* - * Retrieve cost-factors of the foreign server from catalog. - */ -static void -get_server_costs(Oid relid, double *connection_cost, double *transfer_cost) -{ - ForeignTable *table; - ForeignServer *server; - int n; - const char **keywords; - const char **values; - int i; - - /* - * Retrieve generic options from the target table and its server to correct - * costs. - */ - table = GetForeignTable(relid); - server = GetForeignServer(table->serverid); - n = list_length(table->options) + list_length(server->options) + 1; - keywords = (const char **) palloc(sizeof(char *) * n); - values = (const char **) palloc(sizeof(char *) * n); - n = 0; - n += flatten_generic_options(server->options, keywords + n, values + n); - n += flatten_generic_options(table->options, keywords + n, values + n); - keywords[n] = values[n] = NULL; - - for (i = 0; keywords[i]; i++) - { - if (pg_strcasecmp(keywords[i], "connection_cost") == 0) - *connection_cost = strtod(values[i], NULL); - else if (pg_strcasecmp(keywords[i], "transfer_cost") == 0) - *transfer_cost = strtod(values[i], NULL); - } - - pfree(keywords); - pfree(values); -} -#endif /* * Estimate costs of scanning on a foreign table. @@ -776,9 +671,10 @@ get_server_costs(Oid relid, double *connection_cost, double *transfer_cost) * baserel->baserestrictinfo can be used to examine quals on the relation. */ static void -pgEstimateCosts(ForeignPath *path, PlannerInfo *root, RelOptInfo *baserel) +estimate_costs(PlannerInfo *root, RelOptInfo *baserel, Cost *startup_cost, Cost *total_cost) { RangeTblEntry *rte; + int width; double connection_cost = 0.0; double transfer_cost = 0.0; @@ -787,20 +683,17 @@ pgEstimateCosts(ForeignPath *path, PlannerInfo *root, RelOptInfo *baserel) /* * Use cost_seqscan() to get approximate value. */ - cost_seqscan(&path->path, root, baserel); - - /* Get cost factor from catalog to correct costs. */ rte = planner_rt_fetch(baserel->relid, root); -#ifdef NOT_USED - get_server_costs(rte->relid, &connection_cost, &transfer_cost); -#endif +// cost_seqscan(&path->path, root, baserel); + + width = get_relation_data_width(rte->relid, NULL); + /* XXX arbitrary guesses */ connection_cost = 10.0; transfer_cost = 1.0; - path->path.startup_cost += connection_cost; - path->path.total_cost += connection_cost; - path->path.total_cost += transfer_cost * - path->path.parent->width * path->path.parent->rows; + *startup_cost += connection_cost; + *total_cost += connection_cost; + *total_cost += transfer_cost * width * baserel->tuples; } /* ============================================================================ @@ -984,12 +877,25 @@ connect_pg_server(ForeignServer *server, UserMapping *user) for (i = 0, j = 0; all_keywords[i]; i++) { - /* Use only libpq connection options. */ - if (!is_libpq_connection_option(all_keywords[i])) + StringInfoData conninfo; + PQconninfoOption *opt; + + initStringInfo(&conninfo); + appendStringInfo(&conninfo, "%s=%s", all_keywords[i], all_values[i]); + opt = PQconninfoParse(conninfo.data, NULL); + + /* Use only valid libpq connection options. */ + if (opt == NULL) + { + PQconninfoFree(opt); continue; + } keywords[j] = all_keywords[i]; values[j] = all_values[i]; j++; + + PQconninfoFree(opt); + pfree(conninfo.data); } keywords[j] = values[j] = NULL; pfree(all_keywords); @@ -1009,6 +915,7 @@ connect_pg_server(ForeignServer *server, UserMapping *user) return conn; } +#ifdef NOT_USED /* * Mark the connection as "unused", and close it if the caller was the last * user of the connection. @@ -1054,6 +961,7 @@ ReleaseConnection(PGconn *conn) entry->conn = NULL; } } +#endif /* * Clean the connection up via ResourceOwner when pgClose couldn't close the diff --git a/contrib/postgresql_fdw/sql/postgresql_fdw.sql b/contrib/postgresql_fdw/sql/postgresql_fdw.sql index 291c7b6448..441a5b4457 100644 --- a/contrib/postgresql_fdw/sql/postgresql_fdw.sql +++ b/contrib/postgresql_fdw/sql/postgresql_fdw.sql @@ -47,12 +47,10 @@ COPY t2 FROM stdin; \. CREATE FOREIGN TABLE ft1 ( - c1 integer OPTIONS (colname 'invalid'), - c2 text OPTIONS (colname 'C2'), + c1 integer, + c2 text, c3 date ) SERVER loopback1 OPTIONS (relname 't1'); -ALTER FOREIGN TABLE ft1 ALTER COLUMN c1 OPTIONS (SET colname 'C1'); -ALTER FOREIGN TABLE ft1 ALTER COLUMN c2 OPTIONS (DROP colname); CREATE FOREIGN TABLE ft2 ( c1 integer, diff --git a/src/backend/foreign/foreign.c b/src/backend/foreign/foreign.c index 44cd18177c..37bbceee25 100644 --- a/src/backend/foreign/foreign.c +++ b/src/backend/foreign/foreign.c @@ -491,6 +491,35 @@ is_conninfo_option(const char *option, Oid context) } +/* + * Valid options for postgresql_fdw objects. + * + * The list is small - don't bother with bsearch if it stays so. + */ +static struct ConnectionOption fdw_options[] = { + {"relname", ForeignTableRelationId}, + {"nspname", ForeignTableRelationId}, + {NULL, InvalidOid} +}; + + +/* + * Check if the provided option is one of fdw options. + * context is the Oid of the catalog the option came from, or 0 if we + * don't care. + */ +static bool +is_fdw_option(const char *option, Oid context) +{ + struct ConnectionOption *opt; + + for (opt = fdw_options; opt->optname; opt++) + if (context == opt->optcontext && strcmp(opt->optname, option) == 0) + return true; + return false; +} + + /* * Validate the generic option given to SERVER or USER MAPPING. * Raise an ERROR if the option or its value is considered @@ -511,7 +540,8 @@ postgresql_fdw_validator(PG_FUNCTION_ARGS) { DefElem *def = lfirst(cell); - if (!is_conninfo_option(def->defname, catalog)) + if (!is_conninfo_option(def->defname, catalog) && + !is_fdw_option(def->defname, catalog)) { struct ConnectionOption *opt; StringInfoData buf; -- 2.39.5