Re: superuser() shortcuts - Mailing list pgsql-hackers

On Tue, Dec 2, 2014 at 4:11 PM, Stephen Frost <sfrost@snowman.net> wrote:
>> It's pretty clear that the current message is complaining about a
>> permissions problem, so the fact that it doesn't specifically include
>> the words "permission" and "denied" doesn't bother me.  Let me ask the
>> question again: what information do you think is conveyed by your
>> proposed rewrite that is not conveyed by the current message?
>
> I do like that it's explicitly stating that the problem is with
> permissions and not because of some lack-of-capability in the software
> and I like the consistency of messaging (to the point where I was
> suggesting somewhere along the way that we update the error messaging
> documentation to be explicit that the errmsg and the errcode should make
> sense and match up- NOT that we should use some simple mapping from one
> to the other as we'd then be reducing the information provided, but I've
> seen a number of cases where people have the SQL error-code also
> returned in their psql session; I've never been a fan of that as I much
> prefer words over error-codes, but I wonder if they've done that
> because, in some cases, it *isn't* clear what the errcode is from the
> errmsg..).

I think you're dodging my question.  The answer is that there really
*isn't* any additional information conveyed by your proposal rewrite;
the issue is simply that you prefer your version to the current
version.  Which is fair enough, but my preference is different (as is
that of Andres), which is also fair.

> It provides a consistency of messaging that I feel is an improvement.
> That they aren't related in a number of existing cases strikes me as an
> opportunity to improve rather than cases where we really "know better".

Let's consider this case, which perhaps you haven't examined:
       if (es.buffers && !es.analyze)               ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),                               errmsg("EXPLAIN option BUFFERS
 
requires ANALYZE")));

This is basically analogous to the case your complaining about.  The
error message is short and to the point and does not include the words
that appear in the error code.  Now, you could propose to change this,
but any rewording of it is going to be longer than what we have now
for no real improvement in clarity, and it's going to force
translators to do the extra work of retranslating it for no particular
gain.

The burden for changing existing messages is not high, but your desire
for a different style is not sufficient to go change half the error
messages in the backend, or even 20% of them, and I think that 20% is
a conservative estimate of how many we'd have to change to actually
achieve what you're talking about here.

> Perhaps in some of those cases the problem is that there isn't a good
> errcode, and maybe we should actually add an error code instead of
> changing the message.

The error codes are mostly taken from the SQL standard.  It is of
course possible that we should add our own in some cases, but it will
have the disadvantage of reducing the ability of applications written
for other systems to understand our error codes.

> If we want to say that role-attribute-related error messages should be
> "You need X to do Y", then I'll go make those changes, removing the
> 'permission denied' where those are used and be happier that we're at
> least consistent, but it'll be annoying if we ever make those
> role-attributes be GRANT'able rights (GRANT .. ON CLUSTER?)  as those
> errmsg's will then change from "you need X" to "permission denied to do
> Y".  Having the errdetail change bothers me a lot less.

You can't really put "you need X to do Y" in the error message, I
think.  That style is appropriate for an error detail, but not an
error message.

I just don't understand why you want to pointlessly tinker with this.
If this is integrally related to the introduction of finer-grained
superuser permissions, then perhaps it is worth doing, but if that's
the motivation, you haven't made an argument that I can understand as
to why it's necessary.  What I think is that the current messages - or
at least the ones you are complaining about - are fine, and we can
change them on a case-by-case basis as the evolution of the system
demands, but that we shouldn't go whack them around just because you
would have chosen differently from among sane alternatives than what
the original author chose to do.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company