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

Reply via email to