pgsql: Support comments on FOREIGN DATA WRAPPER and SERVER objects.

From 32a8bc339f47788293228a0f61f4f244ee969a51 Mon Sep 17 00:00:00 2001
From: Shigeru Hanada <hanada@metrosystems.co.jp>
Date: Mon, 4 Apr 2011 19:07:42 +0900
Subject: [PATCH 1/5] Implement COMMENT ON USER MAPPING.

Privilege to COMMENT is similar to ALTER/DROP USER MAPPING.
Needs updates for regression tests and documents.
---
 src/backend/catalog/aclchk.c        |   43 +++++++++++++++++++++++++++++++++++
 src/backend/catalog/objectaddress.c |   31 +++++++++++++++++++++++++
 src/backend/foreign/foreign.c       |   36 +++++++++++++++++++++++++++++
 src/backend/parser/gram.y           |    8 ++++++
 src/include/foreign/foreign.h       |    2 +
 src/include/nodes/parsenodes.h      |    1 +
 src/include/utils/acl.h             |    1 +
 7 files changed, 122 insertions(+), 0 deletions(-)

diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c
index aa3d59d..bbc6362 100644
--- a/src/backend/catalog/aclchk.c
+++ b/src/backend/catalog/aclchk.c
@@ -44,6 +44,7 @@
 #include "catalog/pg_type.h"
 #include "catalog/pg_ts_config.h"
 #include "catalog/pg_ts_dict.h"
+#include "catalog/pg_user_mapping.h"
 #include "commands/dbcommands.h"
 #include "commands/proclang.h"
 #include "commands/tablespace.h"
@@ -4643,6 +4644,48 @@ pg_foreign_server_ownercheck(Oid srv_oid, Oid roleid)
 }
 
 /*
+ * Ownership check for a user mapping (specified by OID).
+ *
+ * User mappings don't have owner, so we treat some users as the owner:
+ *  (1) owner of the foreign server
+ *  (2) user whose name matches the user name of the mapping exactly, and has
+ *      USAGE privilege on the server.
+ */
+bool
+pg_user_mapping_ownercheck(Oid um_oid, Oid roleid)
+{
+	HeapTuple	tuple;
+	Oid			userId;
+	Oid			serverId;
+
+	/* Superusers bypass all permission checking. */
+	if (superuser_arg(roleid))
+		return true;
+
+	tuple = SearchSysCache1(USERMAPPINGOID, ObjectIdGetDatum(um_oid));
+	if (!HeapTupleIsValid(tuple))
+		ereport(ERROR,
+				(errcode(ERRCODE_UNDEFINED_OBJECT),
+				 errmsg("user mapping with OID %u does not exist",
+						um_oid)));
+
+	userId = ((Form_pg_user_mapping) GETSTRUCT(tuple))->umuser;
+	serverId = ((Form_pg_user_mapping) GETSTRUCT(tuple))->umserver;
+
+	ReleaseSysCache(tuple);
+
+	if (userId == roleid)
+	{
+		AclResult	aclresult;
+
+		aclresult = pg_foreign_server_aclcheck(serverId, roleid, ACL_USAGE);
+		return (aclresult == ACLCHECK_OK);
+	}
+	else
+		return pg_foreign_server_ownercheck(serverId, roleid);
+}
+
+/*
  * Ownership check for a database (specified by OID).
  */
 bool
diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c
index 0d21d31..c322fd8 100644
--- a/src/backend/catalog/objectaddress.c
+++ b/src/backend/catalog/objectaddress.c
@@ -48,6 +48,7 @@
 #include "catalog/pg_ts_parser.h"
 #include "catalog/pg_ts_template.h"
 #include "catalog/pg_type.h"
+#include "catalog/pg_user_mapping.h"
 #include "commands/dbcommands.h"
 #include "commands/defrem.h"
 #include "commands/extension.h"
@@ -232,6 +233,22 @@ get_object_address(ObjectType objtype, List *objname, List *objargs,
 			address.objectId = get_ts_config_oid(objname, false);
 			address.objectSubId = 0;
 			break;
+		case OBJECT_USER_MAPPING:
+			{
+				char   *username;
+				char   *servername;
+
+				Assert(list_length(objname) == 2);
+
+				username = strVal(linitial(objname));
+				servername = strVal(lsecond(objname));
+				address.classId = UserMappingRelationId;
+				address.objectId = get_user_mapping_oid(username,
+														servername,
+														false);
+				address.objectSubId = 0;
+			}
+			break;
 		default:
 			elog(ERROR, "unrecognized objtype: %d", (int) objtype);
 			/* placate compiler, in case it thinks elog might return */
@@ -682,6 +699,9 @@ object_exists(ObjectAddress address)
 		case ForeignServerRelationId:
 			cache = FOREIGNSERVEROID;
 			break;
+		case UserMappingRelationId:
+			cache = USERMAPPINGOID;
+			break;
 		case TSParserRelationId:
 			cache = TSPARSEROID;
 			break;
@@ -795,6 +815,17 @@ check_object_ownership(Oid roleid, ObjectType objtype, ObjectAddress address,
 				aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_FOREIGN_SERVER,
 							   NameListToString(objname));
 			break;
+		case OBJECT_USER_MAPPING:
+			if (!pg_user_mapping_ownercheck(address.objectId, roleid))
+			{
+				char	   *username = strVal(linitial(objname));
+				ereport(ERROR,
+						(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+						 errmsg("must be owner of user mapping FOR %s SERVER %s",
+								username ? username : "public",
+								strVal(lsecond(objname)))));
+			}
+			break;
 		case OBJECT_LANGUAGE:
 			if (!pg_language_ownercheck(address.objectId, roleid))
 				aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_LANGUAGE,
diff --git a/src/backend/foreign/foreign.c b/src/backend/foreign/foreign.c
index cda90a6..a84358e 100644
--- a/src/backend/foreign/foreign.c
+++ b/src/backend/foreign/foreign.c
@@ -538,3 +538,39 @@ get_foreign_server_oid(const char *servername, bool missing_ok)
 				 errmsg("server \"%s\" does not exist", servername)));
 	return oid;
 }
