Today, I see some error messages have been added, two of which look somewhat inconsistent.
commands/user.c @707: > errmsg("must have admin option on role \"%s\" to add members", @1971: > errmsg("grantor must have ADMIN OPTION on \"%s\"", A grep'ing told me that the latter above is the only outlier among 6 occurrences in total of "admin option/ADMIN OPTION". Don't we unify them? I slightly prefer "ADMIN OPTION" but no problem with them being in small letters. (Attached). In passing, I met the following code in the same file. > if (!have_createrole_privilege() && > !is_admin_of_role(currentUserId, roleid)) > ereport(ERROR, > > (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), > errmsg("must have admin option on role > \"%s\"", > rolename))); The message seems a bit short that it only mentions admin option while omitting CREATEROLE privilege. "must have CREATEROLE privilege or admin option on role %s" might be better. Or we could say just "insufficient privilege" or "permission denied" in the main error message then provide "CREATEROLE privilege or admin option on role %s is required" in DETAILS or HINTS message. The message was added by c33d575899 along with the have_createrole_privilege() call so it is unclear to me whether it is intentional or not. regards. -- Kyotaro Horiguchi NTT Open Source Software Center
diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c index 511ca8d8fd..4a2e5556f1 100644 --- a/src/backend/commands/user.c +++ b/src/backend/commands/user.c @@ -1968,7 +1968,7 @@ check_role_grantor(Oid currentUserId, Oid roleid, Oid grantorId, bool is_grant) select_best_admin(grantorId, roleid) != grantorId) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("grantor must have ADMIN OPTION on \"%s\"", + errmsg("grantor must have admin option on \"%s\"", GetUserNameFromId(roleid, false)))); } else