|From:||Peter Eisentraut <peter_e(at)gmx(dot)net>|
|To:||Stephen Frost <sfrost(at)snowman(dot)net>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>|
|Cc:||"Brightwell, Adam" <adam(dot)brightwell(at)crunchydatasolutions(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>|
|Subject:||Re: superuser() shortcuts|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
On 10/27/14 11:40 AM, Stephen Frost wrote:
> * Alvaro Herrera (alvherre(at)2ndquadrant(dot)com) wrote:
>>> As I started looking at this, there are multiple other places where
>>> these types of error messages occur (opclasscmds.c, user.c,
>>> postinit.c, miscinit.c are just a few), not just around the changes in
>>> this patch. If we change them in one place, wouldn't it be best to
>>> change them in the rest? If that is the case, I'm afraid that might
>>> distract from the purpose of this patch. Perhaps, if we want to
>>> change them, then that should be submitted as a separate patch?
>> Yeah. I'm just saying that maybe this patch should adopt whatever
>> wording we agree to, not that we need to change other places. On the
>> other hand, since so many other places have adopted the different
>> wording, maybe there's a reason for it and if so, does anybody know what
>> it is. But I have to say that it does look inconsistent to me.
> Updated patch attached. Comments welcome.
The changes in
are OK, because the superuser() calls are redundant, and cleaning this
up is clearly in line with planned future work.
I suggest moving the rest of the changes into separate patches.
The error message wording changes might well be worth considering, but
there are tons more "must be $someone to do $something" error messages,
and changing two of them isn't making anything more or less consistent.
The ha*_something_privilege() changes are also not very consistent.
We already have have_createrole_privilege(), which does include a
superuser check, and you add has_replication_privilege() with a
superuser check, but has_catupdate_privilege() and
has_inherit_privilege() don't include a superuser check. That's clearly
Btw., why rename have_createrole_privilege()?
Also, your patch has spaces between tabs. Check for whitespace errors
|Next Message||Andrew Dunstan||2014-11-04 17:57:27||get_cast_func syscache utility function|
|Previous Message||Tom Lane||2014-11-04 16:58:42||Re: alter user set local_preload_libraries.|