Re: superuser() shortcuts

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
Date: 2014-11-04 17:24:15
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

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
a mess.

Btw., why rename have_createrole_privilege()?

Also, your patch has spaces between tabs. Check for whitespace errors
with git.

In response to


Browse pgsql-hackers by date

  From Date Subject
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.