On Mon, Mar 10, 2025 at 11:15:04AM +0530, Ashutosh Sharma wrote:
> On Fri, Mar 7, 2025 at 10:55 PM Nathan Bossart <nathandboss...@gmail.com>
> wrote:
>> I noticed that much of this code is lifted from DropRole(), and the new
>> check_drop_role_dependency() function is only used by DropRole() right
>> before it does the exact same scans. Couldn't we put the new dependency
>> detection in those existing scans in DropRole()?
>
> It can be done, but mixing the code that checks for the drop role
> dependency with the code that removes entries for the role being
> dropped from pg_auth_members could reduce clarity and precision. This
> is more of a sanity check which I felt was necessary before we proceed
> with actually dropping the role, starting with the deletion of drop
> role entries from the system catalogs. I’m aware there’s some code
> duplication, but I think it should be fine.
Looking closer, we probably need to move this check to the second pass,
anyway:
postgres=# CREATE ROLE a CREATEROLE;
CREATE ROLE
postgres=# SET ROLE a;
SET
postgres=> CREATE ROLE b CREATEROLE;
CREATE ROLE
postgres=> SET ROLE b;
SET
postgres=> CREATE ROLE c;
CREATE ROLE
postgres=> RESET ROLE;
RESET
postgres=# DROP ROLE b, c;
ERROR: role "b" cannot be dropped because some objects depend on it
DETAIL: role a inherits ADMIN privileges on role c through role b
postgres=# DROP ROLE c, b;
DROP ROLE
The first DROP ROLE should probably succeed, if for no other reason than
individually dropping role c followed by role b would succeed.
--
nathan
