Re: improving user.c error messages

From: Nathan Bossart <nathandbossart(at)gmail(dot)com>
To: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Robert Haas <robertmhaas(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: improving user.c error messages
Date: 2023-01-27 23:15:07
Message-ID: 20230127231507.GA2221340@nathanxps13
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Jan 27, 2023 at 07:31:19PM +0100, Alvaro Herrera wrote:
> On 2023-Jan-26, Nathan Bossart wrote:
>> ereport(ERROR,
>> (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
>> - errmsg("permission denied: bootstrap user must be superuser")));
>> + errmsg("permission denied to alter role"),
>> + errdetail("The bootstrap user must be superuser.")));
>
> I think this one isn't using the right errcode; this is not a case of
> insufficient privileges. There's no priv you can acquire that lets you
> do it. So I'd change it to unsupported operation.

І fixed this in v4. I've also attached a second patch in which I've
adjusted the messages that Peter mentioned upthread.

One thing that feels a bit odd is how some of the DETAILs mention the
operation being attempted while others do not. For example, we have

ERROR: permission denied to drop role
DETAIL: You must have SUPERUSER privilege to drop roles with SUPERUSER.

In this case, the DETAIL explains the action that is prohibited. In other
cases, we have something like

ERROR: permission denied to alter role
DETAIL: You must have CREATEROLE privilege and ADMIN OPTION on role "myrole".

which does not. I think this is okay because adding "to alter the role" to
the end of the DETAIL seems kind of awkward. But in other cases, such as

ERROR: permission denied to use replication slots
DETAIL: You must have REPLICATION privilege.

adding the operation to the end seems less awkward (i.e., "You must have
REPLICATION privilege to use replication slots."). I don't think there's
any information lost by omitting the action in the DETAIL, so perhaps this
is just a stylistic choice. I think I'm inclined to add the action to the
DETAIL whenever it doesn't make the message lengthy and awkward, and leave
it out otherwise. Thoughts?

--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

Attachment Content-Type Size
v4-0001-Improve-user.c-error-messages.patch text/x-diff 29.8 KB
v4-0002-Improve-more-insufficient-privileges-error-messag.patch text/x-diff 19.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2023-01-27 23:15:50 Re: heapgettup() with NoMovementScanDirection unused in core?
Previous Message Andres Freund 2023-01-27 23:08:50 Re: Optimizing PostgreSQL with LLVM's PGO+LTO