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-01-27 09:16:17
Message-ID: 20150127.181617.181412893.horiguchi.kyotaro@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hello, thank you for the comment.

> Looking at this a bit, I'm not sure completely replacing RoleId with a
> node is the best idea; some of the users of that production in the
> grammar are okay with accepting only normal strings as names, and don't
> need all the CURRENT_USER etc stuff.

You're right. the productions don't need RoleSpec already uses
char* for the role member in its *Stmt structs. I might have done
a bit too much.

Adding new nonterminal RoleId and using it makes a bit duplicate
of check code for "public"/"none" and others but removes other
redundant check for "!= CSTRING" from some production, CREATE
ROLE/USER/GROUP, ALTER ROLE/USER/GROUP RENAME.

RoleId in the patch still has rule components for CURRENT_USER,
SESSION_USER, and CURRENT_ROLE. Without them, the parser prints
an error ununderstandable to users.

| =# alter role current_user rename to "PuBlic";
| ERROR: syntax error at or near "rename"
| LINE 1: alter role current_user rename to "PuBlic";
| ^

With the components, the error message becomes like this.

| =# alter role current_role rename to "PuBlic";
| ERROR: reserved word cannot be used
| LINE 1: alter role current_role rename to "PuBlic";
| ^

> Maybe we need a new production,
> say RoleSpec, and we modify the few productions that need the extra
> flexibility? For instance we could have ALTER USER RoleSpec instead of
> ALTER USER RoleId. But we would keep CREATE USER RoleId, because it
> doesn't make any sense to accept CREATE USER CURRENT_USER.
> I think that would lead to a less invasive patch also.
> Am I making sense?

I think I did what you expected. It was good as the code got
simpler but the invasive-ness dosn't seem to be reduced..

What do you think about this?

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

Attachment Content-Type Size
0001-ALTER-USER-CURRENT_USER-v4.patch text/x-patch 51.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kyotaro HORIGUCHI 2015-01-27 09:24:03 Re: [POC] FETCH limited by bytes.
Previous Message Abhijit Menon-Sen 2015-01-27 09:08:30 Re: pgaudit - an auditing extension for PostgreSQL