From: | Stephen Frost <sfrost(at)snowman(dot)net> |
---|---|
To: | "Brightwell, Adam" <adam(dot)brightwell(at)crunchydatasolutions(dot)com> |
Cc: | PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: superuser() shortcuts |
Date: | 2014-10-02 00:42:46 |
Message-ID: | 20141002004246.GJ28859@tamriel.snowman.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Adam,
* Brightwell, Adam (adam(dot)brightwell(at)crunchydatasolutions(dot)com) wrote:
> > We, as a community, have gotten flak from time-to-time about the
> > superuser role. We also tend to wish to avoid unnecessary
> > optimization as it complicates the code base and makes folks reviewing
> > the code wonder at the exceptions.
>
> I have attached a patch for consideration/discussion that addresses these
> items. I have based it off of current master (0c013e0). I have done some
> cursory testing including check-world with success.
Thanks! Please add it to the next commitfest.
> With all the other pg_has_* functions being found in acl.c I agree that it
> would seem odd that this (or any other related function) would be found
> elsewhere. Since aclchk.c also has a couple of functions that follow the
> pattern of "has_<priv>_privilege", I moved this function there, for now, as
> well as modified it to include superuser checks as they were not previously
> there. The only related function I found in acl.c was "has_rolinherit"
> which is a static function. :-/ There is also a static function
> "has_rolcatupdate" in aclchk.c. I would agree that acl.c (or aclchk.c?)
> would be a more appropriate location for these functions, though in some of
> the above cases, it might require exposing them (I'm not sure that I see
> any harm in doing so).
I don't think has_rolinherit or has_rolcatupdate really need to move and
it seems unlikely that they'd be needed from elsewhere.. Is there a
reason you think they'd need to be exposed? I've not looked at the
patch at all though, perhaps that makes it clear.
> Perhaps refactoring to consolidate all of these in either acl.c or aclchk.c
> with the following pattern might be a step in the right direction?
>
> * has_createrole_privilege
> * has_bypassrls_privilege
These are already in the right place, right?
> * has_inherit_privilege
> * has_catupdate_privilege
These probably don't need to move as they're only used in the .c files
that they're defined in (unless there's a reason that needs to change).
> * has_replication_privilege
This is what I was on about, so I agree that it should be created. ;)
> * has_???_privilege
Right, other things might be 'has_backup_privilege', for things like
pg_start/stop_backup and friends.
> In either case, I think following a "convention" on the naming of these
> functions (unless there is semantic reason not to) would also help to
> reduce any confusion or head scratching.
Agreed.
Thanks!
Stephen
From | Date | Subject | |
---|---|---|---|
Next Message | Craig Ringer | 2014-10-02 03:25:15 | Re: Dynamic LWLock tracing via pg_stat_lwlock (proof of concept) |
Previous Message | Kevin Grittner | 2014-10-02 00:20:44 | Re: bad estimation together with large work_mem generates terrible slow hash joins |