On 21.01.2011 17:57, Robert Haas wrote:
On Fri, Jan 21, 2011 at 10:17 AM, Heikki Linnakangas
<heikki.linnakan...@enterprisedb.com>  wrote:
On 18.01.2011 17:26, Shigeru HANADA wrote:

1) 20110118-no_fdw_perm_check.patch - This patch is not included in
last post.  This had been proposed on 2011-01-05 first, but maybe has
not been reviewd yet.  I re-propose this patch for SQL standard
conformance.  This patch removes permission check that requires USAGE
on the foreign-data wrapper at CREATE FOREIGN TABLE.
Please see original post for details.

http://archives.postgresql.org/message-id/20110105145206.30fd.69899...@metrosystems.co.jp

Committed this part.

How much review have you done of parts (3) and (4)?  The key issue for
all of the FDW work in progress seems to be what the handler API is
going to look like, and so once we get that committed it will unblock
a lot of other things.

I've gone through the code in a bit more detail now. I did a bunch of cosmetic changes along the way, patch attached. I also added a few paragraphs in the docs. We need more extensive documentation, but this at least marks the places where I think the docs need to go.

Comments:

* How can a FDW fetch only the columns required by the scan? The file FDW has to read the whole file anyhow, but it could perhaps skip calling the input function for unnecessary columns. But more importantly, with something like the postgresql_fdw you don't want to fetch any extra columns across the wire. I gather the way to do it is to copy RelOptInfo->attr_needed to private storage at plan stage, and fill the not-needed attributes with NULLs in Iterate. That gets a bit awkward, you need to transform attr_needed to something that can be copied for starters. Or is that information somehow available at execution phase otherwise?

* I think we need something in RelOptInfo to mark foreign tables. At the moment, you need to call IsForeignTable() which does a catalog lookup. Maybe a new RTEKind, or a boolean flag.

* Can/should we make ReScan optional? Could the executor just do EndScan+BeginScan if there's no ReScan function?

* Is there any point in allowing a FDW without a handler? It's totally useless, isn't it? We had the CREATE FOREIGN DATA WRAPPER syntax in previous versions, and it allowed it, but it has always been totally useless so I don't think we need to worry much about backwards-compatibility here.

* Is there any use case for changing the handler or validator function of an existign FDW with ALTER? To me it just seems like an unnecessary complication.

* IMHO the "FDW-info" should always be displayed, without VERBOSE. In my experience with another DBMS that had this feature, the SQL being sent to the remote server was almost always the key piece of information that I was looking for in the query plans.

