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

On 2014-12-04 17:06:02 -0500, Stephen Frost wrote:
> * Andres Freund (andres@2ndquadrant.com) wrote:
> > On 2014-12-04 15:59:17 -0500, Stephen Frost wrote:
> > > I have a hard time wrapping my head around what a *lot* of our users ask
> > > when they see a given error message, but if our error message is 'you
> > > must have a pear-shaped object to run this command' then I imagine that
> > > a new-to-PG user might think "well, I can't create pear shaped objects
> > > in PG, so I guess this just isn't supported."  That's not necessairly
> > > any fault of ours, but I do think "permission denied" reduces the chance
> > > that there's any confusion about the situation.
> > 
> > I've a hard time taking this comment seriously. If can't believe that
> > you think that comment bears relation to the error message we're
> > discussing.
> 
> It's reasonable to think that new users won't understand PG-specific
> things such as role attributes.  A new user might not recognize or
> understand what "replication role" is but they're more than likely to
> get the gist behind "permission denied."

Anyone that's using any of these facilities better know at least halfway
what a permission is.

> > > > The answer is that there really
> > > > *isn't* any additional information conveyed by your proposal rewrite;
> > > 
> > > To be sure it's clear, I *don't* agree with this.  You and I don't see
> > > any additional information in it because we're familiar with the system
> > > and know all about role attributes, the GRANT system, and everything
> > > else.  I'm not looking to change the error message because it's going to
> > > make it clearer to you or to me or to anyone else on this list though.
> > 
> > > The "different style" is what's in the error style guidelines.
> > 
> > I think you're vastly over-interpreting the guidelines because that
> > happens to suite your position.
> 
> So I shouldn't consider what the guidelines have because they agree with
> my position?

Not if there's not actually anything in there.

> Perhaps we should *change* the guidelines if they aren't what we
> should be following.

So, now you've build the full circle.

> > None of the current error message violates any of:
> > 
> > > The primary message should be short, factual, and avoid reference to
> > > implementation details such as specific function names. "Short" means
> > > "should fit on one line under normal conditions". Use a detail message
> > > if needed to keep the primary message short, or if you feel a need to
> > > mention implementation details such as the particular system call that
> > > failed. Both primary and detail messages should be factual. Use a hint
> > > message for suggestions about what to do to fix the problem, especially
> > > if the suggestion might not always be applicable.
> 
> I agree that the text doesn't say "the error message should be related
> to what the errcode used is", but it's certainly the implication of the
> examples provided and, when the text doesn't make any reference, going
> by what is in the example seems perfectly reasonable to me.

If anything it's the absolute contrary.

Messages should always state the reason why an error occurred. For
example:

> BAD:    could not open file %s
> BETTER: could not open file %s (I/O failure)

> Avoid mentioning called function names, either; instead say what the
> code was trying to do:
> BAD:    open() failed: %m
> BETTER: could not open file %s: %m

> BAD:    unknown node type
> BETTER: unrecognized node type: 42

There's simply not a a single example giving credence to your position
in the guidelines I'm looking at. That's *not* to say that your position
is wrong, but holding up the guidelines as a reason for it is absurd.

> > And I don't for a second buy your argument that the permissions involved
> > are an implemementation detail.
> 
> Would you agree with Peter that, rather than focus on if the errdetail()
> involves an implementation detail or not, we should go ahead and add the
> "You must have SELECT rights ..." to the existing cases where we just
> say "permission denied"?

I think adding helpful information isn't a bad idea. It's not always
obvious which permission is required to do something.

> > If you say that you like the new message better because it's more
> > consistent or more beautiful I can buy that. But don't try to find
> > justifications in guidelines when they don't contain that.
> 
> I've said that I like the new messaging because it's more consistent
> time and time again.  You suggest that I argue on that merit alone
> rather than pointing out where we've (intentionally or not) used
> examples which agree with me?  That strikes me as rather silly.

Since there's nothing in the guidelines...

> I didn't bring this up and I'm getting a bit put-out by the constant
> implications that it's just all me and my crazy ideas for consistency.

I don't mind much being outvoted or convinced by better reasoning. But
you've shown so far no recognition of the fact that the new message is
much longer without much of additional detail. And you've largely
argumented using arguments that I found absolutely unconvincing.

I'm now giving in by plonking this thread.

Greetings,

Andres Freund

-- Andres Freund                       http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services