diff --git a/doc/src/sgml/ref/comment.sgml b/doc/src/sgml/ref/comment.sgml index bc848b306999abe7ebc1a917fdb52104c3adc9c7..1cdc49ff5fec1b5065d01820bb21837b9a22eff4 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 48fa6d48b7f49817ff3d87dc6fdddc818708bb19..aa3d59d4c9a31fbdebd07e1e62785f9f8d22d30d 100644 --- a/src/backend/catalog/aclchk.c +++ b/src/backend/catalog/aclchk.c @@ -683,7 +683,7 @@ objectNamesToOids(GrantObjectType objtype, List *objnames) foreach(cell, objnames) { char *fdwname = strVal(lfirst(cell)); - Oid fdwid = GetForeignDataWrapperOidByName(fdwname, false); + Oid fdwid = get_foreign_data_wrapper_oid(fdwname, false); objects = lappend_oid(objects, fdwid); } @@ -692,7 +692,7 @@ objectNamesToOids(GrantObjectType objtype, List *objnames) foreach(cell, objnames) { char *srvname = strVal(lfirst(cell)); - Oid srvid = GetForeignServerOidByName(srvname, false); + Oid srvid = get_foreign_server_oid(srvname, false); objects = lappend_oid(objects, srvid); } @@ -4588,6 +4588,33 @@ pg_ts_config_ownercheck(Oid cfg_oid, Oid roleid) return has_privs_of_role(roleid, ownerId); } +/* + * 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). */ diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c index 880b95df0200905ec428de0a3333a6ae7414372c..0d21d310a650a3b1f01cc4c554e4e6ac06b63fba 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" @@ -52,6 +54,7 @@ #include "commands/proclang.h" #include "commands/tablespace.h" #include "commands/trigger.h" +#include "foreign/foreign.h" #include "libpq/be-fsstubs.h" #include "miscadmin.h" #include "nodes/makefuncs.h" @@ -140,6 +143,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 +300,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 +351,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 +676,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 +785,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 +870,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 acd40c1f4e9edc3a12644efc8f90c85b88806e2c..13d6d882f8852989163e04c759bf71410c4977a0 100644 --- a/src/backend/commands/foreigncmds.c +++ b/src/backend/commands/foreigncmds.c @@ -686,7 +686,7 @@ RemoveForeignDataWrapper(DropFdwStmt *stmt) Oid fdwId; ObjectAddress object; - fdwId = GetForeignDataWrapperOidByName(stmt->fdwname, true); + fdwId = get_foreign_data_wrapper_oid(stmt->fdwname, true); if (!superuser()) ereport(ERROR, @@ -959,7 +959,7 @@ RemoveForeignServer(DropForeignServerStmt *stmt) Oid srvId; ObjectAddress object; - srvId = GetForeignServerOidByName(stmt->servername, true); + 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 44cd18177c6f673469780f8a3fd9d215b43fac6d..cda90a6b0cb83d0626d13ffba06f57df66de81c6 100644 --- a/src/backend/foreign/foreign.c +++ b/src/backend/foreign/foreign.c @@ -79,26 +79,6 @@ GetForeignDataWrapper(Oid fdwid) } -/* - * 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 @@ -107,7 +87,7 @@ GetForeignDataWrapperOidByName(const char *fdwname, bool missing_ok) ForeignDataWrapper * GetForeignDataWrapperByName(const char *fdwname, bool missing_ok) { - Oid fdwId = GetForeignDataWrapperOidByName(fdwname, missing_ok); + Oid fdwId = get_foreign_data_wrapper_oid(fdwname, missing_ok); if (!OidIsValid(fdwId)) return NULL; @@ -171,32 +151,13 @@ GetForeignServer(Oid serverid) } -/* - * 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); + Oid serverid = get_foreign_server_oid(srvname, missing_ok); if (!OidIsValid(serverid)) return NULL; @@ -538,3 +499,42 @@ postgresql_fdw_validator(PG_FUNCTION_ARGS) PG_RETURN_BOOL(true); } + +/* + * 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("server \"%s\" does not exist", servername))); + return oid; +} diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 27fdccae5b831fea616741a2d5000ceb2502ec9f..a22ab66ae5936c352686713ebb6c564872589576 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/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c index 691ba3bd95a510f37665d5ca187b5fdca232b020..2f27018b25648eaf68737b8ea5f08d49362f3d96 100644 --- a/src/backend/utils/adt/acl.c +++ b/src/backend/utils/adt/acl.c @@ -3162,7 +3162,7 @@ convert_foreign_data_wrapper_name(text *fdwname) { char *fdwstr = text_to_cstring(fdwname); - return GetForeignDataWrapperOidByName(fdwstr, false); + return get_foreign_data_wrapper_oid(fdwstr, false); } /* @@ -3928,7 +3928,7 @@ convert_server_name(text *servername) { char *serverstr = text_to_cstring(servername); - return GetForeignServerOidByName(serverstr, false); + return get_foreign_server_oid(serverstr, false); } /* diff --git a/src/include/foreign/foreign.h b/src/include/foreign/foreign.h index d676f3fce7451cbf880eb0c1c063a9250f20cb68..2fda9e39feb14fd4df2a7b45800c28d6d7ea9d0d 100644 --- a/src/include/foreign/foreign.h +++ b/src/include/foreign/foreign.h @@ -70,12 +70,13 @@ typedef struct ForeignTable 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); +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); + #endif /* FOREIGN_H */ diff --git a/src/include/utils/acl.h b/src/include/utils/acl.h index e96323efcc7e335c7797b28a1a4b43e86afcbc7b..b28b764fd528e976daba62efc46912ceb1467732 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 a7473349fdf0cad58f0a3ff8b015fee73eaf2550..c05bcabb71e32b1e321e1664a66babf6d730b9ed 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 3f3978590319583ca9fe173160c287d3655fdc87..0d12b98e2172e94e09947b977a7b41d6e4aaf532 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+