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.
