Comments on SQL/Med objects

Started by Guillaume Lelargealmost 15 years ago11 messages
#1Guillaume Lelarge
guillaume@lelarge.info

Hi,

While working on adding support for SQL/Med objects to pgAdmin, I'm
quite surprised to see there is no way to add comments to SQL/Med
objects. Is this on purpose or is it just something that was simply missed?

Thanks.

--
Guillaume
http://www.postgresql.fr
http://dalibo.com

#2Robert Haas
robertmhaas@gmail.com
In reply to: Guillaume Lelarge (#1)
Re: Comments on SQL/Med objects

On Tue, Mar 22, 2011 at 6:23 PM, Guillaume Lelarge
<guillaume@lelarge.info> wrote:

While working on adding support for SQL/Med objects to pgAdmin, I'm
quite surprised to see there is no way to add comments to SQL/Med
objects. Is this on purpose or is it just something that was simply missed?

I think it's an oversight. We should probably fix this.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#2)
Re: Comments on SQL/Med objects

Robert Haas <robertmhaas@gmail.com> writes:

On Tue, Mar 22, 2011 at 6:23 PM, Guillaume Lelarge
<guillaume@lelarge.info> wrote:

While working on adding support for SQL/Med objects to pgAdmin, I'm
quite surprised to see there is no way to add comments to SQL/Med
objects. Is this on purpose or is it just something that was simply missed?

I think it's an oversight. We should probably fix this.

Yeah, I had a private TODO about that. I'd like to see if we can
refactor the grammar to eliminate some of the duplication there
as well as the potential for oversights of this sort. I believe
that USER MAPPINGs are missing from ObjectType as well as a bunch
of other basic places ...

regards, tom lane

#4Guillaume Lelarge
guillaume@lelarge.info
In reply to: Tom Lane (#3)
Re: Comments on SQL/Med objects

Le 23/03/2011 17:53, Tom Lane a �crit :

Robert Haas <robertmhaas@gmail.com> writes:

On Tue, Mar 22, 2011 at 6:23 PM, Guillaume Lelarge
<guillaume@lelarge.info> wrote:

While working on adding support for SQL/Med objects to pgAdmin, I'm
quite surprised to see there is no way to add comments to SQL/Med
objects. Is this on purpose or is it just something that was simply missed?

I think it's an oversight. We should probably fix this.

Yeah, I had a private TODO about that. I'd like to see if we can
refactor the grammar to eliminate some of the duplication there
as well as the potential for oversights of this sort. I believe
that USER MAPPINGs are missing from ObjectType as well as a bunch
of other basic places ...

OK, great. Thanks for your answers.

--
Guillaume
http://www.postgresql.fr
http://dalibo.com

#5Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#3)
Re: Comments on SQL/Med objects

On Wed, Mar 23, 2011 at 12:53 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Tue, Mar 22, 2011 at 6:23 PM, Guillaume Lelarge
<guillaume@lelarge.info> wrote:

While working on adding support for SQL/Med objects to pgAdmin, I'm
quite surprised to see there is no way to add comments to SQL/Med
objects. Is this on purpose or is it just something that was simply missed?

I think it's an oversight.  We should probably fix this.

Yeah, I had a private TODO about that.  I'd like to see if we can
refactor the grammar to eliminate some of the duplication there
as well as the potential for oversights of this sort.  I believe
that USER MAPPINGs are missing from ObjectType as well as a bunch
of other basic places ...

Are you going to work on this? If not I can pick it up, at least
insofar as making the comment stuff work across the board.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#5)
Re: Comments on SQL/Med objects

Robert Haas <robertmhaas@gmail.com> writes:

On Wed, Mar 23, 2011 at 12:53 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Yeah, I had a private TODO about that. �I'd like to see if we can
refactor the grammar to eliminate some of the duplication there
as well as the potential for oversights of this sort. �I believe
that USER MAPPINGs are missing from ObjectType as well as a bunch
of other basic places ...

Are you going to work on this? If not I can pick it up, at least
insofar as making the comment stuff work across the board.

I'm still up to my rear in collations, so feel free.

regards, tom lane

#7Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#6)
Re: Comments on SQL/Med objects

On Mon, Mar 28, 2011 at 12:06 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Wed, Mar 23, 2011 at 12:53 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Yeah, I had a private TODO about that.  I'd like to see if we can
refactor the grammar to eliminate some of the duplication there
as well as the potential for oversights of this sort.  I believe
that USER MAPPINGs are missing from ObjectType as well as a bunch
of other basic places ...

Are you going to work on this?  If not I can pick it up, at least
insofar as making the comment stuff work across the board.

I'm still up to my rear in collations, so feel free.

OK. I'll work on it this week.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#8Robert Haas
robertmhaas@gmail.com
In reply to: Robert Haas (#7)
1 attachment(s)
Re: Comments on SQL/Med objects

On Mon, Mar 28, 2011 at 12:29 AM, Robert Haas <robertmhaas@gmail.com> wrote:

On Mon, Mar 28, 2011 at 12:06 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Wed, Mar 23, 2011 at 12:53 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Yeah, I had a private TODO about that.  I'd like to see if we can
refactor the grammar to eliminate some of the duplication there
as well as the potential for oversights of this sort.  I believe
that USER MAPPINGs are missing from ObjectType as well as a bunch
of other basic places ...

Are you going to work on this?  If not I can pick it up, at least
insofar as making the comment stuff work across the board.

I'm still up to my rear in collations, so feel free.

OK.  I'll work on it this week.

Attached. Foreign tables are already OK, I believe; it's only foreign
data wrappers and foreign servers that appear to need fixing.

The fact that foreign data wrapper is sometimes abbreviated to fdw and
sometimes not does nothing for the greppability of the code. I'm
wondering if we should go through and fix the constants that
abbreviate it:

ACL_KIND_FDW
ACL_ALL_RIGHTS_FDW
OBJECT_FDW
OCLASS_FDW

It seems to me that it would be a whole lot clearer and easier if
these all spelled it out FOREIGN_DATA_WRAPPER, as we do for similar
object types. Other than a pretty minute back-patch hazard, I don't
see much down side. Thoughts?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Attachments:

foreign-comment.patchapplication/octet-stream; name=foreign-comment.patchDownload
diff --git a/doc/src/sgml/ref/comment.sgml b/doc/src/sgml/ref/comment.sgml
index bc848b3..1cdc49f 100644
--- a/doc/src/sgml/ref/comment.sgml
+++ b/doc/src/sgml/ref/comment.sgml
@@ -33,6 +33,7 @@ COMMENT ON
   DATABASE <replaceable class="PARAMETER">object_name</replaceable> |
   DOMAIN <replaceable class="PARAMETER">object_name</replaceable> |
   EXTENSION <replaceable class="PARAMETER">object_name</replaceable> |
+  FOREIGN DATA WRAPPER <replaceable class="PARAMETER">object_name</replaceable> |
   FOREIGN TABLE <replaceable class="PARAMETER">object_name</replaceable> |
   FUNCTION <replaceable class="PARAMETER">function_name</replaceable> ( [ [ <replaceable class="parameter">argmode</replaceable> ] [ <replaceable class="parameter">argname</replaceable> ] <replaceable class="parameter">argtype</replaceable> [, ...] ] ) |
   INDEX <replaceable class="PARAMETER">object_name</replaceable> |
@@ -45,6 +46,7 @@ COMMENT ON
   RULE <replaceable class="PARAMETER">rule_name</replaceable> ON <replaceable class="PARAMETER">table_name</replaceable> |
   SCHEMA <replaceable class="PARAMETER">object_name</replaceable> |
   SEQUENCE <replaceable class="PARAMETER">object_name</replaceable> |
+  SERVER <replaceable class="PARAMETER">object_name</replaceable> |
   TABLESPACE <replaceable class="PARAMETER">object_name</replaceable> |
   TEXT SEARCH CONFIGURATION <replaceable class="PARAMETER">object_name</replaceable> |
   TEXT SEARCH DICTIONARY <replaceable class="PARAMETER">object_name</replaceable> |
diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c
index 48fa6d4..7b3c63a 100644
--- a/src/backend/catalog/aclchk.c
+++ b/src/backend/catalog/aclchk.c
@@ -4589,6 +4589,33 @@ pg_ts_config_ownercheck(Oid cfg_oid, Oid roleid)
 }
 
 /*
+ * Ownership check for a foreign data wrapper (specified by OID).
+ */
+bool
+pg_foreign_data_wrapper_ownercheck(Oid srv_oid, Oid roleid)
+{
+	HeapTuple	tuple;
+	Oid			ownerId;
+
+	/* Superusers bypass all permission checking. */
+	if (superuser_arg(roleid))
+		return true;
+
+	tuple = SearchSysCache1(FOREIGNDATAWRAPPEROID, ObjectIdGetDatum(srv_oid));
+	if (!HeapTupleIsValid(tuple))
+		ereport(ERROR,
+				(errcode(ERRCODE_UNDEFINED_OBJECT),
+				 errmsg("foreign data wrapper with OID %u does not exist",
+						srv_oid)));
+
+	ownerId = ((Form_pg_foreign_data_wrapper) GETSTRUCT(tuple))->fdwowner;
+
+	ReleaseSysCache(tuple);
+
+	return has_privs_of_role(roleid, ownerId);
+}
+
+/*
  * Ownership check for a foreign server (specified by OID).
  */
 bool
diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c
index 880b95d..ca2dea0 100644
--- a/src/backend/catalog/objectaddress.c
+++ b/src/backend/catalog/objectaddress.c
@@ -30,6 +30,8 @@
 #include "catalog/pg_conversion.h"
 #include "catalog/pg_database.h"
 #include "catalog/pg_extension.h"
+#include "catalog/pg_foreign_data_wrapper.h"
+#include "catalog/pg_foreign_server.h"
 #include "catalog/pg_language.h"
 #include "catalog/pg_largeobject.h"
 #include "catalog/pg_largeobject_metadata.h"
@@ -140,6 +142,8 @@ get_object_address(ObjectType objtype, List *objname, List *objargs,
 		case OBJECT_ROLE:
 		case OBJECT_SCHEMA:
 		case OBJECT_LANGUAGE:
+		case OBJECT_FDW:
+		case OBJECT_FOREIGN_SERVER:
 			address = get_object_address_unqualified(objtype, objname);
 			break;
 		case OBJECT_TYPE:
@@ -295,6 +299,12 @@ get_object_address_unqualified(ObjectType objtype, List *qualname)
 			case OBJECT_LANGUAGE:
 				msg = gettext_noop("language name cannot be qualified");
 				break;
+			case OBJECT_FDW:
+				msg = gettext_noop("foreign data wrapper name cannot be qualified");
+				break;
+			case OBJECT_FOREIGN_SERVER:
+				msg = gettext_noop("server name cannot be qualified");
+				break;
 			default:
 				elog(ERROR, "unrecognized objtype: %d", (int) objtype);
 				msg = NULL;			/* placate compiler */
@@ -340,6 +350,16 @@ get_object_address_unqualified(ObjectType objtype, List *qualname)
 			address.objectId = get_language_oid(name, false);
 			address.objectSubId = 0;
 			break;
+		case OBJECT_FDW:
+			address.classId = ForeignDataWrapperRelationId;
+			address.objectId = get_foreign_data_wrapper_oid(name, false);
+			address.objectSubId = 0;
+			break;
+		case OBJECT_FOREIGN_SERVER:
+			address.classId = ForeignServerRelationId;
+			address.objectId = get_foreign_server_oid(name, false);
+			address.objectSubId = 0;
+			break;
 		default:
 			elog(ERROR, "unrecognized objtype: %d", (int) objtype);
 			/* placate compiler, which doesn't know elog won't return */
@@ -655,6 +675,12 @@ object_exists(ObjectAddress address)
 		case CastRelationId:
 			indexoid = CastOidIndexId;
 			break;
+		case ForeignDataWrapperRelationId:
+			cache = FOREIGNDATAWRAPPEROID;
+			break;
+		case ForeignServerRelationId:
+			cache = FOREIGNSERVEROID;
+			break;
 		case TSParserRelationId:
 			cache = TSPARSEROID;
 			break;
@@ -758,6 +784,11 @@ check_object_ownership(Oid roleid, ObjectType objtype, ObjectAddress address,
 				aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_EXTENSION,
 							   NameListToString(objname));
 			break;
+		case OBJECT_FDW:
+			if (!pg_foreign_data_wrapper_ownercheck(address.objectId, roleid))
+				aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_FDW,
+							   NameListToString(objname));
+			break;
 		case OBJECT_FOREIGN_SERVER:
 			if (!pg_foreign_server_ownercheck(address.objectId, roleid))
 				aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_FOREIGN_SERVER,
@@ -838,7 +869,6 @@ check_object_ownership(Oid roleid, ObjectType objtype, ObjectAddress address,
 							 errmsg("must have CREATEROLE privilege")));
 			}
 			break;
