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 |
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 |