Re: improving user.c error messages

On 16.03.23 16:48, Nathan Bossart wrote:
>> I think the following change in DropRole() is incorrect:
>>
>>          if (!is_admin_of_role(GetUserId(), roleid))
>>              ereport(ERROR,
>>                      (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
>> -                    errmsg("must have admin option on role \"%s\"",
>> -                           role)));
>> +                    errmsg("permission denied to drop role"),
>> +                    errdetail("Only roles with the %s attribute and the %s
>> option on role \"%s\" may drop this role.",
>> +                              "CREATEROLE", "ADMIN",
>> NameStr(roleform->rolname))));
>>
>> The message does not reflect what check is actually performed.  (Perhaps
>> this was confused with a similar but not exactly the same check in
>> RenameRole().)
> Hm.  Is your point that we should only mention the admin option here?  I
> mentioned both createrole and admin option in this message (and the
> createrole check above this point) in an attempt to avoid giving partial
> information.

AFAICT, the mention of CREATEROLE is incorrect, because the code doesn't 
actually check for the CREATEROLE attribute.