-		case OBJECT_FDW:
 		case OBJECT_TSPARSER:
 		case OBJECT_TSTEMPLATE:
 			/* We treat these object types as being owned by superusers */
diff --git a/src/backend/commands/foreigncmds.c b/src/backend/commands/foreigncmds.c
index acd40c1..c5cd51e 100644
--- a/src/backend/commands/foreigncmds.c
+++ b/src/backend/commands/foreigncmds.c
@@ -1395,3 +1395,42 @@ CreateForeignTable(CreateForeignTableStmt *stmt, Oid relid)
 
 	heap_close(ftrel, RowExclusiveLock);
 }
+
+/*
+ * get_foreign_data_wrapper_oid - given a FDW name, look up the OID
+ *
+ * If missing_ok is false, throw an error if name not found.  If true, just
+ * return InvalidOid.
+ */
+Oid
+get_foreign_data_wrapper_oid(const char *fdwname, bool missing_ok)
+{
+	Oid			oid;
+
+	oid = GetSysCacheOid1(FOREIGNDATAWRAPPERNAME, CStringGetDatum(fdwname));
+	if (!OidIsValid(oid) && !missing_ok)
+		ereport(ERROR,
+				(errcode(ERRCODE_UNDEFINED_OBJECT),
+				 errmsg("foreign data wrapper \"%s\" does not exist",
+						fdwname)));
+	return oid;
+}
+
+/*
+ * get_foreign_server_oid - given a FDW name, look up the OID
+ *
+ * If missing_ok is false, throw an error if name not found.  If true, just
+ * return InvalidOid.
+ */
+Oid
+get_foreign_server_oid(const char *servername, bool missing_ok)
+{
+	Oid			oid;
+
+	oid = GetSysCacheOid1(FOREIGNSERVERNAME, CStringGetDatum(servername));
+	if (!OidIsValid(oid) && !missing_ok)
+		ereport(ERROR,
+				(errcode(ERRCODE_UNDEFINED_OBJECT),
+				 errmsg("foreign server \"%s\" does not exist", servername)));
+	return oid;
+}
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 27fdcca..a22ab66 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -4787,11 +4787,12 @@ opt_restart_seqs:
  *	the object associated with the comment. The form of the statement is:
  *
  *	COMMENT ON [ [ DATABASE | DOMAIN | INDEX | SEQUENCE | TABLE | TYPE | VIEW |
- *				   COLLATION | CONVERSION | LANGUAGE | OPERATOR CLASS | LARGE OBJECT |
- *				   CAST | COLUMN | SCHEMA | TABLESPACE | EXTENSION | ROLE |
- *				   TEXT SEARCH PARSER | TEXT SEARCH DICTIONARY |
- *				   TEXT SEARCH TEMPLATE | TEXT SEARCH CONFIGURATION |
- *				   FOREIGN TABLE ] <objname> |
+ *				   COLLATION | CONVERSION | LANGUAGE | OPERATOR CLASS |
+ *				   LARGE OBJECT | CAST | COLUMN | SCHEMA | TABLESPACE |
+ *				   EXTENSION | ROLE | TEXT SEARCH PARSER |
+ *				   TEXT SEARCH DICTIONARY | TEXT SEARCH TEMPLATE |
+ *				   TEXT SEARCH CONFIGURATION | FOREIGN TABLE |
+ *				   FOREIGN DATA WRAPPER | SERVER ] <objname> |
  *				 AGGREGATE <aggname> (arg1, ...) |
  *				 FUNCTION <funcname> (arg1, arg2, ...) |
  *				 OPERATOR <op> (leftoperand_typ, rightoperand_typ) |
@@ -4971,6 +4972,8 @@ comment_type:
 			| EXTENSION 						{ $$ = OBJECT_EXTENSION; }
 			| ROLE								{ $$ = OBJECT_ROLE; }
 			| FOREIGN TABLE						{ $$ = OBJECT_FOREIGN_TABLE; }
+			| SERVER							{ $$ = OBJECT_FOREIGN_SERVER; }
+			| FOREIGN DATA_P WRAPPER			{ $$ = OBJECT_FDW; }
 		;
 
 comment_text:
diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h
index 157ee39..25c4018 100644
--- a/src/include/commands/defrem.h
+++ b/src/include/commands/defrem.h
@@ -167,6 +167,8 @@ extern Datum transformGenericOptions(Oid catalogId,
 									 Datum oldOptions,
 									 List *options,
 									 Oid fdwvalidator);
+extern Oid get_foreign_data_wrapper_oid(const char *fdwname, bool missing_ok);
+extern Oid get_foreign_server_oid(const char *servername, bool missing_ok);
 
 /* support routines in commands/define.c */
 
diff --git a/src/include/utils/acl.h b/src/include/utils/acl.h
index e96323e..b28b764 100644
--- a/src/include/utils/acl.h
+++ b/src/include/utils/acl.h
@@ -315,6 +315,7 @@ extern bool pg_collation_ownercheck(Oid coll_oid, Oid roleid);
 extern bool pg_conversion_ownercheck(Oid conv_oid, Oid roleid);
 extern bool pg_ts_dict_ownercheck(Oid dict_oid, Oid roleid);
 extern bool pg_ts_config_ownercheck(Oid cfg_oid, Oid roleid);
+extern bool pg_foreign_data_wrapper_ownercheck(Oid srv_oid, Oid roleid);
 extern bool pg_foreign_server_ownercheck(Oid srv_oid, Oid roleid);
 extern bool pg_extension_ownercheck(Oid ext_oid, Oid roleid);
 extern bool has_createrole_privilege(Oid roleid);
diff --git a/src/test/regress/expected/foreign_data.out b/src/test/regress/expected/foreign_data.out
index a747334..c05bcab 100644
--- a/src/test/regress/expected/foreign_data.out
+++ b/src/test/regress/expected/foreign_data.out
@@ -14,6 +14,7 @@ CREATE ROLE regress_test_role_super SUPERUSER;
 CREATE ROLE regress_test_indirect;
 CREATE ROLE unprivileged_role;
 CREATE FOREIGN DATA WRAPPER dummy;
+COMMENT ON FOREIGN DATA WRAPPER dummy IS 'useless';
 CREATE FOREIGN DATA WRAPPER postgresql VALIDATOR postgresql_fdw_validator;
 -- At this point we should have 2 built-in wrappers and no servers.
 SELECT fdwname, fdwhandler::regproc, fdwvalidator::regproc, fdwoptions FROM pg_foreign_data_wrapper ORDER BY 1, 2, 3;