* this check in expand_inherited_rtentry seems misplaced:

                /*
                 * SELECT FOR UPDATE/SHARE is not allowed on foreign tables 
because
                 * they are read-only.
                 */
                if (newrelation->rd_rel->relkind == RELKIND_FOREIGN_TABLE &&
                        lockmode != AccessShareLock)
                        ereport(ERROR,
                                        (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                                         errmsg("SELECT FOR UPDATE/SHARE is not 
allowed with foreign tables")));

I don't understand why we'd need to do that for inherited tables in particular. And it's not working for regular non-inherited foreign tables:

postgres=# SELECT * FROM filetbl2 FOR UPDATE;
ERROR:  could not open file "base/11933/16397": No such file or directory

* Need to document how the FDW interacts with transaction commit/rollback. In particular, I believe EndScan is never called if the transaction is aborted. That needs to be noted explicitly, and need to suggest how to clean up any external resources in that case. For example, postgresql_fdw will need to close any open cursors or result sets.

In general, I think the design is sound. What we need is more documentation. It'd also be nice to see the postgresql_fdw brought back to shape so that it works against this latest version of the api patch.

--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com
diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index a65b4bc..06a82a4 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -2986,6 +2986,41 @@ ANALYZE measurement;
   </sect2>
  </sect1>
 
+ <sect1 id="ddl-foreign-data">
+  <title>Foreign Data</title>
+   <indexterm>
+    <primary>foreign data</primary>
+   </indexterm>
+   <para>
+    <productname>PostgreSQL</productname> implements parts of the SQL/MED
+    specification, which allows you to access external data that resides
+    outside PostgreSQL, using normal SQL queries.
+   </para>
+
+   <para>
+    Foreign data is accessed with help from a
+    <firstterm>foreign data wrapper</firstterm>. A foreign data wrapper is a
+    library that can communicate with an external data source, hiding the
+    details of connecting to the data source and fetching data from it. There
+    are several foreign data wrappers available for e.g reading files residing
+    on the server, or to connect to another PostgreSQL instance. If none of
+    the existing foreign data wrappers suite your needs, you can write your
+    own, see <xref linkend="fdwhandler">.
+   </para>
+
+   <para>
+    To access foreign data, you need to create a foreign server to define
+    the connection details to the external data source, using options required
+    by the foreign data wrapper. Then you need to create one or more
+    <firstterm>foreign tables</firstterm> that define the structure of the
+    remote data. A foreign table can be used in queries just like a normal
+    table, but a foreign table has no storage in the PostgreSQL server.
+    Whenever it is used, PostgreSQL asks the foreign data wrapper to fetch
+    the data from the external source.
+   </para>
+
+ </sect1>
+
  <sect1 id="ddl-others">
   <title>Other Database Objects</title>
 
diff --git a/doc/src/sgml/fdwhandler.sgml b/doc/src/sgml/fdwhandler.sgml
new file mode 100644
index 0000000..8ef93bc
--- /dev/null
+++ b/doc/src/sgml/fdwhandler.sgml
@@ -0,0 +1,125 @@
+<!-- doc/src/sgml/fdwhandler.sgml -->
+
+ <chapter id="fdwhandler">
+   <title>Writing A Foreign Data Wrapper</title>
+
+   <indexterm zone="fdwhandler">
+    <primary>foreign data wrapper</primary>
+    <secondary>handler for</secondary>
+   </indexterm>
+
+   <para>
+    All operations on foreign tables are handled through a foreign data
+    wrapper, which consists of a set of functions that the planner and
+    executor call. The foreign data wrapper is responsible for fetching
+    data from the remote data source and returning it to the PostgreSQL
+    executor. This chapter outlines how to write a new foreign data wrapper.
+   </para>
+
+   <para>
+    The FDW provider needs to implement a handler function, and (optionally)
+    a validator function. Both functions must be written in a compiled
+    language such as C, using the version-1 interface.
+
+    The validator function must be registered with
+    <productname>PostgreSQL</productname> as taking two arguments, a text
+    array containing the options, and an oid representing the type of object
+    the options are to be associated with. The handler function must be
+    registered as taking no arguments, and returning the type
+    <type>fdw_handler</type>.  This special pseudotype identifies the
+    function as a fdw handler and prevents it from being called directly in
+    SQL commands. For more details on C language calling conventions and
+    dynamic loading, see <xref linkend="xfunc-c">.
+   </para>
+
+   <para>
+    The validator function is responsible for validating
+    options given in the CREATE FOREIGN DATA WRAPPER, CREATE SERVER and
+    CREATE FOREIGN TABLE commands.
+   </para>
+
+   <para>
+    The handler function returns a struct of function pointers to callback
+    functions that are called by the planner and executor.
+   </para>
+
+   <para>
+    The SQL standard specifies an interface for writing foreign data wrappers.
+    However, PostgreSQL does not implement that API, because the effort to
+    accommodate it into PostgreSQL would be large. The standard API hasn't
+    gained wide adoption anyway, there aren't many standard-compliant foreign
+    data wrappers out there.
+   </para>
+
+   <para>
+    The foreign data wrappers included in the standard distribution
+    are good references when trying to write your own.
+    Look into the <filename>contrib/file_fdw</> subdirectory of the source tree.
+    The <xref linkend="sql-createforeigndatawrapper">
+    reference page also has some useful details.
+   </para>
+
+   <sect1 id="fdw-routines">
+    <title>Foreign Data Wrapper routines</title>
+
+    <para>
+     The FDW handler function returns a FdwRoutine struct with the following
+     callback functions:
+    </para>
+
+    <para>
+<programlisting>
+FdwPlan *
+PlanRelScan (Oid foreigntableid,
+             PlannerInfo *root,
+             RelOptInfo *baserel);
+</programlisting>
+   Plan a scan on a foreign table. This is called when a query is planned.
+   The function must return a palloc'd struct containing cost estimates,
+   a string to show for this scan in EXPLAIN, and any FDW-private information
+   that is needed to execute the foreign scan at a later stage.
+   A prepared plan may be executed many times.
+    </para>
+
+    <para>
+<programlisting>
+FdwExecutionState *
+BeginScan (FdwPlan *plan
+           ParamListInfo *params);
+</programlisting>
+   Begin exeucuting a foreign scan. This is called when a query is executed.
+   The function must return a palloc'd struct containing any private
+   information needed by the Iterate and EndScan calls that follow.
+    </para>
+
+    <para>
+<programlisting>
+void
+Iterate (FdwExecutionState *state,
+         TupleTableSlot *slot);
+</programlisting>
+   Fetch one row from the foreign source. Note that this is called in a
+   short-lived memory context that may be reset between every invocation of
+   Iterate. Create a memory context in BeginScan if you need longer-lived
+   storage.
+    </para>
+
+    <para>
+<programlisting>
+void
+ReScan (FdwExecutionState *state);
+</programlisting>
+   Restarts the scan.
+    </para>
+
+    <para>
+<programlisting>
+void
+EndScan (FdwExecutionState *state);
+</programlisting>
+   Ends the scan.
+    </para>
+
+   </sect1>
+
+ </chapter>
diff --git a/doc/src/sgml/filelist.sgml b/doc/src/sgml/filelist.sgml
index 99437ac..c2f4169 100644
--- a/doc/src/sgml/filelist.sgml
+++ b/doc/src/sgml/filelist.sgml
@@ -82,10 +82,11 @@
 <!entity geqo       SYSTEM "geqo.sgml">
 <!entity gist       SYSTEM "gist.sgml">
 <!entity gin        SYSTEM "gin.sgml">
-<!entity planstats    SYSTEM "planstats.sgml">
+<!entity planstats  SYSTEM "planstats.sgml">
 <!entity indexam    SYSTEM "indexam.sgml">
 <!entity nls        SYSTEM "nls.sgml">
 <!entity plhandler  SYSTEM "plhandler.sgml">
+<!entity fdwhandler SYSTEM "fdwhandler.sgml">
 <!entity protocol   SYSTEM "protocol.sgml">
 <!entity sources    SYSTEM "sources.sgml">
 <!entity storage    SYSTEM "storage.sgml">
diff --git a/doc/src/sgml/postgres.sgml b/doc/src/sgml/postgres.sgml
index 4d32f7d..98d19a5 100644
--- a/doc/src/sgml/postgres.sgml
+++ b/doc/src/sgml/postgres.sgml
@@ -238,6 +238,7 @@
   &sources;
   &nls;
   &plhandler;
+  &fdwhandler;
   &geqo;
   &indexam;
   &gist;
diff --git a/src/backend/commands/foreigncmds.c b/src/backend/commands/foreigncmds.c
index 360016c..6acdc9c 100644
--- a/src/backend/commands/foreigncmds.c
+++ b/src/backend/commands/foreigncmds.c
@@ -330,6 +330,9 @@ lookup_fdw_validator_func(DefElem *validator)
 	/* return value is ignored, so we don't check the type */
 }
 
+/*
+ * Convert a handler function name passed from the parser to an Oid
+ */
 static Oid
 lookup_fdw_handler_func(DefElem *handler)
 {
@@ -365,19 +368,21 @@ parse_func_options(List *func_options, DefElem **validator, DefElem **handler)
 		if (pg_strcasecmp(def->defname, "validator") == 0)
 		{
 			if (*validator)
-				elog(ERROR, "duplicated VALIDATOR");
+				ereport(ERROR,
+						(errcode(ERRCODE_SYNTAX_ERROR),
+						 errmsg("conflicting or redundant options")));
 			*validator = def;
 		}
 		else if (pg_strcasecmp(def->defname, "handler") == 0)
 		{
 			if (*handler)
-				elog(ERROR, "duplicated HANDLER");
+				ereport(ERROR,
+						(errcode(ERRCODE_SYNTAX_ERROR),
+						 errmsg("conflicting or redundant options")));
 			*handler = def;
 		}
 		else
-		{
-			elog(ERROR, "invalid option");
-		}
+			elog(ERROR, "option \"%s\" not recognized", def->defname);
 	}
 }
 
diff --git a/src/backend/executor/execAmi.c b/src/backend/executor/execAmi.c
index b27de28..9f7b138 100644
--- a/src/backend/executor/execAmi.c
+++ b/src/backend/executor/execAmi.c
@@ -21,8 +21,8 @@
 #include "executor/nodeBitmapIndexscan.h"
 #include "executor/nodeBitmapOr.h"
 #include "executor/nodeCtescan.h"
-#include "executor/nodeFunctionscan.h"
 #include "executor/nodeForeignscan.h"
+#include "executor/nodeFunctionscan.h"
 #include "executor/nodeGroup.h"
 #include "executor/nodeGroup.h"
 #include "executor/nodeHash.h"
diff --git a/src/backend/executor/execProcnode.c b/src/backend/executor/execProcnode.c
index 92b83ac..f275b41 100644
--- a/src/backend/executor/execProcnode.c
+++ b/src/backend/executor/execProcnode.c
@@ -85,8 +85,8 @@
 #include "executor/nodeBitmapIndexscan.h"
 #include "executor/nodeBitmapOr.h"
 #include "executor/nodeCtescan.h"
-#include "executor/nodeFunctionscan.h"
 #include "executor/nodeForeignscan.h"
+#include "executor/nodeFunctionscan.h"
 #include "executor/nodeGroup.h"
 #include "executor/nodeHash.h"
 #include "executor/nodeHashjoin.h"
diff --git a/src/backend/executor/nodeForeignscan.c b/src/backend/executor/nodeForeignscan.c
index e6354c7..3183f43 100644
--- a/src/backend/executor/nodeForeignscan.c
+++ b/src/backend/executor/nodeForeignscan.c
@@ -1,7 +1,7 @@
 /*-------------------------------------------------------------------------
  *
  * nodeForeignscan.c
- *	  Support routines for sequential scans of foreign tables.
+ *	  Support routines for scans of foreign tables.
  *
  * Portions Copyright (c) 1996-2011, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
@@ -14,9 +14,9 @@
  */
 /*
  * INTERFACE ROUTINES
- *		ExecForeignScan				sequentially scans a foreign table.
- *		ExecForeignNext				retrieve next tuple in sequential order.
- *		ExecInitForeignScan			creates and initializes a seqscan node.
+ *		ExecForeignScan				scans a foreign table.
+ *		ExecForeignNext				retrieve next tuple.
+ *		ExecInitForeignScan			creates and initializes a foreign scan node.
  *		ExecEndForeignScan			releases any storage allocated.
  *		ExecForeignReScan			rescans the foreign table
  *		ExecForeignMarkPos			marks scan position
@@ -76,6 +76,8 @@ ForeignNext(ForeignScanState *node)
 			ExecMaterializeSlot(slot);
 
 		/* overwrite only tableoid of the tuple */
+		/* XXX: Isn't this a bit dirty? We're scribbling over the field in 
+		 * the heap tuple that might've been allocated by the FDW. */
 		slot->tts_tuple->t_tableOid =
 							RelationGetRelid(node->ss.ss_currentRelation);
 	}
