From 3ff6c164cc1882e8b69d54b4a3ac73b33d4cd955 Mon Sep 17 00:00:00 2001 From: pranav jain Date: Thu, 29 May 2025 06:09:28 +0000 Subject: [PATCH 1/3] initial changes trial Signed-off-by: pranav jain --- contrib/babelfishpg_tsql/src/pl_handler.c | 118 ++++++++++++++++++++++ 1 file changed, 118 insertions(+) diff --git a/contrib/babelfishpg_tsql/src/pl_handler.c b/contrib/babelfishpg_tsql/src/pl_handler.c index 50954f43b3d..2b48a137095 100644 --- a/contrib/babelfishpg_tsql/src/pl_handler.c +++ b/contrib/babelfishpg_tsql/src/pl_handler.c @@ -96,6 +96,9 @@ #include "table_variable_mvcc.h" #include "access/xact.h" +#include +#include "utils/fmgroids.h" +#include "access/genam.h" extern int escape_hatch_set_transaction_isolation_level; extern bool pltsql_recursive_triggers; @@ -177,6 +180,7 @@ static void revoke_type_permission_from_public(PlannedStmt *pstmt, const char *q ProcessUtilityContext context, ParamListInfo params, QueryEnvironment *queryEnv, DestReceiver *dest, QueryCompletion *qc, List *type_name); static void set_current_query_is_create_tbl_check_constraint(Node *expr); static void validateUserAndRole(char *name); +static void shdepDropOwned_utility(List *roleoids, DropBehavior behavior); static void bbf_ExecDropStmt(DropStmt *stmt); @@ -2394,6 +2398,115 @@ validateUserAndRole(char *name) errmsg("'%s' is not a valid name because it contains invalid characters.", name))); } +void +shdepDropOwned_utility(List *roleids, DropBehavior behavior) +{ + Relation sdepRel; + ListCell *cell; + ObjectAddresses *deleteobjs; + + deleteobjs = new_object_addresses(); + + /* + * We don't need this strong a lock here, but we'll call routines that + * acquire RowExclusiveLock. Better get that right now to avoid potential + * deadlock failures. + */ + sdepRel = table_open(SharedDependRelationId, RowExclusiveLock); + + /* + * For each role, find the dependent objects and drop them using the + * regular (non-shared) dependency management. + */ + foreach(cell, roleids) + { + Oid roleid = lfirst_oid(cell); + ScanKeyData key[2]; + SysScanDesc scan; + HeapTuple tuple; + + /* Doesn't work for pinned objects */ + if (IsPinnedObject(AuthIdRelationId, roleid)) + { + ObjectAddress obj; + + obj.classId = AuthIdRelationId; + obj.objectId = roleid; + obj.objectSubId = 0; + + ereport(ERROR, + (errcode(ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST), + errmsg("cannot drop objects owned by %s because they are " + "required by the database system", + getObjectDescription(&obj, false)))); + } + + ScanKeyInit(&key[0], + Anum_pg_shdepend_refclassid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(AuthIdRelationId)); + ScanKeyInit(&key[1], + Anum_pg_shdepend_refobjid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(roleid)); + + scan = systable_beginscan(sdepRel, SharedDependReferenceIndexId, true, + NULL, 2, key); + + while ((tuple = systable_getnext(scan)) != NULL) + { + Form_pg_shdepend sdepForm = (Form_pg_shdepend) GETSTRUCT(tuple); + + /* + * We only operate on shared objects and objects in the current + * database + */ + if (sdepForm->dbid != MyDatabaseId && + sdepForm->dbid != InvalidOid) + continue; + + if (sdepForm->deptype == SHARED_DEPENDENCY_ACL) + { + + /* + * Dependencies on role grants are recorded using + * SHARED_DEPENDENCY_ACL, but unlike a regular ACL list + * which stores all permissions for a particular object in + * a single ACL array, there's a separate catalog row for + * each grant - so removing the grant just means removing + * the entire row. + */ + if (sdepForm->classid != AuthMemRelationId) + { + RemoveRoleFromObjectACL(roleid, + sdepForm->classid, + sdepForm->objid); + break; + } + /* FALLTHROUGH */ + + } + } + + systable_endscan(scan); + } + + /* + * For stability of deletion-report ordering, sort the objects into + * approximate reverse creation order before deletion. (This might also + * make the deletion go a bit faster, since there's less chance of having + * to rearrange the objects due to dependencies.) + */ + sort_object_addresses(deleteobjs); + + /* the dependency mechanism does the actual work */ + performMultipleDeletions(deleteobjs, behavior, 0); + + table_close(sdepRel, RowExclusiveLock); + + free_object_addresses(deleteobjs); +} + /* * Use this hook to handle utility statements that needs special treatment, and @@ -3824,6 +3937,7 @@ bbf_ProcessUtility(PlannedStmt *pstmt, Oid db_owner = get_db_owner_oid(db_name, false); Oid db_accessadmin = get_db_accessadmin_oid(db_name, false); Oid db_securityadmin = get_db_securityadmin_oid(db_name, false); + List *role_oids = NIL; foreach(item, stmt->roles) { @@ -3831,12 +3945,15 @@ bbf_ProcessUtility(PlannedStmt *pstmt, char *user_name; const char *db_principal_type = drop_user ? "user" : "role"; int role_oid; + Oid roleoid; if (rolspec->roletype == ROLESPEC_PUBLIC) rolspec->rolename = PUBLIC_ROLE_NAME; user_name = get_physical_user_name(db_name, rolspec->rolename, false, true); role_oid = get_role_oid(user_name, true); + roleoid = get_role_oid(user_name, true); + role_oids = list_append_unique_oid(role_oids, roleoid); if (!OidIsValid(role_oid) || /* Not found */ (drop_user && get_db_principal_kind(role_oid, db_name) != BBF_USER) || /* Found but not a user in current logical db */ @@ -3914,6 +4031,7 @@ bbf_ProcessUtility(PlannedStmt *pstmt, pfree(rolspec->rolename); rolspec->rolename = user_name; + shdepDropOwned_utility(role_oids,DROP_CASCADE); } } else From ff0638a54278d19631400fa41d0440f0abbbd330 Mon Sep 17 00:00:00 2001 From: pranav jain Date: Tue, 3 Jun 2025 20:42:46 +0000 Subject: [PATCH 2/3] adding basic tests Signed-off-by: pranav jain --- contrib/babelfishpg_tsql/src/pl_handler.c | 13 +++++-------- test/JDBC/expected/BABEL-ROLE.out | 23 +++++++++++++++++++++++ 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/contrib/babelfishpg_tsql/src/pl_handler.c b/contrib/babelfishpg_tsql/src/pl_handler.c index 2b48a137095..afe0859f892 100644 --- a/contrib/babelfishpg_tsql/src/pl_handler.c +++ b/contrib/babelfishpg_tsql/src/pl_handler.c @@ -2401,9 +2401,9 @@ validateUserAndRole(char *name) void shdepDropOwned_utility(List *roleids, DropBehavior behavior) { - Relation sdepRel; - ListCell *cell; - ObjectAddresses *deleteobjs; + Relation sdepRel; + ListCell *cell; + ObjectAddresses *deleteobjs; deleteobjs = new_object_addresses(); @@ -2415,8 +2415,7 @@ shdepDropOwned_utility(List *roleids, DropBehavior behavior) sdepRel = table_open(SharedDependRelationId, RowExclusiveLock); /* - * For each role, find the dependent objects and drop them using the - * regular (non-shared) dependency management. + * For each role, find the dependent objects and drop them only if */ foreach(cell, roleids) { @@ -3945,15 +3944,13 @@ bbf_ProcessUtility(PlannedStmt *pstmt, char *user_name; const char *db_principal_type = drop_user ? "user" : "role"; int role_oid; - Oid roleoid; if (rolspec->roletype == ROLESPEC_PUBLIC) rolspec->rolename = PUBLIC_ROLE_NAME; user_name = get_physical_user_name(db_name, rolspec->rolename, false, true); role_oid = get_role_oid(user_name, true); - roleoid = get_role_oid(user_name, true); - role_oids = list_append_unique_oid(role_oids, roleoid); + role_oids = list_append_unique_oid(role_oids, role_oid); if (!OidIsValid(role_oid) || /* Not found */ (drop_user && get_db_principal_kind(role_oid, db_name) != BBF_USER) || /* Found but not a user in current logical db */ diff --git a/test/JDBC/expected/BABEL-ROLE.out b/test/JDBC/expected/BABEL-ROLE.out index db6ad2c4119..63375993c19 100644 --- a/test/JDBC/expected/BABEL-ROLE.out +++ b/test/JDBC/expected/BABEL-ROLE.out @@ -783,6 +783,29 @@ GO DROP DATABASE db1 GO +--Drop user when user has some permissions +create database mydb +go +use mydb +go +create login u1 with password = '123' +go +create user u1 +go +create table t(a int) +go +grant select on t to u1 +go +drop user u1 +go +drop login u1 +go +drop table t +go +use master +GO +drop database mydb +GO -- Check if catalog is cleaned up SELECT rolname, type, orig_username, database_name FROM sys.babelfish_authid_user_ext From 2943ca50dc9cbc927c734ac3753a248075d55e96 Mon Sep 17 00:00:00 2001 From: pranav jain Date: Fri, 6 Jun 2025 07:48:37 +0000 Subject: [PATCH 3/3] add tests Signed-off-by: pranav jain --- test/JDBC/input/BABEL-ROLE.sql | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/test/JDBC/input/BABEL-ROLE.sql b/test/JDBC/input/BABEL-ROLE.sql index ec8c5a6f437..1da8b55ffe0 100644 --- a/test/JDBC/input/BABEL-ROLE.sql +++ b/test/JDBC/input/BABEL-ROLE.sql @@ -475,6 +475,29 @@ GO DROP DATABASE db1 GO +--Drop user when user has some permissions +create database mydb +go +use mydb +go +create login u1 with password = '123' +go +create user u1 +go +create table t(a int) +go +grant select on t to u1 +go +drop user u1 +go +drop login u1 +go +drop table t +go +use master +GO +drop database mydb +GO -- Check if catalog is cleaned up SELECT rolname, type, orig_username, database_name FROM sys.babelfish_authid_user_ext