+
+/*
+ * get_user_mapping_oid - given a USER name and SERVER name, look up the OID
+ *
+ * If missing_ok is false, throw an error if name not found.  If true, just
+ * return InvalidOid.
+ */
+Oid
+get_user_mapping_oid(const char *username, const char *servername,
+					 bool missing_ok)
+{
+	Oid			useroid;
+	Oid			serveroid;
+	Oid			oid;
+
+	/* Determine umuser of the mapping */
+	if (username == NULL)
+		useroid = 0;
+	else if (strcmp(username, "current_user") == 0)
+		useroid = GetUserId();
+	else
+		useroid = get_role_oid(username, false);
+
+	/* Determine umserver of the mapping */
+	serveroid = get_foreign_server_oid(servername, false);
+
+	oid = GetSysCacheOid2(USERMAPPINGUSERSERVER,
+						  ObjectIdGetDatum(useroid),
+						  ObjectIdGetDatum(serveroid));
+	if (!OidIsValid(oid) && !missing_ok)
+		ereport(ERROR,
+				(errcode(ERRCODE_UNDEFINED_OBJECT),
+				 errmsg("user mapping \"%s\" for the server does not exist",
+						MappingUserName(useroid))));
+	return oid;
+}
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index a22ab66..c234259 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -4954,6 +4954,14 @@ CommentStmt:
 					n->comment = $8;
 					$$ = (Node *) n;
 				}
+			| COMMENT ON USER MAPPING FOR auth_ident SERVER name IS comment_text
+				{
+					CommentStmt *n = makeNode(CommentStmt);
+					n->objtype = OBJECT_USER_MAPPING;
+					n->objname = list_make2(makeString($6), makeString($8));
+					n->comment = $10;
+					$$ = (Node *) n;
+				}
 		;
 
 comment_type:
diff --git a/src/include/foreign/foreign.h b/src/include/foreign/foreign.h
index 2fda9e3..f94d940 100644
--- a/src/include/foreign/foreign.h
+++ b/src/include/foreign/foreign.h
@@ -78,5 +78,7 @@ extern ForeignTable *GetForeignTable(Oid relid);
 
 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);
+extern Oid get_user_mapping_oid(const char *username, const char *servername,
+								bool missing_ok);
 
 #endif   /* FOREIGN_H */
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index d9eac76..c464c49 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -1133,6 +1133,7 @@ typedef enum ObjectType
 	OBJECT_TSPARSER,
 	OBJECT_TSTEMPLATE,
 	OBJECT_TYPE,
+	OBJECT_USER_MAPPING,
 	OBJECT_VIEW
 } ObjectType;
 
diff --git a/src/include/utils/acl.h b/src/include/utils/acl.h
index b28b764..a74bad5 100644
--- a/src/include/utils/acl.h
+++ b/src/include/utils/acl.h
@@ -317,6 +317,7 @@ 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_user_mapping_ownercheck(Oid um_oid, Oid roleid);
 extern bool pg_extension_ownercheck(Oid ext_oid, Oid roleid);
 extern bool has_createrole_privilege(Oid roleid);
 
-- 
1.7.3

From 7ef7ae7d0cb536576d7970b9ccb799addc1386cb Mon Sep 17 00:00:00 2001
From: Shigeru Hanada <hanada@metrosystems.co.jp>
Date: Mon, 4 Apr 2011 19:12:14 +0900
Subject: [PATCH 2/5] Add regression tests for COMMENT ON USER MAPPING.

---
 src/test/regress/expected/foreign_data.out |    3 +++
 src/test/regress/sql/foreign_data.sql      |    3 +++
 2 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/src/test/regress/expected/foreign_data.out b/src/test/regress/expected/foreign_data.out
index c05bcab..81298f6 100644
--- a/src/test/regress/expected/foreign_data.out
+++ b/src/test/regress/expected/foreign_data.out
@@ -567,6 +567,7 @@ RESET ROLE;
 (7 rows)
 
 -- ALTER USER MAPPING
+COMMENT ON USER MAPPING FOR current_user SERVER s8 IS 'postgres on s8';
 ALTER USER MAPPING FOR regress_test_missing_role SERVER s4 OPTIONS (gotcha 'true'); -- ERROR
 ERROR:  role "regress_test_missing_role" does not exist
 ALTER USER MAPPING FOR user SERVER ss4 OPTIONS (gotcha 'true'); -- ERROR
@@ -578,6 +579,8 @@ ERROR:  invalid option "username"
 HINT:  Valid options in this context are: user, password
 ALTER USER MAPPING FOR current_user SERVER s8 OPTIONS (DROP user, SET password 'public');
 SET ROLE regress_test_role;
+COMMENT ON USER MAPPING FOR regress_test_role SERVER s5 IS 'regress_test_role on s5';
+COMMENT ON USER MAPPING FOR public SERVER t1 IS 'public on t1';
 ALTER USER MAPPING FOR current_user SERVER s5 OPTIONS (ADD modified '1');
 ALTER USER MAPPING FOR public SERVER s4 OPTIONS (ADD modified '1'); -- ERROR
 ERROR:  must be owner of foreign server s4
diff --git a/src/test/regress/sql/foreign_data.sql b/src/test/regress/sql/foreign_data.sql
index 0d12b98..068d0c9 100644
--- a/src/test/regress/sql/foreign_data.sql
+++ b/src/test/regress/sql/foreign_data.sql
@@ -230,12 +230,15 @@ RESET ROLE;
 \deu
 
 -- ALTER USER MAPPING
+COMMENT ON USER MAPPING FOR current_user SERVER s8 IS 'postgres on s8';
 ALTER USER MAPPING FOR regress_test_missing_role SERVER s4 OPTIONS (gotcha 'true'); -- ERROR
 ALTER USER MAPPING FOR user SERVER ss4 OPTIONS (gotcha 'true'); -- ERROR
 ALTER USER MAPPING FOR public SERVER s5 OPTIONS (gotcha 'true');            -- ERROR
 ALTER USER MAPPING FOR current_user SERVER s8 OPTIONS (username 'test');    -- ERROR
 ALTER USER MAPPING FOR current_user SERVER s8 OPTIONS (DROP user, SET password 'public');
 SET ROLE regress_test_role;
+COMMENT ON USER MAPPING FOR regress_test_role SERVER s5 IS 'regress_test_role on s5';
+COMMENT ON USER MAPPING FOR public SERVER t1 IS 'public on t1';
 ALTER USER MAPPING FOR current_user SERVER s5 OPTIONS (ADD modified '1');
 ALTER USER MAPPING FOR public SERVER s4 OPTIONS (ADD modified '1'); -- ERROR
 ALTER USER MAPPING FOR public SERVER t1 OPTIONS (ADD modified '1');
