When commit cae565e503 introduced FDW user mappings, it used this in
getObjectDescription for them:

        appendStringInfo(&buffer, _("user mapping for %s"), usename);

This was later mostly copied (by yours truly) as object identity by
commit f8348ea32e wherein I used this:
        appendStringInfo(&buffer, "%s", usename);

As it turns out, this is wrong, because the pg_user_mapping catalog has
a two-column "primary key" which is user OID and server OID.  Therefore
it seems to me that the correct object description and identity must
include both username and server name.  I propose we change the above to
this:

        appendStringInfo(&buffer, _("user mapping for %s in server %s"), 
usename,
                                 srv->servername);

I propose to apply the (appropriately rebased) attached patch to all
back branches.  Note in particular the wording changes in some error
messages in the foreign_data test.

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From 82e8814c9f4b89d31d51b2fa370add1c04ae0f12 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvhe...@alvh.no-ip.org>
Date: Thu, 5 Mar 2015 12:30:53 -0300
Subject: [PATCH] fix user mapping object description/identity

---
 src/backend/catalog/objectaddress.c        | 30 ++++++++++++++++++++----------
 src/test/regress/expected/foreign_data.out | 24 ++++++++++++------------
 2 files changed, 32 insertions(+), 22 deletions(-)

diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c
index 70043fc..0740b4f 100644
--- a/src/backend/catalog/objectaddress.c
+++ b/src/backend/catalog/objectaddress.c
@@ -2510,14 +2510,17 @@ getObjectDescription(const ObjectAddress *object)
 				HeapTuple	tup;
 				Oid			useid;
 				char	   *usename;
+				Form_pg_user_mapping umform;
+				ForeignServer *srv;
 
 				tup = SearchSysCache1(USERMAPPINGOID,
 									  ObjectIdGetDatum(object->objectId));
 				if (!HeapTupleIsValid(tup))
 					elog(ERROR, "cache lookup failed for user mapping %u",
 						 object->objectId);
-
-				useid = ((Form_pg_user_mapping) GETSTRUCT(tup))->umuser;
+				umform = (Form_pg_user_mapping) GETSTRUCT(tup);
+				useid = umform->umuser;
+				srv = GetForeignServer(umform->umserver);
 
 				ReleaseSysCache(tup);
 
@@ -2526,7 +2529,8 @@ getObjectDescription(const ObjectAddress *object)
 				else
 					usename = "public";
 
-				appendStringInfo(&buffer, _("user mapping for %s"), usename);
+				appendStringInfo(&buffer, _("user mapping for %s in server %s"), usename,
+								 srv->servername);
 				break;
 			}
 