@@ -180,8 +182,8 @@ ExecInitForeignScan(ForeignScan *node, EState *estate, int eflags)
 	scanstate->routine = routine;
 
 	/*
-	 * If this execution was not for EXPLAIN w/o ANALYZE flag, initiate the
-	 * foreign scan.
+	 * Tell the FDW to initiate the foreign scan, unless we're just doing
+	 * EXPLAIN (ie, aren't going to run the plan).
 	 */
 	if (!(eflags & EXEC_FLAG_EXPLAIN_ONLY))
 	{
diff --git a/src/backend/foreign/foreign.c b/src/backend/foreign/foreign.c
index 1fdd925..3da8541 100644
--- a/src/backend/foreign/foreign.c
+++ b/src/backend/foreign/foreign.c
@@ -263,10 +263,7 @@ GetForeignTable(Oid relid)
 	Datum		datum;
 	bool		isnull;
 
-	tp = SearchSysCache(FOREIGNTABLEREL,
-						ObjectIdGetDatum(relid),
-						0, 0, 0);
-
+	tp = SearchSysCache1(FOREIGNTABLEREL, ObjectIdGetDatum(relid));
 	if (!HeapTupleIsValid(tp))
 		elog(ERROR, "cache lookup failed for foreign table %u", relid);
 
@@ -297,10 +294,10 @@ GetForeignTable(Oid relid)
 FdwRoutine *
 GetFdwRoutine(Oid fdwhandler)
 {
-	FmgrInfo				flinfo;
-	FunctionCallInfoData	fcinfo;
-	Datum					result;
-	FdwRoutine			   *routine;
+	FmgrInfo	flinfo;
+	FunctionCallInfoData fcinfo;
+	Datum		result;
+	FdwRoutine *routine;
 
 	if (fdwhandler == InvalidOid)
 		elog(ERROR, "foreign-data wrapper has no handler");
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 79c71c4..61c8599 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -572,9 +572,6 @@ _copyFdwPlan(FdwPlan *from)
 {
 	FdwPlan *newnode = makeNode(FdwPlan);
 
-	/*
-	 * copy node superclass fields
-	 */
 	COPY_STRING_FIELD(explainInfo);
 	COPY_SCALAR_FIELD(startup_cost);
 	COPY_SCALAR_FIELD(total_cost);
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index dee46ba..4602ced 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -546,8 +546,8 @@ _outFdwPlan(StringInfo str, FdwPlan *node)
 	WRITE_NODE_TYPE("FDWPLAN");
 
 	WRITE_STRING_FIELD(explainInfo);
-    WRITE_FLOAT_FIELD(startup_cost, "%.2f");
-    WRITE_FLOAT_FIELD(total_cost, "%.2f");
+	WRITE_FLOAT_FIELD(startup_cost, "%.2f");
+	WRITE_FLOAT_FIELD(total_cost, "%.2f");
 	WRITE_NODE_FIELD(private);
 }
 
diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index ae2e134..c196ab6 100644
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -258,7 +258,7 @@ set_plain_rel_pathlist(PlannerInfo *root, RelOptInfo *rel, RangeTblEntry *rte)
 
 	if (IsForeignTable(rte->relid))
 	{
-		/* only foreign scan path is applyable to foreign table */
+		/* only foreign scan path is applicable to a foreign table */
 		add_path(rel, create_foreignscan_path(root, rel));
 	}
 	else
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 938b8af..53ae495 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -352,9 +352,9 @@ create_scan_plan(PlannerInfo *root, Path *best_path)
 
 		case T_ForeignScan:
 			plan = (Plan *) create_foreignscan_plan(root,
-													  best_path,
-													  tlist,
-													  scan_clauses);
+													best_path,
+													tlist,
+													scan_clauses);
 			break;
 
 		default:
diff --git a/src/backend/optimizer/prep/prepunion.c b/src/backend/optimizer/prep/prepunion.c
index 61f6d5f..2605efb 100644
--- a/src/backend/optimizer/prep/prepunion.c
+++ b/src/backend/optimizer/prep/prepunion.c
@@ -1238,13 +1238,14 @@ expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti)
 		}
 
 		/*
-		 * SELECT FOR UPDATE/SHARE is not allowd to foreign tables because
+		 * SELECT FOR UPDATE/SHARE is not allowed on foreign tables because
 		 * they are read-only.
 		 */
 		if (newrelation->rd_rel->relkind == RELKIND_FOREIGN_TABLE &&
 			lockmode != AccessShareLock)
