From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | "Bossart, Nathan" <bossartn(at)amazon(dot)com> |
Cc: | Joshua Brindle <joshua(dot)brindle(at)crunchydata(dot)com>, Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com>, 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 18:00:33 |
Message-ID: | 254275.1635357633@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
"Bossart, Nathan" <bossartn(at)amazon(dot)com> writes:
> 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:
> I think a comment about the intended usage is sufficient.
I agree with the position that a better function header comment is
sufficient. However, if we're to rename it, please not "can_set_role".
That's bad in at least two ways:
* it's quite unclear what "set" means in this context ... maybe the
function is testing whether you're allowed to alter properties of
the role, or do "ALTER ROLE ... SET parameter = value"? It only makes
sense if you already know that the specific command SET ROLE is being
thought of.
* it's unclear which argument is which end of the relationship.
Something like "can_set_role_to()" would help with the second problem,
but I'm not sure it does much for the first one.
On the whole I think the existing name is fine.
regards, tom lane
From | Date | Subject | |
---|---|---|---|
Next Message | Bossart, Nathan | 2021-10-27 19:05:10 | Re: Isn't it better with "autovacuum worker...." instead of "worker took too long to start; canceled" specific to "auto |
Previous Message | Joshua Brindle | 2021-10-27 17:54:06 | Re: [PATCH] Conflation of member/privs for predefined roles |