@@ -3906,19 +3910,18 @@ getObjectIdentityParts(const ObjectAddress *object,
 			{
 				HeapTuple	tup;
 				Oid			useid;
+				Form_pg_user_mapping umform;
+				ForeignServer *srv;
 				const char *usename;
 
-				/* no objname support */
-				if (objname)
-					*objname = NIL;
-
 				tup = SearchSysCache1(USERMAPPINGOID,
 									  ObjectIdGetDatum(object->objectId));
 				if (!HeapTupleIsValid(tup))
 					elog(ERROR, "cache lookup failed for user mapping %u",
 						 object->objectId);
-
-				useid = ((Form_pg_user_mapping) GETSTRUCT(tup))->umuser;
+				umform = (Form_pg_user_mapping) GETSTRUCT(tup);
+				useid = umform->umuser;
+				srv = GetForeignServer(umform->umserver);
 
 				ReleaseSysCache(tup);
 
@@ -3927,7 +3930,14 @@ getObjectIdentityParts(const ObjectAddress *object,
 				else
 					usename = "public";
 
-				appendStringInfoString(&buffer, usename);
+				if (objname)
+				{
+					*objname = list_make1(pstrdup(usename));
+					*objargs = list_make1(pstrdup(srv->servername));
+				}
+
+				appendStringInfo(&buffer, "%s in server %s", usename,
+								 srv->servername);
 				break;
 			}
 
diff --git a/src/test/regress/expected/foreign_data.out b/src/test/regress/expected/foreign_data.out
index 632b7e5..4d0de1f 100644
--- a/src/test/regress/expected/foreign_data.out
+++ b/src/test/regress/expected/foreign_data.out
@@ -247,7 +247,7 @@ CREATE USER MAPPING FOR current_user SERVER s1;
 DROP FOREIGN DATA WRAPPER foo;                              -- ERROR
 ERROR:  cannot drop foreign-data wrapper foo because other objects depend on it
 DETAIL:  server s1 depends on foreign-data wrapper foo
-user mapping for foreign_data_user depends on server s1
+user mapping for foreign_data_user in server s1 depends on server s1
 HINT:  Use DROP ... CASCADE to drop the dependent objects too.
 SET ROLE regress_test_role;
 DROP FOREIGN DATA WRAPPER foo CASCADE;                      -- ERROR
@@ -256,7 +256,7 @@ RESET ROLE;
 DROP FOREIGN DATA WRAPPER foo CASCADE;
 NOTICE:  drop cascades to 2 other objects
 DETAIL:  drop cascades to server s1
-drop cascades to user mapping for foreign_data_user
+drop cascades to user mapping for foreign_data_user in server s1
 \dew+
                                             List of foreign-data wrappers
     Name    |       Owner       | Handler |        Validator         | Access privileges | FDW Options | Description 
@@ -526,10 +526,10 @@ CREATE USER MAPPING FOR current_user SERVER s3;
 
 DROP SERVER s3;                                             -- ERROR
 ERROR:  cannot drop server s3 because other objects depend on it
-DETAIL:  user mapping for foreign_data_user depends on server s3
+DETAIL:  user mapping for foreign_data_user in server s3 depends on server s3
 HINT:  Use DROP ... CASCADE to drop the dependent objects too.
 DROP SERVER s3 CASCADE;
-NOTICE:  drop cascades to user mapping for foreign_data_user
+NOTICE:  drop cascades to user mapping for foreign_data_user in server s3
 \des
              List of foreign servers
  Name |       Owner       | Foreign-data wrapper 
@@ -1183,8 +1183,8 @@ GRANT USAGE ON FOREIGN SERVER s9 TO regress_test_role;
 CREATE USER MAPPING FOR current_user SERVER s9;
 DROP SERVER s9 CASCADE;
 NOTICE:  drop cascades to 2 other objects
-DETAIL:  drop cascades to user mapping for public
-drop cascades to user mapping for unprivileged_role
+DETAIL:  drop cascades to user mapping for public in server s9
+drop cascades to user mapping for unprivileged_role in server s9
 RESET ROLE;
 CREATE SERVER s9 FOREIGN DATA WRAPPER foo;
 GRANT USAGE ON FOREIGN SERVER s9 TO unprivileged_role;
@@ -1256,14 +1256,14 @@ DROP ROLE regress_test_role;                                -- ERROR
 ERROR:  role "regress_test_role" cannot be dropped because some objects depend on it
 DETAIL:  privileges for server s4
 privileges for foreign-data wrapper foo
-owner of user mapping for regress_test_role
-owner of user mapping for regress_test_role
+owner of user mapping for regress_test_role in server s6
+owner of user mapping for regress_test_role in server s5
 owner of server s5
 owner of server t2
 DROP SERVER s5 CASCADE;
-NOTICE:  drop cascades to user mapping for regress_test_role
+NOTICE:  drop cascades to user mapping for regress_test_role in server s5
 DROP SERVER t1 CASCADE;
-NOTICE:  drop cascades to user mapping for public
+NOTICE:  drop cascades to user mapping for public in server t1
 DROP SERVER t2;
 DROP USER MAPPING FOR regress_test_role SERVER s6;
 -- This test causes some order dependent cascade detail output,
@@ -1274,8 +1274,8 @@ NOTICE:  drop cascades to 5 other objects
 \set VERBOSITY default
 DROP SERVER s8 CASCADE;
 NOTICE:  drop cascades to 2 other objects
-DETAIL:  drop cascades to user mapping for foreign_data_user
-drop cascades to user mapping for public
+DETAIL:  drop cascades to user mapping for foreign_data_user in server s8
+drop cascades to user mapping for public in server s8
 DROP ROLE regress_test_indirect;
 DROP ROLE regress_test_role;
 DROP ROLE unprivileged_role;                                -- ERROR
-- 
2.1.4

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to