Re: alter user/role CURRENT_USER

From: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
To: alvherre(at)2ndquadrant(dot)com
Cc: tgl(at)sss(dot)pgh(dot)pa(dot)us, adam(dot)brightwell(at)crunchydatasolutions(dot)com, sfrost(at)snowman(dot)net, marti(at)juffo(dot)org, rushabh(dot)lathia(at)gmail(dot)com, pgsql-hackers(at)postgresql(dot)org
Subject: Re: alter user/role CURRENT_USER
Date: 2015-03-12 12:38:12
Message-ID: 20150312.213812.115476889.horiguchi.kyotaro@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Thank you for completing this and very sorry not to respond these
days.

I understood that it is committed after I noticed that rebasing
my code failed..

Although after committed, I found some issues as I looked on
it. Please forgive me to comment it now after all this time.

Several changes in docs according to the changed syntax and one
change in code itself to allow CURRENT_USER in GRANT <roleid> TO
<roleid> syntax.

At Mon, 9 Mar 2015 15:50:32 -0300, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> wrote in <20150309185032(dot)GQ3291(at)alvh(dot)no-ip(dot)org>
> Alvaro Herrera wrote:
>
> > With this patch applied, doing
> > \h ALTER ROLE
> > in psql looks quite odd: note how wide it has become. Maybe we should
> > be doing this differently? (Hmm, why don't we accept ALL in the first SET
> > line? Maybe that's just a mistake and the four lines should be all
> > identical in the first half ...)
>
> I have fixed the remaining issues, completed the doc changes, and
> pushed. Given the lack of feedback I had to follow my gut on the best
> way to change the docs. I added the regression test you submitted with
> some additional changes, mainly to make sure they don't fail immediately
> when other databases exist; maybe some more concurrency or platform
> issues will show up there, but let's see what the buildfarm says.
>
> Thanks Horiguchi-san for the patch and everyone for the reviews. (It's
> probably worthwhile giving things an extra look.)

====
| =# alter role current_user rename to "PubLic";
| ERROR: CURRENT_USER cannot be used as a role name
| LINE 1: alter role current_user rename to "PubLic";
| ^

The error message sounds somewhat different from the intention. I
think the following message would be clearer.

| ERROR: CURRENT_USER cannot be used as a role name here

====
The document sql-altergroup.html says

| ALTER GROUP role_specification ADD USER user_name [, ... ]

But current_user is also usable in user_name list. So the doc
should be as following, but it would not be necessary to be fixed
because it is an obsolete commnand..

| ALTER GROUP role_specification ADD USER role_specification [, ... ]

"ALTER GROUP role_spec ADD/DROP USER role_spec" is naturally
denied so I think no additional description is needed.

====
sql-alterpolicy.html

"ALTER POLICY name ON table_name TO" also accepts current_user
and so as the role to which the policy applies.

# As a different topic, the syntax "ALTER POLICY <pname> ON
# <tname> TO <user>" looks a bit wired, it might be better be to
# be "ON <tname> APPLY TO <user>" but I shouldn't try to fix it
# since it is a long standing syntax..

====
sql-createtablespace.html

"OWNER username" should be "OWNER user_name | (CURRENT|SESSION)_USER"

====
sql-drop-owned.html, sql-reassign-owned.html

"name" should be " {name | (CURRENT|SESSION)_USER}"

For REASSIGN OWNED, TO clause also needs the same fix.

======
sql-grant.html, sql-revoke.html,

"GRANT <roles> TO <roles>" and "REVOKE <roles> FROM <roles>" are
the modern equivalents of the deprecated syntaxes "ALTER <roles>
ADD USER <roles>" and "ALTER <roles> DROP USER <roles>"
respectively. But the current parser infrastructure doesn't allow
coexistence of the two following syntaxes but I couldn't find the
way to their coexistence.

# more precisely, I guess the GRANT followed by both
# privelege_list and role_list will steps out of the realm of
# LALR(1), which I don't know well of..

"GRANT <privs> ON ..."
"GRANT <roles> TO ..."

After some struggle, I decided to add special rules
(CURRENT|SESSION)_USER to the non-terminal "privilege" and make a
room to store RoleSpec in AccessPriv. This is quite ugly but
there seems no way other than that to accomplish it.. (AccessPriv
already conveys other than the information different from what
its name represents:p)

After this fix, the commands like following are processed
properly. public and none are simply handled as nonexistent
names.

GRANT test1 TO CURRENT_USER;

GRANT <priv> ON <table> TO <role> properly rejects CURRENT_USER
as <priv>.

But the change in gram.y cuases changes in preproc.y as following,

> privilege:
> SELECT opt_column_list
> {
...
> | ColId opt_column_list
> {
> $$ = cat_str(2,$1,$2);
> }
+ | CURRENT_USER
+ {
+ $$ = mm_strdup("current_user");
+ }
+ | SESSION_USER
+ {
+ $$ = mm_strdup("session_user");
+ }
> ;

I don't understand what this change causes...

=====

I haven't looked on the docs for syntaxes related to
AlterOwnerStatement. But perhaps they don't be wrong.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

Attachment Content-Type Size
0001-Some-additional-changes-for-ALTER-ROLE-USER-CURRENT_.patch text/x-patch 17.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tomas Vondra 2015-03-12 13:07:26 Re: logical column ordering
Previous Message Sawada Masahiko 2015-03-12 12:15:14 Re: Proposal : REINDEX xxx VERBOSE