Re: [PATCH v2] use has_privs_for_role for predefined roles

On 3/28/22 15:56, Robert Haas wrote:
> On Mon, Mar 21, 2022 at 4:15 PM Joe Conway <mail@joeconway.com> wrote:
>> Robert -- any opinion on this? If I am not mistaken it is code that you
>> are actively working on.
> 
> Woops, I only just saw this. I don't mind if you want to change the
> calls to is_member_of_role() in basebackup_server.c and
> basebackup_to_shell.c to has_privs_of_role().

No worries -- I will take care of that shortly.

> However, it's not clear to me why it's different than the calls we
> have in other places, like calculate_database_size() and the
> relatively widely-used check_is_member_of_role().

I will have to go refresh my memory, but when I looked at those sites 
closely it all made sense to me.

I think most if not all of them were checking for the ability to switch 
to the other role, not actually checking for privileges by virtue of 
belonging to that role.

> As long as we have a bunch of different practices in different parts
> of the code base I can't see people getting this right consistently
> ... leaving aside any possible disagreement about which way is
> "right".
When I take the next pass I can consider whether additional comments 
will help and report back.

Joe
-- 
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development