-			ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-							errmsg("SELECT FOR UPDATE/SHARE is not allowed with foreign tables")));
+			ereport(ERROR,
+					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+					 errmsg("SELECT FOR UPDATE/SHARE is not allowed with foreign tables")));
 
 		/*
 		 * Build an RTE for the child, and attach to query's rangetable list.
diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c
index 134ec7e..d53f7aa 100644
--- a/src/backend/optimizer/util/pathnode.c
+++ b/src/backend/optimizer/util/pathnode.c
@@ -1435,7 +1435,7 @@ create_foreignscan_path(PlannerInfo *root, RelOptInfo *rel)
 	pathnode->path.pathtype = T_ForeignScan;
 	pathnode->path.parent = rel;
 	pathnode->path.pathkeys = NIL;	/* result is always unordered */
-	
+
 	rte = planner_rt_fetch(rel->relid, root);
 	routine = GetFdwRoutineByRelId(rte->relid);
 	if (routine->PlanRelScan == NULL)
diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c
index 1d60af8..8d477a6 100644
--- a/src/backend/optimizer/util/plancat.c
+++ b/src/backend/optimizer/util/plancat.c
@@ -455,7 +455,7 @@ estimate_rel_size(Relation rel, int32 *attr_widths,
 			*tuples = 1;
 			break;
 		case RELKIND_FOREIGN_TABLE:
-			/* foreign tables has no storage, trust statistics  */
+			/* foreign tables have no storage, trust statistics  */
 			*pages = rel->rd_rel->relpages;
 			*tuples = rel->rd_rel->reltuples;
 			break;
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 2a381a0..23cc95a 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -5986,14 +5986,30 @@ getForeignDataWrappers(int *numForeignDataWrappers)
 	/* Make sure we are in proper schema */
 	selectSourceSchema("pg_catalog");
 
-	appendPQExpBuffer(query, "SELECT tableoid, oid, fdwname, "
-		"(%s fdwowner) AS rolname, fdwvalidator::pg_catalog.regproc, "
-		"fdwhandler::pg_catalog.regproc, fdwacl,"
-					  "array_to_string(ARRAY("
-		 "		SELECT option_name || ' ' || quote_literal(option_value) "
-	   "		FROM pg_options_to_table(fdwoptions)), ', ') AS fdwoptions "
-					  "FROM pg_foreign_data_wrapper",
-					  username_subquery);
+	if (g_fout->remoteVersion >= 90100)
+	{
+		appendPQExpBuffer(query, "SELECT tableoid, oid, fdwname, "
+						  "(%s fdwowner) AS rolname, "
+						  "fdwvalidator::pg_catalog.regproc, "
+						  "fdwhandler::pg_catalog.regproc, fdwacl,"
+						  "array_to_string(ARRAY("
+						  "		SELECT option_name || ' ' || quote_literal(option_value) "
+						  "		FROM pg_options_to_table(fdwoptions)), ', ') AS fdwoptions "
+						  "FROM pg_foreign_data_wrapper",
+						  username_subquery);
+	}
+	else
+	{
+		appendPQExpBuffer(query, "SELECT tableoid, oid, fdwname, "
+						  "(%s fdwowner) AS rolname, "
+						  "fdwvalidator::pg_catalog.regproc, "
+						  "0::oid AS fdwhandler, fdwacl,"
+						  "array_to_string(ARRAY("
+						  "		SELECT option_name || ' ' || quote_literal(option_value) "
+						  "		FROM pg_options_to_table(fdwoptions)), ', ') AS fdwoptions "
+						  "FROM pg_foreign_data_wrapper",
+						  username_subquery);
+	}
 
 	res = PQexec(g_conn, query->data);
 	check_sql_result(res, g_conn, query->data, PGRES_TUPLES_OK);
@@ -6026,7 +6042,6 @@ getForeignDataWrappers(int *numForeignDataWrappers)
 		fdwinfo[i].fdwoptions = strdup(PQgetvalue(res, i, i_fdwoptions));
 		fdwinfo[i].fdwacl = strdup(PQgetvalue(res, i, i_fdwacl));
 
-
 		/* Decide whether we want to dump it */
 		selectDumpableObject(&(fdwinfo[i].dobj));
 	}
