Skip to content

Commit

Permalink
Fix drop database issue after MVU to PG17 (#474)
Browse files Browse the repository at this point in the history
After performing MVU to PG17, we weren't able to drop the existing database because a new dependency was getting added between the dbo system role and physical babelfish database. This dependency is getting introduced when running the TSQL upgrade script babelfishpg_tsql--3.4.0--4.0.0.sql, which grants permissions to the bbf_role_admin role. The new dependency is expected and is a result of a community change 514a3de. However, even though there is a function RemoveRoleFromInitPriv() to clean up these dependencies when executing DROP OWNED BY commands, it is not working correctly for system roles. This is because the function assumes the grantor of rights is always the same as the owner, which is not the case for system roles, where the grantor should be sysadmin.

This commit made changes to use sysadmin as grantor when generating ACLs for physical babelfish database while removing role from initial privileges.

Related Task: BABEL-5410 
Signed-off-by: Sumit Jaiswal [email protected]
  • Loading branch information
sumitj824 authored Nov 14, 2024
1 parent 3b3c493 commit f9e9557
Showing 1 changed file with 21 additions and 1 deletion.
22 changes: 21 additions & 1 deletion src/backend/catalog/aclchk.c
Original file line number Diff line number Diff line change
Expand Up @@ -4942,6 +4942,7 @@ RemoveRoleFromInitPriv(Oid roleid, Oid classid, Oid objid, int32 objsubid)
int nnewmembers;
Oid *oldmembers;
Oid *newmembers;
Oid grantorId;

/* Search for existing pg_init_privs entry for the target object. */
rel = table_open(InitPrivsRelationId, RowExclusiveLock);
Expand Down Expand Up @@ -5005,14 +5006,33 @@ RemoveRoleFromInitPriv(Oid roleid, Oid classid, Oid objid, int32 objsubid)
* Generate new ACL. Grantor of rights is always the same as the owner.
*/
if (old_acl != NULL)
{
Oid sysadminOid;
const char *babelfish_db_name;

/*
* For babelfish database, grantor will be sysadmin instead of object owner.
*/
if (bbf_get_sysadmin_oid_hook &&
classid == DatabaseRelationId &&
(babelfish_db_name = GetConfigOption("babelfishpg_tsql.database_name", true, false)) &&
objid == get_database_oid(babelfish_db_name, true) &&
is_member_of_role(GetUserId(), sysadminOid = (*bbf_get_sysadmin_oid_hook)()))
{
grantorId = sysadminOid;
}
else
grantorId = ownerId;

new_acl = merge_acl_with_grant(old_acl,
false, /* is_grant */
false, /* grant_option */
DROP_RESTRICT,
list_make1_oid(roleid),
ACLITEM_ALL_PRIV_BITS,
ownerId,
grantorId,
ownerId);
}
else
new_acl = NULL; /* this case shouldn't happen, probably */

Expand Down

0 comments on commit f9e9557

Please sign in to comment.