Re: improving user.c error messages - Mailing list pgsql-hackers

From Peter Eisentraut
Subject Re: improving user.c error messages
Date
Msg-id e1bbef97-ed13-f6ad-8d21-b2ebc251394b@enterprisedb.com
Whole thread Raw
In response to Re: improving user.c error messages  (Nathan Bossart <nathandbossart@gmail.com>)
Responses Re: improving user.c error messages
List pgsql-hackers
On 07.02.23 21:10, Nathan Bossart wrote:
>>     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?
> Here is a new patch set with this change and some other light editing.

I'm concerned about the loose use of "privilege" here.  A privilege is 
something I can grant.  So if someone doesn't have the "REPLICATION 
privilege", as in the above example, I would expect to be able to do 
"GRANT REPLICATION TO someuser".  Since that is not what is happening, 
we should use some other term.  The documentation around CREATE USER 
uses the terms "attribute" and "option" (and also "privilege") for these 
things.

Similarly -- this is an existing issue but we might as well look at it 
-- in something like

     must be superuser or a role with privileges of the
     pg_write_server_files role

the phrase "a role with the privileges of that other role" seems 
ambiguous.  Doesn't it really mean you must be a member of that role?

I also feel that in sentences like

     "You must have %s privilege to create roles."

a "the" is missing.




pgsql-hackers by date:

Previous
From: Michael Paquier
Date:
Subject: Re: Missing free_var() at end of accum_sum_final()?
Next
From: Amit Langote
Date:
Subject: Re: ExecRTCheckPerms() and many prunable partitions (checkAsUser)