-- 
1.7.3

From f672ce5d77d60f0422a6eae6c7db56e00a52eaec Mon Sep 17 00:00:00 2001
From: Shigeru Hanada <hanada@metrosystems.co.jp>
Date: Mon, 4 Apr 2011 19:25:22 +0900
Subject: [PATCH 3/5] Document about COMMENT ON USER MAPPING.

---
 doc/src/sgml/ref/comment.sgml |   30 ++++++++++++++++++++++++++++++
 doc/src/sgml/ref/grant.sgml   |    2 +-
 2 files changed, 31 insertions(+), 1 deletions(-)

diff --git a/doc/src/sgml/ref/comment.sgml b/doc/src/sgml/ref/comment.sgml
index 1cdc49f..c03691a 100644
--- a/doc/src/sgml/ref/comment.sgml
+++ b/doc/src/sgml/ref/comment.sgml
@@ -54,6 +54,7 @@ COMMENT ON
   TEXT SEARCH TEMPLATE <replaceable class="PARAMETER">object_name</replaceable> |
   TRIGGER <replaceable class="PARAMETER">trigger_name</replaceable> ON <replaceable class="PARAMETER">table_name</replaceable> |
   TYPE <replaceable class="PARAMETER">object_name</replaceable> |
+  USER MAPPING FOR { <replaceable class="PARAMETER">user_name</replaceable> | USER | CURRENT_USER | PUBLIC } SERVER <replaceable class="PARAMETER">server_name</replaceable> |
   VIEW <replaceable class="PARAMETER">object_name</replaceable>
 } IS '<replaceable class="PARAMETER">text</replaceable>'
 </synopsis>
@@ -78,6 +79,10 @@ COMMENT ON
    Roles don't have owners, so the rule for <literal>COMMENT ON ROLE</> is
    that you must be superuser to comment on a superuser role, or have the
    <literal>CREATEROLE</> privilege to comment on non-superuser roles.
+   User mappings don't have owners, so the rule for <literal>COMMENT ON USER
+   MAPPING</> is that the owner of a foreign server can comment on user
+   mappings for the server for any user.  In adittion, user who is granted
+   USAGE privilege on the SERVER can set the comment on his own mapping.
    Of course, a superuser can comment on anything.
   </para>
 
@@ -213,6 +218,30 @@ COMMENT ON
     </varlistentry>
 
    <varlistentry>
+    <term><replaceable>user_name</replaceable></term>
+    <term><literal>USER</literal></term>
+    <term><literal>CURRENT_USER</literal></term>
+    <term><literal>PUBLIC</literal></term>
+    <listitem>
+     <para>
+      The name of the user of the target user mapping.
+      <literal>CURRENT_USER</literal> and <literal>USER</literal> match the
+      name of the current user.  <literal>PUBLIC</literal> is used to match all
+      present and future user names in the system.
+     </para>
+    </listitem>
+   </varlistentry>
+
+   <varlistentry>
+    <term><replaceable>server_name</replaceable></term>
+    <listitem>
+     <para>
+      The name of the server of the target user mapping.
+     </para>
+    </listitem>
+   </varlistentry>
+
+   <varlistentry>
     <term><replaceable class="parameter">text</replaceable></term>
     <listitem>
      <para>
@@ -288,6 +317,7 @@ COMMENT ON TEXT SEARCH PARSER my_parser IS 'Splits text into words';
 COMMENT ON TEXT SEARCH TEMPLATE snowball IS 'Snowball stemmer';
 COMMENT ON TRIGGER my_trigger ON my_table IS 'Used for RI';
 COMMENT ON TYPE complex IS 'Complex number data type';
+COMMENT ON USER MAPPING FOR PUBLIC SERVER my_server IS 'Default mapping';
 COMMENT ON VIEW my_view IS 'View of departmental costs';
 </programlisting>
   </para>
diff --git a/doc/src/sgml/ref/grant.sgml b/doc/src/sgml/ref/grant.sgml
index 72ecc45..6a934d6 100644
--- a/doc/src/sgml/ref/grant.sgml
+++ b/doc/src/sgml/ref/grant.sgml
@@ -340,7 +340,7 @@ GRANT <replaceable class="PARAMETER">role_name</replaceable> [, ...] TO <replace
       </para>
       <para>
        For servers, this privilege enables the grantee to create,
-       alter, and drop his own user's user mappings associated with
+       alter, drop and comment his own user's user mappings associated with
        that server.  Also, it enables the grantee to query the options
        of the server and associated user mappings.
       </para>
-- 
1.7.3

From 42097594aa45ed1012e710adcc895b84c4b13407 Mon Sep 17 00:00:00 2001
From: Shigeru Hanada <hanada@metrosystems.co.jp>
Date: Mon, 4 Apr 2011 19:32:02 +0900
Subject: [PATCH 4/5] Fix pg_dump to dump COMMENT ON USER MAPPING statements.

