improving user.c error messages

From: Nathan Bossart <nathandbossart(at)gmail(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: improving user.c error messages
Date: 2023-01-26 00:22:51
Message-ID: 20230126002251.GA1506128@nathanxps13
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

moving this discussion to a new thread...

On Thu, Jan 19, 2023 at 10:20:33AM -0500, Robert Haas wrote:
> On Wed, Jan 18, 2023 at 6:17 PM Nathan Bossart <nathandbossart(at)gmail(dot)com> wrote:
>> However, as the attribute
>> system becomes more sophisticated, I think we ought to improve the error
>> messages in user.c. IMHO messages like "permission denied" could be
>> greatly improved with some added context.
>> This probably shouldn't block your patch, but I think it's worth doing in
>> v16 since there are other changes in this area. I'm happy to help.
> That would be great. I agree that it's good to try to improve the
> error messages. It hasn't been entirely clear to me how to do that.
> For instance, I don't think we want to say something like:
> ERROR: must have CREATEROLE privilege and ADMIN OPTION on the target
> role, or in lieu of both of those to be superuser, to set the
> CONNECTION LIMIT for another role
> ERROR: must have CREATEROLE privilege and ADMIN OPTION on the target
> role, plus also CREATEDB, or in lieu of all that to be superuser, to
> remove the CREATEDB property from another role
> Such messages are long and we'd end up with a lot of variants. It's
> possible that the messages could be multi-tier. For instance, if we
> determine that you're trying to manage users and you don't have
> permission to manage ANY user, we could say:
> ERROR: permission to manage roles denied
> DETAIL: You must have the CREATEROLE privilege or be a superuser to
> manage roles.
> If you could potentially manage some user, but not the one you're
> trying to manage, we could say:
> ERROR: permission to manage role "%s" denied
> DETAIL: You need ADMIN OPTION on the target role to manage it.
> If you have permission to manage the target role but not in the
> requested manner, we could then say something like:
> ERROR: permission to manage CREATEDB for role "%s" denied
> DETAIL: You need CREATEDB to manage it.
> This is just one idea, and maybe not the best one. I'm just trying to
> say that I think this is basically an organizational problem. We need
> a plan for how we're going to report errors that is not too
> complicated to implement with reasonable effort, and that will produce
> messages that users will understand. I'd be delighted if you wanted to
> provide either ideas or patches...

Here is an early draft of some modest improvements to the user.c error
messages. I basically just tried to standardize the style of and add
context to the existing error messages. I used errhint() for this extra
context, but errdetail() would work, too. This isn't perfect. You might
still have to go through a couple rounds of errors before your role has all
the privileges it needs for a command, but this seems to improve matters a

I think there is still a lot of room for improvement, but I wanted to at
least get the discussion started before I went too far.

Nathan Bossart
Amazon Web Services:

Attachment Content-Type Size
enhance_error_messages.patch text/x-diff 28.3 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Thomas Munro 2023-01-26 00:23:41 Re: suppressing useless wakeups in logical/worker.c
Previous Message Karl O. Pinc 2023-01-26 00:03:25 Re: drop postmaster symlink