Thanks, Nathan, for reviewing the patch. Below are my comments inline:
On Thu, Mar 6, 2025 at 1:43 AM Nathan Bossart <nathandbossart@gmail.com> wrote:
>
>
> * The patch alleges to only block DROP ROLE commands when there exists
> _both_ admins of the target role and roles for which the target role is
> an admin. However, it's not clear to me why both need to be true. I
> might be able to glean the reason if I read this thread carefully or
> spend more time thinking about it, but IMHO that patch itself should make
> it obvious. I'd suggest expanding the comment atop
> check_drop_role_dependency().
>
I'll update the comments above the check_drop_role_dependency()
function to clarify things.
> * Does this introduce any race conditions? For example, is it possible for
> the new check to pass and then for a dependency to be added before the
> drop completes?
>
I believe it is; I may need to adjust the location from where I'm
calling check_drop_role_dependency() to take care of this. I'll
address this in the next patch version. Thanks for bringing up this
concern.
--
With Regards,
Ashutosh Sharma.