@@ -211,6 +212,7 @@ DROP ROLE regress_test_role_super;
 
 CREATE FOREIGN DATA WRAPPER foo;
 CREATE SERVER s1 FOREIGN DATA WRAPPER foo;
+COMMENT ON SERVER s1 IS 'foreign server';
 CREATE USER MAPPING FOR current_user SERVER s1;
 \dew+
                                    List of foreign-data wrappers
diff --git a/src/test/regress/sql/foreign_data.sql b/src/test/regress/sql/foreign_data.sql
index 3f39785..0d12b98 100644
--- a/src/test/regress/sql/foreign_data.sql
+++ b/src/test/regress/sql/foreign_data.sql
@@ -21,6 +21,7 @@ CREATE ROLE regress_test_indirect;
 CREATE ROLE unprivileged_role;
 
 CREATE FOREIGN DATA WRAPPER dummy;
+COMMENT ON FOREIGN DATA WRAPPER dummy IS 'useless';
 CREATE FOREIGN DATA WRAPPER postgresql VALIDATOR postgresql_fdw_validator;
 
 -- At this point we should have 2 built-in wrappers and no servers.
@@ -99,6 +100,7 @@ DROP ROLE regress_test_role_super;
 
 CREATE FOREIGN DATA WRAPPER foo;
 CREATE SERVER s1 FOREIGN DATA WRAPPER foo;
+COMMENT ON SERVER s1 IS 'foreign server';
 CREATE USER MAPPING FOR current_user SERVER s1;
 \dew+
 \des+
