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.