Re: Letter case of "admin option"

From: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>
To: robertmhaas(at)gmail(dot)com
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org, alvherre(at)alvh(dot)no-ip(dot)org
Subject: Re: Letter case of "admin option"
Date: 2022-08-25 05:36:39
Message-ID: 20220825.143639.1085937869614690515.horikyota.ntt@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Thanks for the comment.

At Tue, 23 Aug 2022 09:58:47 -0400, Robert Haas <robertmhaas(at)gmail(dot)com> wrote in
> On Mon, Aug 22, 2022 at 9:29 PM Kyotaro Horiguchi
> <horikyota(dot)ntt(at)gmail(dot)com> wrote:
> In the case of GRANT, that's more ambiguous, because the word OPTION
> actually appears in the syntax. But isn't that sort of accidental?

Yeah I think so. My intension is to let the translators do their work
more mechanically. A capital-letter word is automatically recognized
as a keyword then can be copied as-is.

I would translate "ADMIN OPTION" to "ADMIN OPTION" in Japanese but
"admin option" is translated to "管理者オプション" which is a bit hard
for the readers to come up with the connection to "ADMIN OPTION" (or
ADMIN <roles>). I guess this is somewhat simliar to use "You need to
give capability to administrate the role" to suggest users to add WITH
ADMIN OPTION to the role.

Maybe Álvaro has a similar difficulty on it.

> It's quite possible to give someone the right to administer a role
> without ever mentioning the OPTION keyword:

Mmm.. Fair point.

> In short, I'm wondering whether we should regard ADMIN as the name of
> the option, but OPTION as part of the GRANT syntax, and hence
> capitalize it "ADMIN option". However, if the non-English speakers on
> this list have a strong preference for something else I'm certainly
> not going to fight about it.

"ADMIN option" which is translated into "ADMINオプション" is fine by
me. I hope Álvaro thinks the same way.

What do you think about the 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.
>
> Yeah, I wasn't sure what to do about this. We do not mention superuser
> privileges in every message where they theoretically apply, because it
> would make a lot of messages longer for not much benefit. CREATEROLE
> is a similar case and I think, but am not sure, that we treat it
> similarly. So in my mind it is a judgement call what to do here, and
> if other people think that what I picked wasn't best, we can change
> it.
>
> For what it's worth, I'm hoping to eventually remove the CREATEROLE
> exception here. The superuser exception will remain, of course.

If it were simply "permission denied", I don't think about the details
then seek for the way to allow that. But I don't mean to fight this
for now.

For the record, I would prefer the follwoing message for this sort of
failure.

ERROR: permission denied
DETAILS: CREATEROLE or ADMIN option is required for the role.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

Attachment Content-Type Size
Fix-letter-cases-of-ADMIN-option.patch text/x-patch 24.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message John Naylor 2022-08-25 06:35:45 Re: [PATCH] Optimize json_lex_string by batching character copying
Previous Message Bharath Rupireddy 2022-08-25 05:18:08 Re: Avoid erroring out when unable to remove or parse logical rewrite files to save checkpoint work