#9Shigeru HANADA
hanada@metrosystems.co.jp
In reply to: Robert Haas (#8)
4 attachment(s)
Re: Comments on SQL/Med objects

On Thu, 31 Mar 2011 11:24:27 -0400
Robert Haas <robertmhaas@gmail.com> wrote:

Attached. Foreign tables are already OK, I believe; it's only foreign
data wrappers and foreign servers that appear to need fixing.

The patch seems good for basic functionarity. I've tested the patch
and noticed that get_foreign_data_wrapper_oid() is same as
GetForeignDataWrapperOidByName(), so they could be merged. Also
GetForeignServerOidByName() could be merged.

I changed "foreign data wrapper" in message to "foreign-data wrapper"
for consistency, but it's revertable.

Please see merge_oid_funcs.patch which can be applied onto your patch.

I think some supports can be added for comments on SQL/MED objects.

- pg_dump support for comment on fdw and server
- psql describe commands (\dew+ and \des+)
- psql TAB completion

Please see attached patches for each feature.

While testing pg_dump, I noticed that comment of extension's member
objects are not dumped by pg_dump. Those comments should be dumped
after CREATE EXTENSION statement?

Regards,
--
Shigeru Hanada

Attachments:

20110401_merge_oid_funcs.patchapplication/octet-stream; name=20110401_merge_oid_funcs.patchDownload
diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c
index 7b3c63a..9706104 100644
*** a/src/backend/catalog/aclchk.c
--- b/src/backend/catalog/aclchk.c
***************
*** 45,53 ****
  #include "catalog/pg_ts_config.h"
  #include "catalog/pg_ts_dict.h"
  #include "commands/dbcommands.h"
  #include "commands/proclang.h"
  #include "commands/tablespace.h"
- #include "foreign/foreign.h"
  #include "miscadmin.h"
  #include "parser/parse_func.h"
  #include "utils/acl.h"
--- 45,53 ----
  #include "catalog/pg_ts_config.h"
  #include "catalog/pg_ts_dict.h"
  #include "commands/dbcommands.h"
+ #include "commands/defrem.h"
  #include "commands/proclang.h"
  #include "commands/tablespace.h"
  #include "miscadmin.h"
  #include "parser/parse_func.h"
  #include "utils/acl.h"
*************** objectNamesToOids(GrantObjectType objtyp
*** 683,689 ****
  			foreach(cell, objnames)
  			{
  				char	   *fdwname = strVal(lfirst(cell));
! 				Oid			fdwid = GetForeignDataWrapperOidByName(fdwname, false);
  
  				objects = lappend_oid(objects, fdwid);
  			}
--- 683,689 ----
  			foreach(cell, objnames)
  			{
  				char	   *fdwname = strVal(lfirst(cell));
! 				Oid			fdwid = get_foreign_data_wrapper_oid(fdwname, false);
  
  				objects = lappend_oid(objects, fdwid);
  			}
*************** objectNamesToOids(GrantObjectType objtyp
*** 692,698 ****
  			foreach(cell, objnames)
  			{
  				char	   *srvname = strVal(lfirst(cell));
! 				Oid			srvid = GetForeignServerOidByName(srvname, false);
  
  				objects = lappend_oid(objects, srvid);
  			}
--- 692,698 ----
  			foreach(cell, objnames)
  			{
  				char	   *srvname = strVal(lfirst(cell));
! 				Oid			srvid = get_foreign_server_oid(srvname, false);
  
  				objects = lappend_oid(objects, srvid);
  			}
diff --git a/src/backend/commands/foreigncmds.c b/src/backend/commands/foreigncmds.c
index c5cd51e..c73162e 100644
*** a/src/backend/commands/foreigncmds.c
--- b/src/backend/commands/foreigncmds.c
*************** RemoveForeignDataWrapper(DropFdwStmt *st
*** 686,692 ****
  	Oid			fdwId;
  	ObjectAddress object;
  
! 	fdwId = GetForeignDataWrapperOidByName(stmt->fdwname, true);
  
  	if (!superuser())
  		ereport(ERROR,
--- 686,692 ----
  	Oid			fdwId;
  	ObjectAddress object;
  
! 	fdwId = get_foreign_data_wrapper_oid(stmt->fdwname, true);
  
  	if (!superuser())
  		ereport(ERROR,
*************** RemoveForeignServer(DropForeignServerStm
*** 959,965 ****
  	Oid			srvId;
  	ObjectAddress object;
  
! 	srvId = GetForeignServerOidByName(stmt->servername, true);
  
  	if (!OidIsValid(srvId))
  	{
--- 959,965 ----
  	Oid			srvId;
  	ObjectAddress object;
  
! 	srvId = get_foreign_server_oid(stmt->servername, true);
  
  	if (!OidIsValid(srvId))
  	{
diff --git a/src/backend/foreign/foreign.c b/src/backend/foreign/foreign.c
index 44cd181..be837d8 100644
*** a/src/backend/foreign/foreign.c
--- b/src/backend/foreign/foreign.c
***************
*** 19,24 ****
--- 19,25 ----
  #include "catalog/pg_foreign_table.h"
  #include "catalog/pg_type.h"
  #include "catalog/pg_user_mapping.h"
+ #include "commands/defrem.h"
  #include "foreign/fdwapi.h"
  #include "foreign/foreign.h"
  #include "funcapi.h"
*************** GetForeignDataWrapper(Oid fdwid)
*** 79,104 ****
  }
  
  
- /*
-  * GetForeignDataWrapperOidByName - look up the foreign-data wrapper
-  * OID by name.
-  */
- Oid
- GetForeignDataWrapperOidByName(const char *fdwname, bool missing_ok)
- {
- 	Oid			fdwId;
- 
- 	fdwId = GetSysCacheOid1(FOREIGNDATAWRAPPERNAME, CStringGetDatum(fdwname));
- 
- 	if (!OidIsValid(fdwId) && !missing_ok)
- 		ereport(ERROR,
- 				(errcode(ERRCODE_UNDEFINED_OBJECT),
- 				 errmsg("foreign-data wrapper \"%s\" does not exist",
- 						fdwname)));
- 
- 	return fdwId;
- }
- 
  
  /*
   * GetForeignDataWrapperByName - look up the foreign-data wrapper
--- 80,85 ----
*************** GetForeignDataWrapperOidByName(const cha
*** 107,113 ****
  ForeignDataWrapper *
  GetForeignDataWrapperByName(const char *fdwname, bool missing_ok)
  {
! 	Oid			fdwId = GetForeignDataWrapperOidByName(fdwname, missing_ok);
  
  	if (!OidIsValid(fdwId))
  		return NULL;
--- 88,94 ----
  ForeignDataWrapper *
  GetForeignDataWrapperByName(const char *fdwname, bool missing_ok)
  {
! 	Oid			fdwId = get_foreign_data_wrapper_oid(fdwname, missing_ok);
  
  	if (!OidIsValid(fdwId))
  		return NULL;
*************** GetForeignServer(Oid serverid)
*** 172,202 ****
  
  
  /*
-  * GetForeignServerByName - look up the foreign server oid by name.
-  */
- Oid
- GetForeignServerOidByName(const char *srvname, bool missing_ok)
- {
- 	Oid			serverid;
- 
- 	serverid = GetSysCacheOid1(FOREIGNSERVERNAME, CStringGetDatum(srvname));
- 
- 	if (!OidIsValid(serverid) && !missing_ok)
- 		ereport(ERROR,
- 				(errcode(ERRCODE_UNDEFINED_OBJECT),
- 				 errmsg("server \"%s\" does not exist", srvname)));
- 
- 	return serverid;
- }
- 
- 
- /*
   * GetForeignServerByName - look up the foreign server definition by name.
   */
  ForeignServer *
  GetForeignServerByName(const char *srvname, bool missing_ok)
  {
! 	Oid			serverid = GetForeignServerOidByName(srvname, missing_ok);
  
  	if (!OidIsValid(serverid))
  		return NULL;
--- 153,164 ----
  
  
  /*
   * GetForeignServerByName - look up the foreign server definition by name.
   */
  ForeignServer *
  GetForeignServerByName(const char *srvname, bool missing_ok)
  {
! 	Oid			serverid = get_foreign_server_oid(srvname, missing_ok);
  
  	if (!OidIsValid(serverid))
  		return NULL;
diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c
index 691ba3b..4cec65c 100644
*** a/src/backend/utils/adt/acl.c
--- b/src/backend/utils/adt/acl.c
***************
*** 22,27 ****
--- 22,28 ----
  #include "catalog/pg_type.h"
  #include "catalog/pg_class.h"
  #include "commands/dbcommands.h"
+ #include "commands/defrem.h"
  #include "commands/proclang.h"
  #include "commands/tablespace.h"
  #include "foreign/foreign.h"
*************** convert_foreign_data_wrapper_name(text *
*** 3162,3168 ****
  {
  	char	   *fdwstr = text_to_cstring(fdwname);
  
! 	return GetForeignDataWrapperOidByName(fdwstr, false);
  }
  
  /*
--- 3163,3169 ----
  {
  	char	   *fdwstr = text_to_cstring(fdwname);
  
! 	return get_foreign_data_wrapper_oid(fdwstr, false);
  }
  
  /*
*************** convert_server_name(text *servername)
*** 3928,3934 ****
  {
  	char	   *serverstr = text_to_cstring(servername);
  
! 	return GetForeignServerOidByName(serverstr, false);
  }
  
  /*
--- 3929,3935 ----
  {
  	char	   *serverstr = text_to_cstring(servername);
  
! 	return get_foreign_server_oid(serverstr, false);
  }
  
  /*
diff --git a/src/include/foreign/foreign.h b/src/include/foreign/foreign.h
index d676f3f..b4bfa1b 100644
*** a/src/include/foreign/foreign.h
--- b/src/include/foreign/foreign.h
*************** typedef struct ForeignTable
*** 70,81 ****
  
  extern ForeignServer *GetForeignServer(Oid serverid);
  extern ForeignServer *GetForeignServerByName(const char *name, bool missing_ok);
- extern Oid	GetForeignServerOidByName(const char *name, bool missing_ok);
  extern UserMapping *GetUserMapping(Oid userid, Oid serverid);
  extern ForeignDataWrapper *GetForeignDataWrapper(Oid fdwid);
  extern ForeignDataWrapper *GetForeignDataWrapperByName(const char *name,
  							bool missing_ok);
- extern Oid	GetForeignDataWrapperOidByName(const char *name, bool missing_ok);
  extern ForeignTable *GetForeignTable(Oid relid);
  
  #endif   /* FOREIGN_H */
--- 70,79 ----
20110401_pg_dump_support.patchapplication/octet-stream; name=20110401_pg_dump_support.patchDownload
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 5561295..a6ffb78 100644
*** a/src/bin/pg_dump/pg_dump.c
--- b/src/bin/pg_dump/pg_dump.c
*************** dumpForeignDataWrapper(Archive *fout, Fd
*** 11059,11064 ****
--- 11059,11069 ----
  			NULL, fdwinfo->rolname,
  			fdwinfo->fdwacl);
  
+ 	/* Dump foreign-data wraper Comments */
+ 	dumpComment(fout, labelq->data,
+ 				NULL, "",
+ 				fdwinfo->dobj.catId, 0, fdwinfo->dobj.dumpId);
+ 
  	free(qfdwname);
  
  	destroyPQExpBuffer(q);
*************** dumpForeignServer(Archive *fout, Foreign
*** 11163,11168 ****
--- 11168,11178 ----
  					 srvinfo->rolname,
  					 srvinfo->dobj.catId, srvinfo->dobj.dumpId);
  
+ 	/* Dump foreign server Comments */
+ 	dumpComment(fout, labelq->data,
+ 				NULL, "",
+ 				srvinfo->dobj.catId, 0, srvinfo->dobj.dumpId);
+ 
  	free(qsrvname);
  
  	destroyPQExpBuffer(q);
20110401_psql_describe.patchapplication/octet-stream; name=20110401_psql_describe.patchDownload
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index a89c938..13d0a64 100644
*** a/src/bin/psql/describe.c
--- b/src/bin/psql/describe.c
*************** listForeignDataWrappers(const char *patt
*** 3593,3601 ****
  		appendPQExpBuffer(&buf,
  						  ",\n  fdwoptions AS \"%s\"",
  						  gettext_noop("Options"));
  	}
  
! 	appendPQExpBuffer(&buf, "\nFROM pg_catalog.pg_foreign_data_wrapper\n");
  
  	processSQLNamePattern(pset.db, &buf, pattern, false, false,
  						  NULL, "fdwname", NULL, NULL);
--- 3593,3607 ----
  		appendPQExpBuffer(&buf,
  						  ",\n  fdwoptions AS \"%s\"",
  						  gettext_noop("Options"));
+ 		appendPQExpBuffer(&buf,
+ 						  ",\n  description AS \"%s\"",
+ 						  gettext_noop("Description"));
  	}
  
! 	appendPQExpBuffer(&buf, "\nFROM pg_catalog.pg_foreign_data_wrapper w \n");
! 	appendPQExpBuffer(&buf, 
! 					  "LEFT JOIN pg_catalog.pg_description c ON c.objoid = w.oid "
! 					  "AND c.classoid = 'pg_catalog.pg_foreign_data_wrapper'::pg_catalog.regclass\n");
  
  	processSQLNamePattern(pset.db, &buf, pattern, false, false,
  						  NULL, "fdwname", NULL, NULL);
*************** listForeignServers(const char *pattern, 
*** 3653,3667 ****
  						  ",\n"
  						  "  s.srvtype AS \"%s\",\n"
  						  "  s.srvversion AS \"%s\",\n"
! 						  "  s.srvoptions AS \"%s\"",
  						  gettext_noop("Type"),
  						  gettext_noop("Version"),
! 						  gettext_noop("Options"));
  	}
  
  	appendPQExpBuffer(&buf,
! 					  "\nFROM pg_catalog.pg_foreign_server s\n"
! 	   "     JOIN pg_catalog.pg_foreign_data_wrapper f ON f.oid=s.srvfdw\n");
  
  	processSQLNamePattern(pset.db, &buf, pattern, false, false,
  						  NULL, "s.srvname", NULL, NULL);
--- 3659,3677 ----
  						  ",\n"
  						  "  s.srvtype AS \"%s\",\n"
  						  "  s.srvversion AS \"%s\",\n"
! 						  "  s.srvoptions AS \"%s\",\n"
! 						  "  d.description AS \"%s\"\n",
  						  gettext_noop("Type"),
  						  gettext_noop("Version"),
! 						  gettext_noop("Options"),
! 						  gettext_noop("Description"));
  	}
  
  	appendPQExpBuffer(&buf,
! 		"\nFROM pg_catalog.pg_foreign_server s "
! 		"\n     JOIN pg_catalog.pg_foreign_data_wrapper f ON f.oid=s.srvfdw "
! 		"\nLEFT JOIN pg_catalog.pg_description d ON d.objoid=s.oid "
! 		"\n      AND d.classoid = 'pg_catalog.pg_foreign_server'::regclass ");
  
  	processSQLNamePattern(pset.db, &buf, pattern, false, false,
  						  NULL, "s.srvname", NULL, NULL);
diff --git a/src/test/regress/expected/foreign_data.out b/src/test/regress/expected/foreign_data.out
index c05bcab..7d187d8 100644
*** a/src/test/regress/expected/foreign_data.out
--- b/src/test/regress/expected/foreign_data.out
*************** ERROR:  foreign-data wrapper "foo" alrea
*** 52,63 ****
  DROP FOREIGN DATA WRAPPER foo;
  CREATE FOREIGN DATA WRAPPER foo OPTIONS (testing '1');
  \dew+
!                                      List of foreign-data wrappers
!     Name    |       Owner       | Handler |        Validator         | Access privileges |   Options   
! ------------+-------------------+---------+--------------------------+-------------------+-------------
!  dummy      | foreign_data_user | -       | -                        |                   | 
!  foo        | foreign_data_user | -       | -                        |                   | {testing=1}
!  postgresql | foreign_data_user | -       | postgresql_fdw_validator |                   | 
  (3 rows)
  
  DROP FOREIGN DATA WRAPPER foo;
--- 52,63 ----
  DROP FOREIGN DATA WRAPPER foo;
  CREATE FOREIGN DATA WRAPPER foo OPTIONS (testing '1');
  \dew+
!                                             List of foreign-data wrappers
!     Name    |       Owner       | Handler |        Validator         | Access privileges |   Options   | Description 
! ------------+-------------------+---------+--------------------------+-------------------+-------------+-------------
!  dummy      | foreign_data_user | -       | -                        |                   |             | useless
!  foo        | foreign_data_user | -       | -                        |                   | {testing=1} | 
!  postgresql | foreign_data_user | -       | postgresql_fdw_validator |                   |             | 
  (3 rows)
  
  DROP FOREIGN DATA WRAPPER foo;
*************** CREATE FOREIGN DATA WRAPPER foo OPTIONS 
*** 65,76 ****
  ERROR:  option "testing" provided more than once
  CREATE FOREIGN DATA WRAPPER foo OPTIONS (testing '1', another '2');
  \dew+
!                                           List of foreign-data wrappers
!     Name    |       Owner       | Handler |        Validator         | Access privileges |        Options        
! ------------+-------------------+---------+--------------------------+-------------------+-----------------------
!  dummy      | foreign_data_user | -       | -                        |                   | 
!  foo        | foreign_data_user | -       | -                        |                   | {testing=1,another=2}
!  postgresql | foreign_data_user | -       | postgresql_fdw_validator |                   | 
  (3 rows)
  
  DROP FOREIGN DATA WRAPPER foo;
--- 65,76 ----
  ERROR:  option "testing" provided more than once
  CREATE FOREIGN DATA WRAPPER foo OPTIONS (testing '1', another '2');
  \dew+
!                                                  List of foreign-data wrappers
!     Name    |       Owner       | Handler |        Validator         | Access privileges |        Options        | Description 
! ------------+-------------------+---------+--------------------------+-------------------+-----------------------+-------------
!  dummy      | foreign_data_user | -       | -                        |                   |                       | useless
!  foo        | foreign_data_user | -       | -                        |                   | {testing=1,another=2} | 
!  postgresql | foreign_data_user | -       | postgresql_fdw_validator |                   |                       | 
  (3 rows)
  
  DROP FOREIGN DATA WRAPPER foo;
*************** HINT:  Must be superuser to create a for
*** 81,92 ****
  RESET ROLE;
  CREATE FOREIGN DATA WRAPPER foo VALIDATOR postgresql_fdw_validator;
  \dew+
!                                    List of foreign-data wrappers
!     Name    |       Owner       | Handler |        Validator         | Access privileges | Options 
! ------------+-------------------+---------+--------------------------+-------------------+---------
!  dummy      | foreign_data_user | -       | -                        |                   | 
!  foo        | foreign_data_user | -       | postgresql_fdw_validator |                   | 
!  postgresql | foreign_data_user | -       | postgresql_fdw_validator |                   | 
  (3 rows)
  
  -- ALTER FOREIGN DATA WRAPPER
--- 81,92 ----
  RESET ROLE;
  CREATE FOREIGN DATA WRAPPER foo VALIDATOR postgresql_fdw_validator;
  \dew+
!                                           List of foreign-data wrappers
!     Name    |       Owner       | Handler |        Validator         | Access privileges | Options | Description 
! ------------+-------------------+---------+--------------------------+-------------------+---------+-------------
!  dummy      | foreign_data_user | -       | -                        |                   |         | useless
!  foo        | foreign_data_user | -       | postgresql_fdw_validator |                   |         | 
!  postgresql | foreign_data_user | -       | postgresql_fdw_validator |                   |         | 
  (3 rows)
  
  -- ALTER FOREIGN DATA WRAPPER
*************** ALTER FOREIGN DATA WRAPPER foo VALIDATOR
*** 98,109 ****
  ERROR:  function bar(text[], oid) does not exist
  ALTER FOREIGN DATA WRAPPER foo NO VALIDATOR;
  \dew+
!                                    List of foreign-data wrappers
!     Name    |       Owner       | Handler |        Validator         | Access privileges | Options 
! ------------+-------------------+---------+--------------------------+-------------------+---------
!  dummy      | foreign_data_user | -       | -                        |                   | 
!  foo        | foreign_data_user | -       | -                        |                   | 
!  postgresql | foreign_data_user | -       | postgresql_fdw_validator |                   | 
  (3 rows)
  
  ALTER FOREIGN DATA WRAPPER foo OPTIONS (a '1', b '2');
--- 98,109 ----
  ERROR:  function bar(text[], oid) does not exist
  ALTER FOREIGN DATA WRAPPER foo NO VALIDATOR;
  \dew+
!                                           List of foreign-data wrappers
!     Name    |       Owner       | Handler |        Validator         | Access privileges | Options | Description 
! ------------+-------------------+---------+--------------------------+-------------------+---------+-------------
!  dummy      | foreign_data_user | -       | -                        |                   |         | useless
!  foo        | foreign_data_user | -       | -                        |                   |         | 
!  postgresql | foreign_data_user | -       | postgresql_fdw_validator |                   |         | 
  (3 rows)
  
  ALTER FOREIGN DATA WRAPPER foo OPTIONS (a '1', b '2');
*************** ALTER FOREIGN DATA WRAPPER foo OPTIONS (
*** 113,146 ****
  ERROR:  option "c" not found
  ALTER FOREIGN DATA WRAPPER foo OPTIONS (ADD x '1', DROP x);
  \dew+
!                                     List of foreign-data wrappers
!     Name    |       Owner       | Handler |        Validator         | Access privileges |  Options  
! ------------+-------------------+---------+--------------------------+-------------------+-----------
!  dummy      | foreign_data_user | -       | -                        |                   | 
!  foo        | foreign_data_user | -       | -                        |                   | {a=1,b=2}
!  postgresql | foreign_data_user | -       | postgresql_fdw_validator |                   | 
  (3 rows)
  
  ALTER FOREIGN DATA WRAPPER foo OPTIONS (DROP a, SET b '3', ADD c '4');
  \dew+
!                                     List of foreign-data wrappers
!     Name    |       Owner       | Handler |        Validator         | Access privileges |  Options  
! ------------+-------------------+---------+--------------------------+-------------------+-----------
!  dummy      | foreign_data_user | -       | -                        |                   | 
!  foo        | foreign_data_user | -       | -                        |                   | {b=3,c=4}
!  postgresql | foreign_data_user | -       | postgresql_fdw_validator |                   | 
  (3 rows)
  
  ALTER FOREIGN DATA WRAPPER foo OPTIONS (a '2');
  ALTER FOREIGN DATA WRAPPER foo OPTIONS (b '4');             -- ERROR
  ERROR:  option "b" provided more than once
  \dew+
!                                       List of foreign-data wrappers
!     Name    |       Owner       | Handler |        Validator         | Access privileges |    Options    
! ------------+-------------------+---------+--------------------------+-------------------+---------------
!  dummy      | foreign_data_user | -       | -                        |                   | 
!  foo        | foreign_data_user | -       | -                        |                   | {b=3,c=4,a=2}
!  postgresql | foreign_data_user | -       | postgresql_fdw_validator |                   | 
  (3 rows)
  
  SET ROLE regress_test_role;
--- 113,146 ----
  ERROR:  option "c" not found
  ALTER FOREIGN DATA WRAPPER foo OPTIONS (ADD x '1', DROP x);
  \dew+
!                                            List of foreign-data wrappers
!     Name    |       Owner       | Handler |        Validator         | Access privileges |  Options  | Description 
! ------------+-------------------+---------+--------------------------+-------------------+-----------+-------------
!  dummy      | foreign_data_user | -       | -                        |                   |           | useless
!  foo        | foreign_data_user | -       | -                        |                   | {a=1,b=2} | 
!  postgresql | foreign_data_user | -       | postgresql_fdw_validator |                   |           | 
  (3 rows)
  
  ALTER FOREIGN DATA WRAPPER foo OPTIONS (DROP a, SET b '3', ADD c '4');
  \dew+
!                                            List of foreign-data wrappers
!     Name    |       Owner       | Handler |        Validator         | Access privileges |  Options  | Description 
! ------------+-------------------+---------+--------------------------+-------------------+-----------+-------------
!  dummy      | foreign_data_user | -       | -                        |                   |           | useless
!  foo        | foreign_data_user | -       | -                        |                   | {b=3,c=4} | 
!  postgresql | foreign_data_user | -       | postgresql_fdw_validator |                   |           | 
  (3 rows)
  
  ALTER FOREIGN DATA WRAPPER foo OPTIONS (a '2');
  ALTER FOREIGN DATA WRAPPER foo OPTIONS (b '4');             -- ERROR
  ERROR:  option "b" provided more than once
  \dew+
!                                              List of foreign-data wrappers
!     Name    |       Owner       | Handler |        Validator         | Access privileges |    Options    | Description 
! ------------+-------------------+---------+--------------------------+-------------------+---------------+-------------
!  dummy      | foreign_data_user | -       | -                        |                   |               | useless
!  foo        | foreign_data_user | -       | -                        |                   | {b=3,c=4,a=2} | 
!  postgresql | foreign_data_user | -       | postgresql_fdw_validator |                   |               | 
  (3 rows)
  
  SET ROLE regress_test_role;
*************** HINT:  Must be superuser to alter a fore
*** 150,161 ****
  SET ROLE regress_test_role_super;
  ALTER FOREIGN DATA WRAPPER foo OPTIONS (ADD d '5');
  \dew+
!                                         List of foreign-data wrappers
!     Name    |       Owner       | Handler |        Validator         | Access privileges |      Options      
! ------------+-------------------+---------+--------------------------+-------------------+-------------------
!  dummy      | foreign_data_user | -       | -                        |                   | 
!  foo        | foreign_data_user | -       | -                        |                   | {b=3,c=4,a=2,d=5}
!  postgresql | foreign_data_user | -       | postgresql_fdw_validator |                   | 
  (3 rows)
  
  ALTER FOREIGN DATA WRAPPER foo OWNER TO regress_test_role;  -- ERROR
--- 150,161 ----
  SET ROLE regress_test_role_super;
  ALTER FOREIGN DATA WRAPPER foo OPTIONS (ADD d '5');
  \dew+
!                                                List of foreign-data wrappers
!     Name    |       Owner       | Handler |        Validator         | Access privileges |      Options      | Description 
! ------------+-------------------+---------+--------------------------+-------------------+-------------------+-------------
!  dummy      | foreign_data_user | -       | -                        |                   |                   | useless
!  foo        | foreign_data_user | -       | -                        |                   | {b=3,c=4,a=2,d=5} | 
!  postgresql | foreign_data_user | -       | postgresql_fdw_validator |                   |                   | 
  (3 rows)
  
  ALTER FOREIGN DATA WRAPPER foo OWNER TO regress_test_role;  -- ERROR
*************** ERROR:  permission denied to alter forei
*** 169,180 ****
  HINT:  Must be superuser to alter a foreign-data wrapper.
  RESET ROLE;
  \dew+
!                                            List of foreign-data wrappers
!     Name    |          Owner          | Handler |        Validator         | Access privileges |      Options      
! ------------+-------------------------+---------+--------------------------+-------------------+-------------------
!  dummy      | foreign_data_user       | -       | -                        |                   | 
!  foo        | regress_test_role_super | -       | -                        |                   | {b=3,c=4,a=2,d=5}
!  postgresql | foreign_data_user       | -       | postgresql_fdw_validator |                   | 
  (3 rows)
  
  -- DROP FOREIGN DATA WRAPPER
--- 169,180 ----
  HINT:  Must be superuser to alter a foreign-data wrapper.
  RESET ROLE;
  \dew+
!                                                   List of foreign-data wrappers
!     Name    |          Owner          | Handler |        Validator         | Access privileges |      Options      | Description 
! ------------+-------------------------+---------+--------------------------+-------------------+-------------------+-------------
!  dummy      | foreign_data_user       | -       | -                        |                   |                   | useless
!  foo        | regress_test_role_super | -       | -                        |                   | {b=3,c=4,a=2,d=5} | 
!  postgresql | foreign_data_user       | -       | postgresql_fdw_validator |                   |                   | 
  (3 rows)
  
  -- DROP FOREIGN DATA WRAPPER
*************** ERROR:  foreign-data wrapper "nonexisten
*** 183,194 ****
  DROP FOREIGN DATA WRAPPER IF EXISTS nonexistent;
  NOTICE:  foreign-data wrapper "nonexistent" does not exist, skipping
  \dew+
!                                            List of foreign-data wrappers
!     Name    |          Owner          | Handler |        Validator         | Access privileges |      Options      
! ------------+-------------------------+---------+--------------------------+-------------------+-------------------
!  dummy      | foreign_data_user       | -       | -                        |                   | 
!  foo        | regress_test_role_super | -       | -                        |                   | {b=3,c=4,a=2,d=5}
!  postgresql | foreign_data_user       | -       | postgresql_fdw_validator |                   | 
  (3 rows)
  
  DROP ROLE regress_test_role_super;                          -- ERROR
--- 183,194 ----
  DROP FOREIGN DATA WRAPPER IF EXISTS nonexistent;
  NOTICE:  foreign-data wrapper "nonexistent" does not exist, skipping
  \dew+
!                                                   List of foreign-data wrappers
!     Name    |          Owner          | Handler |        Validator         | Access privileges |      Options      | Description 
! ------------+-------------------------+---------+--------------------------+-------------------+-------------------+-------------
!  dummy      | foreign_data_user       | -       | -                        |                   |                   | useless
!  foo        | regress_test_role_super | -       | -                        |                   | {b=3,c=4,a=2,d=5} | 
!  postgresql | foreign_data_user       | -       | postgresql_fdw_validator |                   |                   | 
  (3 rows)
  
  DROP ROLE regress_test_role_super;                          -- ERROR
*************** ALTER ROLE regress_test_role_super SUPER
*** 203,213 ****
  DROP FOREIGN DATA WRAPPER foo;
  DROP ROLE regress_test_role_super;
  \dew+
!                                    List of foreign-data wrappers
!     Name    |       Owner       | Handler |        Validator         | Access privileges | Options 
! ------------+-------------------+---------+--------------------------+-------------------+---------
!  dummy      | foreign_data_user | -       | -                        |                   | 
!  postgresql | foreign_data_user | -       | postgresql_fdw_validator |                   | 
  (2 rows)
  
  CREATE FOREIGN DATA WRAPPER foo;
--- 203,213 ----
  DROP FOREIGN DATA WRAPPER foo;
  DROP ROLE regress_test_role_super;
  \dew+
!                                           List of foreign-data wrappers
!     Name    |       Owner       | Handler |        Validator         | Access privileges | Options | Description 
! ------------+-------------------+---------+--------------------------+-------------------+---------+-------------
!  dummy      | foreign_data_user | -       | -                        |                   |         | useless
!  postgresql | foreign_data_user | -       | postgresql_fdw_validator |                   |         | 
  (2 rows)
  
  CREATE FOREIGN DATA WRAPPER foo;
*************** CREATE SERVER s1 FOREIGN DATA WRAPPER fo
*** 215,233 ****
  COMMENT ON SERVER s1 IS 'foreign server';
  CREATE USER MAPPING FOR current_user SERVER s1;
  \dew+
!                                    List of foreign-data wrappers
!     Name    |       Owner       | Handler |        Validator         | Access privileges | Options 
! ------------+-------------------+---------+--------------------------+-------------------+---------
!  dummy      | foreign_data_user | -       | -                        |                   | 
!  foo        | foreign_data_user | -       | -                        |                   | 
!  postgresql | foreign_data_user | -       | postgresql_fdw_validator |                   | 
  (3 rows)
  
  \des+
!                                     List of foreign servers
!  Name |       Owner       | Foreign-data wrapper | Access privileges | Type | Version | Options 
! ------+-------------------+----------------------+-------------------+------+---------+---------
!  s1   | foreign_data_user | foo                  |                   |      |         | 
  (1 row)
  
  \deu+
--- 215,233 ----
  COMMENT ON SERVER s1 IS 'foreign server';
  CREATE USER MAPPING FOR current_user SERVER s1;
  \dew+
!                                           List of foreign-data wrappers
!     Name    |       Owner       | Handler |        Validator         | Access privileges | Options | Description 
! ------------+-------------------+---------+--------------------------+-------------------+---------+-------------
!  dummy      | foreign_data_user | -       | -                        |                   |         | useless
!  foo        | foreign_data_user | -       | -                        |                   |         | 
!  postgresql | foreign_data_user | -       | postgresql_fdw_validator |                   |         | 
  (3 rows)
  
  \des+
!                                              List of foreign servers
!  Name |       Owner       | Foreign-data wrapper | Access privileges | Type | Version | Options |  Description   
! ------+-------------------+----------------------+-------------------+------+---------+---------+----------------
!  s1   | foreign_data_user | foo                  |                   |      |         |         | foreign server
  (1 row)
  
  \deu+
*************** NOTICE:  drop cascades to 2 other object
*** 252,268 ****
  DETAIL:  drop cascades to server s1
  drop cascades to user mapping for foreign_data_user
  \dew+
!                                    List of foreign-data wrappers
!     Name    |       Owner       | Handler |        Validator         | Access privileges | Options 
! ------------+-------------------+---------+--------------------------+-------------------+---------
!  dummy      | foreign_data_user | -       | -                        |                   | 
!  postgresql | foreign_data_user | -       | postgresql_fdw_validator |                   | 
  (2 rows)
  
  \des+
!                               List of foreign servers
!  Name | Owner | Foreign-data wrapper | Access privileges | Type | Version | Options 
! ------+-------+----------------------+-------------------+------+---------+---------
  (0 rows)
  
  \deu+
--- 252,268 ----
  DETAIL:  drop cascades to server s1
  drop cascades to user mapping for foreign_data_user
  \dew+
!                                           List of foreign-data wrappers
!     Name    |       Owner       | Handler |        Validator         | Access privileges | Options | Description 
! ------------+-------------------+---------+--------------------------+-------------------+---------+-------------
!  dummy      | foreign_data_user | -       | -                        |                   |         | useless
!  postgresql | foreign_data_user | -       | postgresql_fdw_validator |                   |         | 
  (2 rows)
  
  \des+
!                                      List of foreign servers
!  Name | Owner | Foreign-data wrapper | Access privileges | Type | Version | Options | Description 
! ------+-------+----------------------+-------------------+------+---------+---------+-------------
  (0 rows)
  
  \deu+
*************** ERROR:  invalid option "foo"
*** 289,305 ****
  HINT:  Valid options in this context are: authtype, service, connect_timeout, dbname, host, hostaddr, port, tty, options, requiressl, sslmode, gsslib
  CREATE SERVER s8 FOREIGN DATA WRAPPER postgresql OPTIONS (host 'localhost', dbname 's8db');
  \des+
!                                                 List of foreign servers
!  Name |       Owner       | Foreign-data wrapper | Access privileges |  Type  | Version |           Options            
! ------+-------------------+----------------------+-------------------+--------+---------+------------------------------
!  s1   | foreign_data_user | foo                  |                   |        |         | 
!  s2   | foreign_data_user | foo                  |                   |        |         | {host=a,dbname=b}
!  s3   | foreign_data_user | foo                  |                   | oracle |         | 
!  s4   | foreign_data_user | foo                  |                   | oracle |         | {host=a,dbname=b}
!  s5   | foreign_data_user | foo                  |                   |        | 15.0    | 
!  s6   | foreign_data_user | foo                  |                   |        | 16.0    | {host=a,dbname=b}
!  s7   | foreign_data_user | foo                  |                   | oracle | 17.0    | {host=a,dbname=b}
!  s8   | foreign_data_user | postgresql           |                   |        |         | {host=localhost,dbname=s8db}
  (8 rows)
  
  SET ROLE regress_test_role;
--- 289,305 ----
  HINT:  Valid options in this context are: authtype, service, connect_timeout, dbname, host, hostaddr, port, tty, options, requiressl, sslmode, gsslib
  CREATE SERVER s8 FOREIGN DATA WRAPPER postgresql OPTIONS (host 'localhost', dbname 's8db');
  \des+
!                                                        List of foreign servers
!  Name |       Owner       | Foreign-data wrapper | Access privileges |  Type  | Version |           Options            | Description 
! ------+-------------------+----------------------+-------------------+--------+---------+------------------------------+-------------
!  s1   | foreign_data_user | foo                  |                   |        |         |                              | 
!  s2   | foreign_data_user | foo                  |                   |        |         | {host=a,dbname=b}            | 
!  s3   | foreign_data_user | foo                  |                   | oracle |         |                              | 
!  s4   | foreign_data_user | foo                  |                   | oracle |         | {host=a,dbname=b}            | 
!  s5   | foreign_data_user | foo                  |                   |        | 15.0    |                              | 
!  s6   | foreign_data_user | foo                  |                   |        | 16.0    | {host=a,dbname=b}            | 
!  s7   | foreign_data_user | foo                  |                   | oracle | 17.0    | {host=a,dbname=b}            | 
!  s8   | foreign_data_user | postgresql           |                   |        |         | {host=localhost,dbname=s8db} | 
  (8 rows)
  
  SET ROLE regress_test_role;
*************** SET ROLE regress_test_role;
*** 311,328 ****
  CREATE SERVER t1 FOREIGN DATA WRAPPER foo;
  RESET ROLE;
  \des+
!                                                 List of foreign servers
!  Name |       Owner       | Foreign-data wrapper | Access privileges |  Type  | Version |           Options            
! ------+-------------------+----------------------+-------------------+--------+---------+------------------------------
!  s1   | foreign_data_user | foo                  |                   |        |         | 
!  s2   | foreign_data_user | foo                  |                   |        |         | {host=a,dbname=b}
!  s3   | foreign_data_user | foo                  |                   | oracle |         | 
!  s4   | foreign_data_user | foo                  |                   | oracle |         | {host=a,dbname=b}
!  s5   | foreign_data_user | foo                  |                   |        | 15.0    | 
!  s6   | foreign_data_user | foo                  |                   |        | 16.0    | {host=a,dbname=b}
!  s7   | foreign_data_user | foo                  |                   | oracle | 17.0    | {host=a,dbname=b}
!  s8   | foreign_data_user | postgresql           |                   |        |         | {host=localhost,dbname=s8db}
!  t1   | regress_test_role | foo                  |                   |        |         | 
  (9 rows)
  
  REVOKE USAGE ON FOREIGN DATA WRAPPER foo FROM regress_test_role;
--- 311,328 ----
  CREATE SERVER t1 FOREIGN DATA WRAPPER foo;
  RESET ROLE;
  \des+
!                                                        List of foreign servers
!  Name |       Owner       | Foreign-data wrapper | Access privileges |  Type  | Version |           Options            | Description 
! ------+-------------------+----------------------+-------------------+--------+---------+------------------------------+-------------
!  s1   | foreign_data_user | foo                  |                   |        |         |                              | 
!  s2   | foreign_data_user | foo                  |                   |        |         | {host=a,dbname=b}            | 
!  s3   | foreign_data_user | foo                  |                   | oracle |         |                              | 
!  s4   | foreign_data_user | foo                  |                   | oracle |         | {host=a,dbname=b}            | 
!  s5   | foreign_data_user | foo                  |                   |        | 15.0    |                              | 
!  s6   | foreign_data_user | foo                  |                   |        | 16.0    | {host=a,dbname=b}            | 
!  s7   | foreign_data_user | foo                  |                   | oracle | 17.0    | {host=a,dbname=b}            | 
!  s8   | foreign_data_user | postgresql           |                   |        |         | {host=localhost,dbname=s8db} | 
!  t1   | regress_test_role | foo                  |                   |        |         |                              | 
  (9 rows)
  
  REVOKE USAGE ON FOREIGN DATA WRAPPER foo FROM regress_test_role;
*************** GRANT regress_test_indirect TO regress_t
*** 335,353 ****
  SET ROLE regress_test_role;
  CREATE SERVER t2 FOREIGN DATA WRAPPER foo;
  \des+
!                                                 List of foreign servers
!  Name |       Owner       | Foreign-data wrapper | Access privileges |  Type  | Version |           Options            
! ------+-------------------+----------------------+-------------------+--------+---------+------------------------------
!  s1   | foreign_data_user | foo                  |                   |        |         | 
!  s2   | foreign_data_user | foo                  |                   |        |         | {host=a,dbname=b}
!  s3   | foreign_data_user | foo                  |                   | oracle |         | 
!  s4   | foreign_data_user | foo                  |                   | oracle |         | {host=a,dbname=b}
!  s5   | foreign_data_user | foo                  |                   |        | 15.0    | 
!  s6   | foreign_data_user | foo                  |                   |        | 16.0    | {host=a,dbname=b}
!  s7   | foreign_data_user | foo                  |                   | oracle | 17.0    | {host=a,dbname=b}
!  s8   | foreign_data_user | postgresql           |                   |        |         | {host=localhost,dbname=s8db}
!  t1   | regress_test_role | foo                  |                   |        |         | 
!  t2   | regress_test_role | foo                  |                   |        |         | 
  (10 rows)
  
  RESET ROLE;
--- 335,353 ----
  SET ROLE regress_test_role;
  CREATE SERVER t2 FOREIGN DATA WRAPPER foo;
  \des+
!                                                        List of foreign servers
!  Name |       Owner       | Foreign-data wrapper | Access privileges |  Type  | Version |           Options            | Description 
! ------+-------------------+----------------------+-------------------+--------+---------+------------------------------+-------------
!  s1   | foreign_data_user | foo                  |                   |        |         |                              | 
!  s2   | foreign_data_user | foo                  |                   |        |         | {host=a,dbname=b}            | 
!  s3   | foreign_data_user | foo                  |                   | oracle |         |                              | 
!  s4   | foreign_data_user | foo                  |                   | oracle |         | {host=a,dbname=b}            | 
!  s5   | foreign_data_user | foo                  |                   |        | 15.0    |                              | 
!  s6   | foreign_data_user | foo                  |                   |        | 16.0    | {host=a,dbname=b}            | 
!  s7   | foreign_data_user | foo                  |                   | oracle | 17.0    | {host=a,dbname=b}            | 
!  s8   | foreign_data_user | postgresql           |                   |        |         | {host=localhost,dbname=s8db} | 
!  t1   | regress_test_role | foo                  |                   |        |         |                              | 
!  t2   | regress_test_role | foo                  |                   |        |         |                              | 
  (10 rows)
  
  RESET ROLE;
*************** ALTER SERVER s3 OPTIONS (tnsname 'orcl',
*** 365,385 ****
  GRANT USAGE ON FOREIGN SERVER s1 TO regress_test_role;
  GRANT USAGE ON FOREIGN SERVER s6 TO regress_test_role2 WITH GRANT OPTION;
  \des+
!                                                            List of foreign servers
!  Name |       Owner       | Foreign-data wrapper |            Access privileges            |  Type  | Version |           Options            
! ------+-------------------+----------------------+-----------------------------------------+--------+---------+------------------------------
!  s1   | foreign_data_user | foo                  | foreign_data_user=U/foreign_data_user  +|        | 1.0     | {servername=s1}
!       |                   |                      | regress_test_role=U/foreign_data_user   |        |         | 
!  s2   | foreign_data_user | foo                  |                                         |        | 1.1     | {host=a,dbname=b}
!  s3   | foreign_data_user | foo                  |                                         | oracle |         | {tnsname=orcl,port=1521}
!  s4   | foreign_data_user | foo                  |                                         | oracle |         | {host=a,dbname=b}
!  s5   | foreign_data_user | foo                  |                                         |        | 15.0    | 
!  s6   | foreign_data_user | foo                  | foreign_data_user=U/foreign_data_user  +|        | 16.0    | {host=a,dbname=b}
!       |                   |                      | regress_test_role2=U*/foreign_data_user |        |         | 
!  s7   | foreign_data_user | foo                  |                                         | oracle | 17.0    | {host=a,dbname=b}
!  s8   | foreign_data_user | postgresql           |                                         |        |         | {host=localhost,dbname=s8db}
!  t1   | regress_test_role | foo                  |                                         |        |         | 
!  t2   | regress_test_role | foo                  |                                         |        |         | 
  (10 rows)
  
  SET ROLE regress_test_role;
--- 365,385 ----
  GRANT USAGE ON FOREIGN SERVER s1 TO regress_test_role;
  GRANT USAGE ON FOREIGN SERVER s6 TO regress_test_role2 WITH GRANT OPTION;
  \des+
!                                                                   List of foreign servers
!  Name |       Owner       | Foreign-data wrapper |            Access privileges            |  Type  | Version |           Options            | Description 
! ------+-------------------+----------------------+-----------------------------------------+--------+---------+------------------------------+-------------
!  s1   | foreign_data_user | foo                  | foreign_data_user=U/foreign_data_user  +|        | 1.0     | {servername=s1}              | 
!       |                   |                      | regress_test_role=U/foreign_data_user   |        |         |                              | 
!  s2   | foreign_data_user | foo                  |                                         |        | 1.1     | {host=a,dbname=b}            | 
!  s3   | foreign_data_user | foo                  |                                         | oracle |         | {tnsname=orcl,port=1521}     | 
!  s4   | foreign_data_user | foo                  |                                         | oracle |         | {host=a,dbname=b}            | 
!  s5   | foreign_data_user | foo                  |                                         |        | 15.0    |                              | 
!  s6   | foreign_data_user | foo                  | foreign_data_user=U/foreign_data_user  +|        | 16.0    | {host=a,dbname=b}            | 
!       |                   |                      | regress_test_role2=U*/foreign_data_user |        |         |                              | 
!  s7   | foreign_data_user | foo                  |                                         | oracle | 17.0    | {host=a,dbname=b}            | 
!  s8   | foreign_data_user | postgresql           |                                         |        |         | {host=localhost,dbname=s8db} | 
!  t1   | regress_test_role | foo                  |                                         |        |         |                              | 
!  t2   | regress_test_role | foo                  |                                         |        |         |                              | 
  (10 rows)
  
  SET ROLE regress_test_role;
*************** ERROR:  role "regress_test_indirect" can
*** 416,436 ****
  DETAIL:  owner of server s1
  privileges for foreign-data wrapper foo
  \des+
!                                                               List of foreign servers
!  Name |         Owner         | Foreign-data wrapper |            Access privileges            |  Type  | Version |             Options             
! ------+-----------------------+----------------------+-----------------------------------------+--------+---------+---------------------------------
!  s1   | regress_test_indirect | foo                  | foreign_data_user=U/foreign_data_user  +|        | 1.1     | {servername=s1}
!       |                       |                      | regress_test_role=U/foreign_data_user   |        |         | 
!  s2   | foreign_data_user     | foo                  |                                         |        | 1.1     | {host=a,dbname=b}
!  s3   | foreign_data_user     | foo                  |                                         | oracle |         | {tnsname=orcl,port=1521}
!  s4   | foreign_data_user     | foo                  |                                         | oracle |         | {host=a,dbname=b}
!  s5   | foreign_data_user     | foo                  |                                         |        | 15.0    | 
!  s6   | foreign_data_user     | foo                  | foreign_data_user=U/foreign_data_user  +|        | 16.0    | {host=a,dbname=b}
!       |                       |                      | regress_test_role2=U*/foreign_data_user |        |         | 
!  s7   | foreign_data_user     | foo                  |                                         | oracle | 17.0    | {host=a,dbname=b}
!  s8   | foreign_data_user     | postgresql           |                                         |        |         | {dbname=db1,connect_timeout=30}
!  t1   | regress_test_role     | foo                  |                                         |        |         | 
!  t2   | regress_test_role     | foo                  |                                         |        |         | 
  (10 rows)
  
  -- DROP SERVER
--- 416,436 ----
  DETAIL:  owner of server s1
  privileges for foreign-data wrapper foo
  \des+
!                                                                      List of foreign servers
!  Name |         Owner         | Foreign-data wrapper |            Access privileges            |  Type  | Version |             Options             | Description 
! ------+-----------------------+----------------------+-----------------------------------------+--------+---------+---------------------------------+-------------
!  s1   | regress_test_indirect | foo                  | foreign_data_user=U/foreign_data_user  +|        | 1.1     | {servername=s1}                 | 
!       |                       |                      | regress_test_role=U/foreign_data_user   |        |         |                                 | 
!  s2   | foreign_data_user     | foo                  |                                         |        | 1.1     | {host=a,dbname=b}               | 
!  s3   | foreign_data_user     | foo                  |                                         | oracle |         | {tnsname=orcl,port=1521}        | 
!  s4   | foreign_data_user     | foo                  |                                         | oracle |         | {host=a,dbname=b}               | 
!  s5   | foreign_data_user     | foo                  |                                         |        | 15.0    |                                 | 
!  s6   | foreign_data_user     | foo                  | foreign_data_user=U/foreign_data_user  +|        | 16.0    | {host=a,dbname=b}               | 
!       |                       |                      | regress_test_role2=U*/foreign_data_user |        |         |                                 | 
!  s7   | foreign_data_user     | foo                  |                                         | oracle | 17.0    | {host=a,dbname=b}               | 
!  s8   | foreign_data_user     | postgresql           |                                         |        |         | {dbname=db1,connect_timeout=30} | 
!  t1   | regress_test_role     | foo                  |                                         |        |         |                                 | 
!  t2   | regress_test_role     | foo                  |                                         |        |         |                                 | 
  (10 rows)
  
  -- DROP SERVER
20110401_psql_tab.patchapplication/octet-stream; name=20110401_psql_tab.patchDownload
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index e72c5b9..994c053 100644
*** a/src/bin/psql/tab-complete.c
--- b/src/bin/psql/tab-complete.c
*************** psql_completion(char *text, int start, i
*** 714,720 ****
  			   *prev2_wd,
  			   *prev3_wd,
  			   *prev4_wd,
! 			   *prev5_wd;
  
  	static const char *const sql_commands[] = {
  		"ABORT", "ALTER", "ANALYZE", "BEGIN", "CHECKPOINT", "CLOSE", "CLUSTER",
--- 714,721 ----
  			   *prev2_wd,
  			   *prev3_wd,
  			   *prev4_wd,
! 			   *prev5_wd,
! 			   *prev6_wd;
  
  	static const char *const sql_commands[] = {
  		"ABORT", "ALTER", "ANALYZE", "BEGIN", "CHECKPOINT", "CLOSE", "CLUSTER",
*************** psql_completion(char *text, int start, i
*** 762,767 ****
--- 763,769 ----
  	prev3_wd = previous_word(start, 2);
  	prev4_wd = previous_word(start, 3);
  	prev5_wd = previous_word(start, 4);
+ 	prev6_wd = previous_word(start, 5);
  
  	/* If a backslash command was started, continue */
  	if (text[0] == '\\')
*************** psql_completion(char *text, int start, i
*** 1547,1559 ****
  			 pg_strcasecmp(prev_wd, "ON") == 0)
  	{
  		static const char *const list_COMMENT[] =
! 		{"CAST", "COLLATION", "CONVERSION", "DATABASE", "FOREIGN TABLE", "INDEX", "LANGUAGE", "RULE", "SCHEMA",
  			"SEQUENCE", "TABLE", "TYPE", "VIEW", "COLUMN", "AGGREGATE", "FUNCTION",
  			"OPERATOR", "TRIGGER", "CONSTRAINT", "DOMAIN", "LARGE OBJECT",
  		"TABLESPACE", "TEXT SEARCH", "ROLE", NULL};
  
  		COMPLETE_WITH_LIST(list_COMMENT);
  	}
  	else if (pg_strcasecmp(prev4_wd, "COMMENT") == 0 &&
  			 pg_strcasecmp(prev3_wd, "ON") == 0 &&
  			 pg_strcasecmp(prev2_wd, "TEXT") == 0 &&
--- 1549,1571 ----
  			 pg_strcasecmp(prev_wd, "ON") == 0)
  	{
  		static const char *const list_COMMENT[] =
! 		{"CAST", "COLLATION", "CONVERSION", "DATABASE", "FOREIGN DATA WRAPPER",
! 			"SERVER", "FOREIGN TABLE", "INDEX", "LANGUAGE", "RULE", "SCHEMA",
  			"SEQUENCE", "TABLE", "TYPE", "VIEW", "COLUMN", "AGGREGATE", "FUNCTION",
  			"OPERATOR", "TRIGGER", "CONSTRAINT", "DOMAIN", "LARGE OBJECT",
  		"TABLESPACE", "TEXT SEARCH", "ROLE", NULL};
  
  		COMPLETE_WITH_LIST(list_COMMENT);
  	}
+ 	else if (pg_strcasecmp(prev3_wd, "COMMENT") == 0 &&
+ 			 pg_strcasecmp(prev2_wd, "ON") == 0 &&
+ 			 pg_strcasecmp(prev_wd, "FOREIGN") == 0)
+ 	{
+ 		static const char *const list_TRANS2[] =
+ 		{"DATA WRAPPER", "TABLE", NULL};
+ 
+ 		COMPLETE_WITH_LIST(list_TRANS2);
+ 	}
  	else if (pg_strcasecmp(prev4_wd, "COMMENT") == 0 &&
  			 pg_strcasecmp(prev3_wd, "ON") == 0 &&
  			 pg_strcasecmp(prev2_wd, "TEXT") == 0 &&
*************** psql_completion(char *text, int start, i
*** 1566,1571 ****
--- 1578,1585 ----
  	}
  	else if ((pg_strcasecmp(prev4_wd, "COMMENT") == 0 &&
  			  pg_strcasecmp(prev3_wd, "ON") == 0) ||
+ 			 (pg_strcasecmp(prev6_wd, "COMMENT") == 0 &&
+ 			  pg_strcasecmp(prev5_wd, "ON") == 0) ||
  			 (pg_strcasecmp(prev5_wd, "ON") == 0 &&
  			  pg_strcasecmp(prev4_wd, "TEXT") == 0 &&
  			  pg_strcasecmp(prev3_wd, "SEARCH") == 0))
#10Robert Haas
robertmhaas@gmail.com
In reply to: Shigeru HANADA (#9)
Re: Comments on SQL/Med objects

On Fri, Apr 1, 2011 at 9:40 AM, Shigeru HANADA
<hanada@metrosystems.co.jp> wrote:

On Thu, 31 Mar 2011 11:24:27 -0400
Robert Haas <robertmhaas@gmail.com> wrote:

Attached.  Foreign tables are already OK, I believe; it's only foreign
data wrappers and foreign servers that appear to need fixing.

The patch seems good for basic functionarity.  I've tested the patch
and noticed that get_foreign_data_wrapper_oid() is same as
GetForeignDataWrapperOidByName(), so they could be merged.  Also
GetForeignServerOidByName() could be merged.

I changed "foreign data wrapper" in message to "foreign-data wrapper"
for consistency, but it's revertable.

Please see merge_oid_funcs.patch which can be applied onto your patch.

Thanks for the review, good catches. Committed those two patches
together with a bit of further rearrangement.

I think some supports can be added for comments on SQL/MED objects.

 - pg_dump support for comment on fdw and server
 - psql describe commands (\dew+ and \des+)
 - psql TAB completion

Please see attached patches for each feature.

I'll take a look at these next.

While testing pg_dump, I noticed that comment of extension's member
objects are not dumped by pg_dump.  Those comments should be dumped
after CREATE EXTENSION statement?

No, I don't believe that would be correct.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#11Robert Haas
robertmhaas@gmail.com
In reply to: Shigeru HANADA (#9)
Re: Comments on SQL/Med objects

On Fri, Apr 1, 2011 at 9:40 AM, Shigeru HANADA
<hanada@metrosystems.co.jp> wrote:

 - pg_dump support for comment on fdw and server

Applied, good catch, thanks.

 - psql describe commands (\dew+ and \des+)

Not sure if we want this behavior change or not. Any other opinions?
It doesn't look like there's any particular consistency in terms of
which backslash commands include a description always (e.g. \dT),
which ones include it only when + is specified (e.g. \dt), and which
don't include it at all (e.g. \dc).

 - psql TAB completion

Committed this also.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company