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