---
 src/bin/pg_dump/pg_dump.c |   28 +++++++++++++++++++++++-----
 1 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 90cb9ab..925c4c0 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -11197,16 +11197,21 @@ dumpUserMappings(Archive *fout,
 	PQExpBuffer delq;
 	PQExpBuffer query;
 	PQExpBuffer tag;
+	PQExpBuffer label;
 	PGresult   *res;
 	int			ntups;
+	int			i_tableoid;
+	int			i_umid;
 	int			i_usename;
 	int			i_umoptions;
 	int			i;
+	CatalogId	catId;
 
 	q = createPQExpBuffer();
 	tag = createPQExpBuffer();
 	delq = createPQExpBuffer();
 	query = createPQExpBuffer();
+	label = createPQExpBuffer();
 
 	/*
 	 * We read from the publicly accessible view pg_user_mappings, so as not
@@ -11218,8 +11223,11 @@ dumpUserMappings(Archive *fout,
 	 */
 	selectSourceSchema("pg_catalog");
 
+	resetPQExpBuffer(query);
 	appendPQExpBuffer(query,
-					  "SELECT usename, "
+					  "SELECT 'pg_catalog.pg_user_mapping'::pg_catalog.regclass::oid AS tableoid, "
+					  "umid, "
+					  "usename, "
 					  "array_to_string(ARRAY(SELECT option_name || ' ' || quote_literal(option_value) FROM pg_options_to_table(umoptions)), ', ') AS umoptions\n"
 					  "FROM pg_user_mappings "
 					  "WHERE srvid = %u",
@@ -11229,6 +11237,8 @@ dumpUserMappings(Archive *fout,
 	check_sql_result(res, g_conn, query->data, PGRES_TUPLES_OK);
 
 	ntups = PQntuples(res);
+	i_tableoid = PQfnumber(res, "tableoid");
+	i_umid = PQfnumber(res, "umid");
 	i_usename = PQfnumber(res, "usename");
 	i_umoptions = PQfnumber(res, "umoptions");
 
@@ -11237,12 +11247,17 @@ dumpUserMappings(Archive *fout,
 		char	   *usename;
 		char	   *umoptions;
 
+		catId.tableoid = atooid(PQgetvalue(res, i, i_tableoid));
+		catId.oid = atooid(PQgetvalue(res, i, i_umid));
 		usename = PQgetvalue(res, i, i_usename);
 		umoptions = PQgetvalue(res, i, i_umoptions);
 
+		resetPQExpBuffer(label);
+		appendPQExpBuffer(label, "USER MAPPING FOR %s", fmtId(usename));
+		appendPQExpBuffer(label, " SERVER %s", fmtId(servername));
+
 		resetPQExpBuffer(q);
-		appendPQExpBuffer(q, "CREATE USER MAPPING FOR %s", fmtId(usename));
-		appendPQExpBuffer(q, " SERVER %s", fmtId(servername));
+		appendPQExpBuffer(q, "CREATE %s", label->data);
 
 		if (umoptions && strlen(umoptions) > 0)
 			appendPQExpBuffer(q, " OPTIONS (%s)", umoptions);
@@ -11250,8 +11265,7 @@ dumpUserMappings(Archive *fout,
 		appendPQExpBuffer(q, ";\n");
 
 		resetPQExpBuffer(delq);
-		appendPQExpBuffer(delq, "DROP USER MAPPING FOR %s", fmtId(usename));
-		appendPQExpBuffer(delq, " SERVER %s;\n", fmtId(servername));
+		appendPQExpBuffer(delq, "DROP %s", label->data);
 
 		resetPQExpBuffer(tag);
 		appendPQExpBuffer(tag, "USER MAPPING %s SERVER %s",
@@ -11266,10 +11280,14 @@ dumpUserMappings(Archive *fout,
 					 q->data, delq->data, NULL,
 					 &dumpId, 1,
 					 NULL, NULL);
+
+		/* Dump User Mapping Comments */
+		dumpComment(fout, label->data, NULL, owner, catId, 0, dumpId);
 	}
 
 	PQclear(res);
 
+	destroyPQExpBuffer(label);
 	destroyPQExpBuffer(query);
 	destroyPQExpBuffer(delq);
 	destroyPQExpBuffer(q);
-- 
1.7.3

From bdb270450a48ea11b9d672f4a57e79192b488131 Mon Sep 17 00:00:00 2001
From: Shigeru Hanada <hanada@metrosystems.co.jp>
Date: Mon, 4 Apr 2011 19:33:00 +0900
Subject: [PATCH 5/5] Fix psql to complete COMMENT ON USER MAPPING statement.

---
 src/bin/psql/tab-complete.c |   33 +++++++++++++++++++++++++++++----
 1 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 994c053..a6a4307 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1550,10 +1550,11 @@ psql_completion(char *text, int start, int end)
 	{
 		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};
+			"SERVER", "USER MAPPING FOR", "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);
 	}
@@ -1576,6 +1577,30 @@ psql_completion(char *text, int start, int end)
 
 		COMPLETE_WITH_LIST(list_TRANS2);
 	}
+	else if (pg_strcasecmp(prev3_wd, "COMMENT") == 0 &&
+			 pg_strcasecmp(prev2_wd, "ON") == 0 &&
+			 pg_strcasecmp(prev_wd, "USER") == 0)
+		COMPLETE_WITH_CONST("MAPPING FOR");
+	else if (pg_strcasecmp(prev4_wd, "COMMENT") == 0 &&
+			 pg_strcasecmp(prev3_wd, "ON") == 0 &&
+			 pg_strcasecmp(prev2_wd, "USER") == 0 &&
+			 pg_strcasecmp(prev_wd, "MAPPING") == 0)
+		COMPLETE_WITH_CONST("FOR");
+	else if (pg_strcasecmp(prev5_wd, "COMMENT") == 0 &&
+			 pg_strcasecmp(prev4_wd, "ON") == 0 &&
+			 pg_strcasecmp(prev3_wd, "USER") == 0 &&
+			 pg_strcasecmp(prev2_wd, "MAPPING") == 0 &&
+			 pg_strcasecmp(prev_wd, "FOR") == 0)
+		COMPLETE_WITH_QUERY(Query_for_list_of_roles
+							" UNION SELECT 'CURRENT_USER'"
+							" UNION SELECT 'PUBLIC'"
+							" UNION SELECT 'USER'");
+	else if (pg_strcasecmp(prev6_wd, "COMMENT") == 0 &&
+			 pg_strcasecmp(prev5_wd, "ON") == 0 &&
+			 pg_strcasecmp(prev4_wd, "USER") == 0 &&
+			 pg_strcasecmp(prev3_wd, "MAPPING") == 0 &&
+			 pg_strcasecmp(prev2_wd, "FOR") == 0)
+		COMPLETE_WITH_CONST("SERVER");
 	else if ((pg_strcasecmp(prev4_wd, "COMMENT") == 0 &&
 			  pg_strcasecmp(prev3_wd, "ON") == 0) ||
 			 (pg_strcasecmp(prev6_wd, "COMMENT") == 0 &&
-- 
1.7.3

diff --git a/doc/src/sgml/ref/comment.sgml b/doc/src/sgml/ref/comment.sgml
index 1cdc49f..4d08abf 100644
*** a/doc/src/sgml/ref/comment.sgml
--- b/doc/src/sgml/ref/comment.sgml
*************** COMMENT ON
*** 54,59 ****
--- 54,60 ----
    TEXT SEARCH TEMPLATE <replaceable class="PARAMETER">object_name</replaceable> |
    TRIGGER <replaceable class="PARAMETER">trigger_name</replaceable> ON <replaceable class="PARAMETER">table_name</replaceable> |
    TYPE <replaceable class="PARAMETER">object_name</replaceable> |
+   USER MAPPING FOR { <replaceable class="PARAMETER">user_name</replaceable> | USER | CURRENT_USER | PUBLIC } SERVER <replaceable class="PARAMETER">server_name</replaceable> |
    VIEW <replaceable class="PARAMETER">object_name</replaceable>
  } IS '<replaceable class="PARAMETER">text</replaceable>'
  </synopsis>
*************** COMMENT ON
*** 78,83 ****
--- 79,88 ----
     Roles don't have owners, so the rule for <literal>COMMENT ON ROLE</> is
     that you must be superuser to comment on a superuser role, or have the
     <literal>CREATEROLE</> privilege to comment on non-superuser roles.
+    User mappings don't have owners, so the rule for <literal>COMMENT ON USER
+    MAPPING</> is that the owner of a foreign server can comment on user
+    mappings for the server for any user.  In addition, user who is granted
+    USAGE privilege on the SERVER can set the comment on his own mapping.
     Of course, a superuser can comment on anything.
    </para>
  
*************** COMMENT ON
*** 213,218 ****
--- 218,244 ----
      </varlistentry>
  
     <varlistentry>
+     <term><replaceable>user_name</replaceable></term>
+     <listitem>
+      <para>
+       The name of the user of the target user mapping.
+       <literal>CURRENT_USER</literal> and <literal>USER</literal> match the
+       name of the current user.  <literal>PUBLIC</literal> is used to match all
+       present and future user names in the system.
+      </para>
+     </listitem>
+    </varlistentry>
+ 
+    <varlistentry>
+     <term><replaceable>server_name</replaceable></term>
+     <listitem>
+      <para>
+       The name of the server of the target user mapping.
+      </para>
+     </listitem>
+    </varlistentry>
+ 
+    <varlistentry>
      <term><replaceable class="parameter">text</replaceable></term>
      <listitem>
       <para>
*************** COMMENT ON TEXT SEARCH PARSER my_parser 
*** 288,293 ****
--- 314,320 ----
  COMMENT ON TEXT SEARCH TEMPLATE snowball IS 'Snowball stemmer';
  COMMENT ON TRIGGER my_trigger ON my_table IS 'Used for RI';
  COMMENT ON TYPE complex IS 'Complex number data type';
+ COMMENT ON USER MAPPING FOR PUBLIC SERVER my_server IS 'Default mapping';
  COMMENT ON VIEW my_view IS 'View of departmental costs';
  </programlisting>
    </para>
diff --git a/doc/src/sgml/ref/grant.sgml b/doc/src/sgml/ref/grant.sgml
index 72ecc45..6a934d6 100644
*** a/doc/src/sgml/ref/grant.sgml
--- b/doc/src/sgml/ref/grant.sgml
*************** GRANT <replaceable class="PARAMETER">rol
*** 340,346 ****
        </para>
        <para>
         For servers, this privilege enables the grantee to create,
!        alter, and drop his own user's user mappings associated with
         that server.  Also, it enables the grantee to query the options
         of the server and associated user mappings.
        </para>
--- 340,346 ----
        </para>
        <para>
         For servers, this privilege enables the grantee to create,
!        alter, drop and comment his own user's user mappings associated with
         that server.  Also, it enables the grantee to query the options
         of the server and associated user mappings.
        </para>
diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c
index aa3d59d..49d4841 100644
*** a/src/backend/catalog/aclchk.c
--- b/src/backend/catalog/aclchk.c
***************
*** 44,50 ****
--- 44,52 ----
  #include "catalog/pg_type.h"
  #include "catalog/pg_ts_config.h"
  #include "catalog/pg_ts_dict.h"
+ #include "catalog/pg_user_mapping.h"
  #include "commands/dbcommands.h"
+ #include "commands/defrem.h"
  #include "commands/proclang.h"
  #include "commands/tablespace.h"
  #include "foreign/foreign.h"
*************** pg_foreign_server_ownercheck(Oid srv_oid
*** 4643,4648 ****
--- 4645,4704 ----
  }
  
  /*
+  * Ownership check for a user mapping (specified by OID).
+  *
+  * User mappings don't have owner, so we treat users who can execute DDLs
+  * on the mapping as the owner.  Please see user_mapping_ddl_aclcheck() for
+  * detailed conditions.
+  *
+  * Note that this function issues error message and quits if the user was not
+  * an owner.
+  */
+ bool
+ pg_user_mapping_ownercheck(Oid um_oid, Oid roleid)
+ {
+ 	HeapTuple	tuple;
+ 	Oid			user_oid;
+ 	Oid			srv_oid;
+ 	NameData	srvname;
+ 
+ 	/* Superusers bypass all permission checking. */
+ 	if (superuser_arg(roleid))
+ 		return true;
+ 
+ 	/* Retrieve oid pair of the user mapping from syscache. */
+ 	tuple = SearchSysCache1(USERMAPPINGOID, ObjectIdGetDatum(um_oid));
+ 	if (!HeapTupleIsValid(tuple))
+ 		ereport(ERROR,
+ 				(errcode(ERRCODE_UNDEFINED_OBJECT),
+ 				 errmsg("user mapping with OID %u does not exist",
+ 						um_oid)));
+ 
+ 	user_oid = ((Form_pg_user_mapping) GETSTRUCT(tuple))->umuser;
+ 	srv_oid = ((Form_pg_user_mapping) GETSTRUCT(tuple))->umserver;
+ 
+ 	ReleaseSysCache(tuple);
+ 
+ 	/* Retrieve name of the server from syscache. */
+ 	tuple = SearchSysCache1(FOREIGNSERVEROID, ObjectIdGetDatum(srv_oid));
+ 	if (!HeapTupleIsValid(tuple))
+ 		ereport(ERROR,
+ 				(errcode(ERRCODE_UNDEFINED_OBJECT),
+ 				 errmsg("foreign server with OID %u does not exist",
+ 						srv_oid)));
+ 
+ 	strcpy(NameStr(srvname),
+ 		   NameStr(((Form_pg_foreign_server) GETSTRUCT(tuple))->srvname));
+ 
+ 	ReleaseSysCache(tuple);
+ 
+ 	/* Assume users who can execute DDLs on the user mapping as owner. */
+ 	user_mapping_ddl_aclcheck(user_oid, srv_oid, srvname.data);
+ 
+ 	return true;
+ }
+ 
+ /*
   * Ownership check for a database (specified by OID).
   */
  bool
diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c
index 0d21d31..630fb60 100644
*** a/src/backend/catalog/objectaddress.c
--- b/src/backend/catalog/objectaddress.c
***************
*** 48,53 ****
--- 48,54 ----
  #include "catalog/pg_ts_parser.h"
  #include "catalog/pg_ts_template.h"
  #include "catalog/pg_type.h"
+ #include "catalog/pg_user_mapping.h"
  #include "commands/dbcommands.h"
  #include "commands/defrem.h"
  #include "commands/extension.h"
*************** get_object_address(ObjectType objtype, L
*** 232,237 ****
--- 233,254 ----
  			address.objectId = get_ts_config_oid(objname, false);
  			address.objectSubId = 0;
  			break;
+ 		case OBJECT_USER_MAPPING:
+ 			{
+ 				char   *username;
+ 				char   *servername;
+ 
+ 				Assert(list_length(objname) == 2);
+ 
+ 				username = strVal(linitial(objname));
+ 				servername = strVal(lsecond(objname));
+ 				address.classId = UserMappingRelationId;
+ 				address.objectId = get_user_mapping_oid(username,
+ 														servername,
+ 														false);
+ 				address.objectSubId = 0;
+ 			}
+ 			break;
  		default:
  			elog(ERROR, "unrecognized objtype: %d", (int) objtype);
  			/* placate compiler, in case it thinks elog might return */
*************** object_exists(ObjectAddress address)
*** 682,687 ****
--- 699,707 ----
  		case ForeignServerRelationId:
  			cache = FOREIGNSERVEROID;
  			break;
+ 		case UserMappingRelationId:
+ 			cache = USERMAPPINGOID;
+ 			break;
  		case TSParserRelationId:
  			cache = TSPARSEROID;
  			break;
*************** check_object_ownership(Oid roleid, Objec
*** 795,800 ****
--- 815,824 ----
  				aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_FOREIGN_SERVER,
  							   NameListToString(objname));
  			break;
+ 		case OBJECT_USER_MAPPING:
+ 			/* pg_user_mapping_ownercheck() never returns on error case */
+ 			pg_user_mapping_ownercheck(address.objectId, roleid);
+ 			break;
  		case OBJECT_LANGUAGE:
  			if (!pg_language_ownercheck(address.objectId, roleid))
  				aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_LANGUAGE,
diff --git a/src/backend/commands/foreigncmds.c b/src/backend/commands/foreigncmds.c
index 13d6d88..2400fc4 100644
*** a/src/backend/commands/foreigncmds.c
--- b/src/backend/commands/foreigncmds.c
*************** transformGenericOptions(Oid catalogId,
*** 175,181 ****
  /*
   * Convert the user mapping user name to OID
   */
! static Oid
  GetUserOidFromMapping(const char *username, bool missing_ok)
  {
  	if (!username)
--- 175,181 ----
  /*
   * Convert the user mapping user name to OID
   */
! Oid
  GetUserOidFromMapping(const char *username, bool missing_ok)
  {
  	if (!username)
*************** RemoveForeignServerById(Oid srvId)
*** 1014,1024 ****
  
  
  /*
!  * Common routine to check permission for user-mapping-related DDL
!  * commands.  We allow server owners to operate on any mapping, and
!  * users to operate on their own mapping.
   */
! static void
  user_mapping_ddl_aclcheck(Oid umuserid, Oid serverid, const char *servername)
  {
  	Oid			curuserid = GetUserId();
--- 1014,1026 ----
  
  
  /*
!  * Common routine to check permission for user-mapping-related DDL commands,
!  * such as CREATE, ALTER, DROP and COMMENT ON.  We allow server owners to
!  * operate on any mapping, and users who have USAGE privilege on the server to
!  * operate on their own mapping.  Note that PUBLIC mapping can be operated by
!  * only server owners.
   */
! void
  user_mapping_ddl_aclcheck(Oid umuserid, Oid serverid, const char *servername)
  {
  	Oid			curuserid = GetUserId();
diff --git a/src/backend/foreign/foreign.c b/src/backend/foreign/foreign.c
index cda90a6..ce4f618 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"
*************** get_foreign_server_oid(const char *serve
*** 538,540 ****
--- 539,572 ----
  				 errmsg("server \"%s\" does not exist", servername)));
  	return oid;
  }
+ 
+ /*
+  * get_user_mapping_oid - given a USER name and SERVER name, look up the OID
+  *
+  * If missing_ok is false, throw an error if name not found.  If true, just
+  * return InvalidOid.
+  */
+ Oid
+ get_user_mapping_oid(const char *username, const char *servername,
+ 					 bool missing_ok)
+ {
+ 	Oid			useroid;
+ 	Oid			serveroid;
+ 	Oid			oid;
+ 
+ 	/* Determine umuser of the mapping */
+ 	useroid = GetUserOidFromMapping(username, false);
+ 
+ 	/* Determine umserver of the mapping */
+ 	serveroid = get_foreign_server_oid(servername, false);
+ 
+ 	oid = GetSysCacheOid2(USERMAPPINGUSERSERVER,
+ 						  ObjectIdGetDatum(useroid),
+ 						  ObjectIdGetDatum(serveroid));
+ 	if (!OidIsValid(oid) && !missing_ok)
+ 		ereport(ERROR,
+ 				(errcode(ERRCODE_UNDEFINED_OBJECT),
+ 				 errmsg("user mapping \"%s\" for the server does not exist",
+ 						MappingUserName(useroid))));
+ 	return oid;
+ }
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index a22ab66..c234259 100644
*** a/src/backend/parser/gram.y
--- b/src/backend/parser/gram.y
*************** CommentStmt:
*** 4954,4959 ****
--- 4954,4967 ----
  					n->comment = $8;
  					$$ = (Node *) n;
  				}
+ 			| COMMENT ON USER MAPPING FOR auth_ident SERVER name IS comment_text
+ 				{
+ 					CommentStmt *n = makeNode(CommentStmt);
+ 					n->objtype = OBJECT_USER_MAPPING;
+ 					n->objname = list_make2(makeString($6), makeString($8));
+ 					n->comment = $10;
+ 					$$ = (Node *) n;
+ 				}
  		;
  
  comment_type:
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 90cb9ab..cceb034 100644
*** a/src/bin/pg_dump/pg_dump.c
--- b/src/bin/pg_dump/pg_dump.c
*************** dumpUserMappings(Archive *fout,
*** 11197,11212 ****
--- 11197,11217 ----
  	PQExpBuffer delq;
  	PQExpBuffer query;
  	PQExpBuffer tag;
+ 	PQExpBuffer label;
  	PGresult   *res;
  	int			ntups;
+ 	int			i_tableoid;
+ 	int			i_umid;
  	int			i_usename;
  	int			i_umoptions;
  	int			i;
+ 	CatalogId	catId;
  
  	q = createPQExpBuffer();
  	tag = createPQExpBuffer();
  	delq = createPQExpBuffer();
  	query = createPQExpBuffer();
+ 	label = createPQExpBuffer();
  
  	/*
  	 * We read from the publicly accessible view pg_user_mappings, so as not
*************** dumpUserMappings(Archive *fout,
*** 11219,11225 ****
  	selectSourceSchema("pg_catalog");
  
  	appendPQExpBuffer(query,
! 					  "SELECT usename, "
  					  "array_to_string(ARRAY(SELECT option_name || ' ' || quote_literal(option_value) FROM pg_options_to_table(umoptions)), ', ') AS umoptions\n"
  					  "FROM pg_user_mappings "
  					  "WHERE srvid = %u",
--- 11224,11232 ----
  	selectSourceSchema("pg_catalog");
  
  	appendPQExpBuffer(query,
! 					  "SELECT 'pg_catalog.pg_user_mapping'::pg_catalog.regclass::oid AS tableoid, "
! 					  "umid, "
! 					  "usename, "
  					  "array_to_string(ARRAY(SELECT option_name || ' ' || quote_literal(option_value) FROM pg_options_to_table(umoptions)), ', ') AS umoptions\n"
  					  "FROM pg_user_mappings "
  					  "WHERE srvid = %u",
*************** dumpUserMappings(Archive *fout,
*** 11229,11234 ****
--- 11236,11243 ----
  	check_sql_result(res, g_conn, query->data, PGRES_TUPLES_OK);
  
  	ntups = PQntuples(res);
+ 	i_tableoid = PQfnumber(res, "tableoid");
+ 	i_umid = PQfnumber(res, "umid");
  	i_usename = PQfnumber(res, "usename");
  	i_umoptions = PQfnumber(res, "umoptions");
  
*************** dumpUserMappings(Archive *fout,
*** 11237,11248 ****
  		char	   *usename;
  		char	   *umoptions;
  
  		usename = PQgetvalue(res, i, i_usename);
  		umoptions = PQgetvalue(res, i, i_umoptions);
  
  		resetPQExpBuffer(q);
! 		appendPQExpBuffer(q, "CREATE USER MAPPING FOR %s", fmtId(usename));
! 		appendPQExpBuffer(q, " SERVER %s", fmtId(servername));
  
  		if (umoptions && strlen(umoptions) > 0)
  			appendPQExpBuffer(q, " OPTIONS (%s)", umoptions);
--- 11246,11262 ----
  		char	   *usename;
  		char	   *umoptions;
  
+ 		catId.tableoid = atooid(PQgetvalue(res, i, i_tableoid));
+ 		catId.oid = atooid(PQgetvalue(res, i, i_umid));
  		usename = PQgetvalue(res, i, i_usename);
  		umoptions = PQgetvalue(res, i, i_umoptions);
  
+ 		resetPQExpBuffer(label);
+ 		appendPQExpBuffer(label, "USER MAPPING FOR %s", fmtId(usename));
+ 		appendPQExpBuffer(label, " SERVER %s", fmtId(servername));
+ 
  		resetPQExpBuffer(q);
! 		appendPQExpBuffer(q, "CREATE %s", label->data);
  
  		if (umoptions && strlen(umoptions) > 0)
  			appendPQExpBuffer(q, " OPTIONS (%s)", umoptions);
*************** dumpUserMappings(Archive *fout,
*** 11250,11257 ****
  		appendPQExpBuffer(q, ";\n");
  
  		resetPQExpBuffer(delq);
! 		appendPQExpBuffer(delq, "DROP USER MAPPING FOR %s", fmtId(usename));
! 		appendPQExpBuffer(delq, " SERVER %s;\n", fmtId(servername));
  
  		resetPQExpBuffer(tag);
  		appendPQExpBuffer(tag, "USER MAPPING %s SERVER %s",
--- 11264,11270 ----
  		appendPQExpBuffer(q, ";\n");
  
  		resetPQExpBuffer(delq);
! 		appendPQExpBuffer(delq, "DROP %s", label->data);
  
  		resetPQExpBuffer(tag);
  		appendPQExpBuffer(tag, "USER MAPPING %s SERVER %s",
*************** dumpUserMappings(Archive *fout,
*** 11266,11275 ****
--- 11279,11292 ----
  					 q->data, delq->data, NULL,
  					 &dumpId, 1,
  					 NULL, NULL);
+ 
+ 		/* Dump User Mapping Comments */
+ 		dumpComment(fout, label->data, NULL, owner, catId, 0, dumpId);
  	}
  
  	PQclear(res);
  
+ 	destroyPQExpBuffer(label);
  	destroyPQExpBuffer(query);
  	destroyPQExpBuffer(delq);
  	destroyPQExpBuffer(q);
diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h
index 157ee39..60e5914 100644
*** a/src/include/commands/defrem.h
--- b/src/include/commands/defrem.h
*************** extern Datum transformGenericOptions(Oid
*** 167,172 ****
--- 167,176 ----
  									 Datum oldOptions,
  									 List *options,
  									 Oid fdwvalidator);
+ extern Oid GetUserOidFromMapping(const char *username, bool missing_ok);
+ extern void user_mapping_ddl_aclcheck(Oid umuserid,
+ 									  Oid serverid,
+ 									  const char *servername);
  
  /* support routines in commands/define.c */
  
diff --git a/src/include/foreign/foreign.h b/src/include/foreign/foreign.h
index 2fda9e3..f94d940 100644
*** a/src/include/foreign/foreign.h
--- b/src/include/foreign/foreign.h
*************** extern ForeignTable *GetForeignTable(Oid
*** 78,82 ****
--- 78,84 ----
  
  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);
+ extern Oid get_user_mapping_oid(const char *username, const char *servername,
+ 								bool missing_ok);
  
  #endif   /* FOREIGN_H */
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index d9eac76..c464c49 100644
*** a/src/include/nodes/parsenodes.h
--- b/src/include/nodes/parsenodes.h
*************** typedef enum ObjectType
*** 1133,1138 ****
--- 1133,1139 ----
  	OBJECT_TSPARSER,
  	OBJECT_TSTEMPLATE,
  	OBJECT_TYPE,
+ 	OBJECT_USER_MAPPING,
  	OBJECT_VIEW
  } ObjectType;
  
diff --git a/src/include/utils/acl.h b/src/include/utils/acl.h
index b28b764..a74bad5 100644
*** a/src/include/utils/acl.h
--- b/src/include/utils/acl.h
*************** extern bool pg_ts_dict_ownercheck(Oid di
*** 317,322 ****
--- 317,323 ----
  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_user_mapping_ownercheck(Oid um_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 c05bcab..81298f6 100644
*** a/src/test/regress/expected/foreign_data.out
--- b/src/test/regress/expected/foreign_data.out
*************** RESET ROLE;
*** 567,572 ****
--- 567,573 ----
  (7 rows)
  
  -- ALTER USER MAPPING
+ COMMENT ON USER MAPPING FOR current_user SERVER s8 IS 'postgres on s8';
  ALTER USER MAPPING FOR regress_test_missing_role SERVER s4 OPTIONS (gotcha 'true'); -- ERROR
  ERROR:  role "regress_test_missing_role" does not exist
  ALTER USER MAPPING FOR user SERVER ss4 OPTIONS (gotcha 'true'); -- ERROR
*************** ERROR:  invalid option "username"
*** 578,583 ****
--- 579,586 ----
  HINT:  Valid options in this context are: user, password
  ALTER USER MAPPING FOR current_user SERVER s8 OPTIONS (DROP user, SET password 'public');
  SET ROLE regress_test_role;
+ COMMENT ON USER MAPPING FOR regress_test_role SERVER s5 IS 'regress_test_role on s5';
+ COMMENT ON USER MAPPING FOR public SERVER t1 IS 'public on t1';
  ALTER USER MAPPING FOR current_user SERVER s5 OPTIONS (ADD modified '1');
  ALTER USER MAPPING FOR public SERVER s4 OPTIONS (ADD modified '1'); -- ERROR
  ERROR:  must be owner of foreign server s4
diff --git a/src/test/regress/sql/foreign_data.sql b/src/test/regress/sql/foreign_data.sql
index 0d12b98..068d0c9 100644
*** a/src/test/regress/sql/foreign_data.sql
--- b/src/test/regress/sql/foreign_data.sql
*************** RESET ROLE;
*** 230,241 ****
--- 230,244 ----
  \deu
  
  -- ALTER USER MAPPING
+ COMMENT ON USER MAPPING FOR current_user SERVER s8 IS 'postgres on s8';
  ALTER USER MAPPING FOR regress_test_missing_role SERVER s4 OPTIONS (gotcha 'true'); -- ERROR
  ALTER USER MAPPING FOR user SERVER ss4 OPTIONS (gotcha 'true'); -- ERROR
  ALTER USER MAPPING FOR public SERVER s5 OPTIONS (gotcha 'true');            -- ERROR
  ALTER USER MAPPING FOR current_user SERVER s8 OPTIONS (username 'test');    -- ERROR
  ALTER USER MAPPING FOR current_user SERVER s8 OPTIONS (DROP user, SET password 'public');
  SET ROLE regress_test_role;
+ COMMENT ON USER MAPPING FOR regress_test_role SERVER s5 IS 'regress_test_role on s5';
+ COMMENT ON USER MAPPING FOR public SERVER t1 IS 'public on t1';
  ALTER USER MAPPING FOR current_user SERVER s5 OPTIONS (ADD modified '1');
  ALTER USER MAPPING FOR public SERVER s4 OPTIONS (ADD modified '1'); -- ERROR
  ALTER USER MAPPING FOR public SERVER t1 OPTIONS (ADD modified '1');

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 994c053..a6a4307 100644
*** a/src/bin/psql/tab-complete.c
--- b/src/bin/psql/tab-complete.c
*************** psql_completion(char *text, int start, i
*** 1550,1559 ****
  	{
  		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);
  	}
--- 1550,1560 ----
  	{
  		static const char *const list_COMMENT[] =
  		{"CAST", "COLLATION", "CONVERSION", "DATABASE", "FOREIGN DATA WRAPPER",
! 			"SERVER", "USER MAPPING FOR", "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);
  	}
*************** psql_completion(char *text, int start, i
*** 1576,1581 ****
--- 1577,1606 ----
  
  		COMPLETE_WITH_LIST(list_TRANS2);
  	}
+ 	else if (pg_strcasecmp(prev3_wd, "COMMENT") == 0 &&
+ 			 pg_strcasecmp(prev2_wd, "ON") == 0 &&
+ 			 pg_strcasecmp(prev_wd, "USER") == 0)
+ 		COMPLETE_WITH_CONST("MAPPING FOR");
+ 	else if (pg_strcasecmp(prev4_wd, "COMMENT") == 0 &&
+ 			 pg_strcasecmp(prev3_wd, "ON") == 0 &&
+ 			 pg_strcasecmp(prev2_wd, "USER") == 0 &&
+ 			 pg_strcasecmp(prev_wd, "MAPPING") == 0)
+ 		COMPLETE_WITH_CONST("FOR");
+ 	else if (pg_strcasecmp(prev5_wd, "COMMENT") == 0 &&
+ 			 pg_strcasecmp(prev4_wd, "ON") == 0 &&
+ 			 pg_strcasecmp(prev3_wd, "USER") == 0 &&
+ 			 pg_strcasecmp(prev2_wd, "MAPPING") == 0 &&
+ 			 pg_strcasecmp(prev_wd, "FOR") == 0)
+ 		COMPLETE_WITH_QUERY(Query_for_list_of_roles
+ 							" UNION SELECT 'CURRENT_USER'"
+ 							" UNION SELECT 'PUBLIC'"
+ 							" UNION SELECT 'USER'");
+ 	else if (pg_strcasecmp(prev6_wd, "COMMENT") == 0 &&
+ 			 pg_strcasecmp(prev5_wd, "ON") == 0 &&
+ 			 pg_strcasecmp(prev4_wd, "USER") == 0 &&
+ 			 pg_strcasecmp(prev3_wd, "MAPPING") == 0 &&
+ 			 pg_strcasecmp(prev2_wd, "FOR") == 0)
+ 		COMPLETE_WITH_CONST("SERVER");
  	else if ((pg_strcasecmp(prev4_wd, "COMMENT") == 0 &&
  			  pg_strcasecmp(prev3_wd, "ON") == 0) ||
  			 (pg_strcasecmp(prev6_wd, "COMMENT") == 0 &&