diff --git a/src/include/foreign/fdwapi.h b/src/include/foreign/fdwapi.h
index 7be1999..9afdbe7 100644
--- a/src/include/foreign/fdwapi.h
+++ b/src/include/foreign/fdwapi.h
@@ -21,7 +21,7 @@
  * context with copyObject. It means that FdwPlan, a part of ForeignScan plan
  * node, and its contents must have copyObject support too.
  */
-struct FdwPlan
+typedef struct
 {
 	NodeTag type;
 
@@ -46,20 +46,18 @@ struct FdwPlan
 #endif
 
 	/*
-	 * FDW-private data. FDW must guarantee that every elements in this list
-	 * have copyObject support.  If FDW needs to store arbitrary data such as
-	 * non-Node structure, Const of bytea would be able to use as a container.
+	 * FDW-private data. Anything stored here needs to have copyObject
+	 * support.  If FDW needs to store arbitrary data such as a non-Node
+	 * structure, Const of bytea can be used as a container.
 	 */
-	List *private;
-};
-typedef struct FdwPlan FdwPlan;
+	Node *private;
+} FdwPlan;
 
-struct FdwExecutionState
+typedef struct
 {
-	/* FDW-private data */
+	/* FDW-private data. This doesn't need to have copyObject support. */
 	void *private;
-};
-typedef struct FdwExecutionState FdwExecutionState;
+} FdwExecutionState;
 
 /*
  * Common interface routines of FDW, inspired by the FDW API in the SQL/MED
@@ -72,7 +70,7 @@ typedef struct FdwExecutionState FdwExecutionState;
  * with BeginScan. The implementation should fill in the cost estimates in
  * FdwPlan, and may store private information.
  */
-struct FdwRoutine
+typedef struct
 {
 	/*
 	 * Plan a scan on a foreign table. 'foreigntableid' identifies the foreign
@@ -144,7 +142,6 @@ struct FdwRoutine
 	 */
 	FdwPlan *(*PlanQuery)(PlannerInfo *root, Query query);
 #endif
-};
-typedef struct FdwRoutine FdwRoutine;
+} FdwRoutine;
 
 #endif   /* FDWAPI_H */
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 7d1c681..561d512 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -1415,7 +1415,7 @@ typedef struct ForeignScanState
 {
 	ScanState		ss;			/* its first field is NodeTag */
 	FdwRoutine	   *routine;
-	FdwExecutionState *fstate;	/* private data for each data wrapper */
+	FdwExecutionState *fstate;	/* private state of the wrapper */
 } ForeignScanState;
 
 /* ----------------------------------------------------------------
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to