Re: [PATCH] remove is_member_of_role() from header, add can_set_role()

From: "Bossart, Nathan" <bossartn(at)amazon(dot)com>
To: Joshua Brindle <joshua(dot)brindle(at)crunchydata(dot)com>, Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [PATCH] remove is_member_of_role() from header, add can_set_role()
Date: 2021-10-27 17:30:31
Message-ID: 4A997FCC-C08C-440B-80C0-9957D892E9DD@amazon.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 10/27/21, 10:22 AM, "Joshua Brindle" <joshua(dot)brindle(at)crunchydata(dot)com> wrote:
> On Wed, Oct 27, 2021 at 1:12 PM Mark Dilger
> <mark(dot)dilger(at)enterprisedb(dot)com> wrote:
>> I don't understand the purpose of this. You are defining can_set_role(member,role) as a simple wrapper around is_member_of_role(member,role). Couldn't the comment:
>>
>> + *
>> + * Do not use this for privilege checking, instead use has_privs_of_role()
>>
>> be added to the header for is_member_of_role() without needing the new wrapper function?
>
> It could be, but the intent is to dissuade it from being used, so
> getting rid of it and making an explicit version that has a sole use
> seemed useful.
>
> It's possible that it's being used inappropriately out-of-tree so this
> would also prevent that.

I think a comment about the intended usage is sufficient. However,
renaming the function or replacing it with a wrapper might break
extensions and encourage the authors to reevaluate. That could be a
good thing.

Nathan

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bossart, Nathan 2021-10-27 17:34:13 Re: [PATCH] Conflation of member/privs for predefined roles
Previous Message Joshua Brindle 2021-10-27 17:27:14 Re: [PATCH] Conflation of member/privs for predefined roles