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: Robert Haas <robertmhaas(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, tgl(at)sss(dot)pgh(dot)pa(dot)us
Subject: Re: improving user.c error messages
Date: 2023-01-26 19:13:58
Message-ID: 20230126191358.GA1660026@nathanxps13
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Thanks for taking a look.

On Thu, Jan 26, 2023 at 10:07:39AM +0100, Alvaro Herrera wrote:
> Please use
> errdetail("You must have %s privilege to create roles with %s.",
> in this kind of message where multiple copies appear that only differ in
> the keyword to use, to avoid creating four copies of essentially the
> same string.
> This applies in several places.

I did this in v2.

>> - errmsg("must have createdb privilege to change createdb attribute")));
>> + errmsg("permission denied to alter role"),
>> + errhint("You must have CREATEDB privilege to alter roles with CREATEDB.")));
> I think this one is a bit ambiguous; does "with" mean that roles that
> have that priv cannot be changed, or does it mean that you cannot meddle
> with that bit in particular? I think it'd be better to say
> "You must have %s privilege to change the %s attribute."
> or something like that.

Yeah, it's probably better to say "to alter roles with %s" to refer to
roles that presently have the attribute and "to change the %s attribute"
when referring to privileges for the attribute. I did this in v2, too.

I've also switched from errhint() to errdetail() as suggested by Tom.

Nathan Bossart
Amazon Web Services:

Attachment Content-Type Size
v2-0001-Improve-user.c-error-messages.patch text/x-diff 29.8 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Matthias van de Meent 2023-01-26 19:26:00 Re: New strategies for freezing, advancing relfrozenxid early
Previous Message Tom Lane 2023-01-26 18:54:08 Re: suppressing useless wakeups in logical/worker.c