Re: alter user/role CURRENT_USER
| От | Brightwell, Adam |
|---|---|
| Тема | Re: alter user/role CURRENT_USER |
| Дата | |
| Msg-id | CAKRt6CTkNWKAmhWzAvqawdQpHA3x9SrEaHJ3ofuyD6YcYaALyw@mail.gmail.com обсуждение исходный текст |
| Ответ на | Re: alter user/role CURRENT_USER (Rushabh Lathia <rushabh.lathia@gmail.com>) |
| Ответы |
Re: alter user/role CURRENT_USER
|
| Список | pgsql-hackers |
Kyotaro,
Food for thought. Couldn't you reduce the following block:
+ if (strcmp(stmt->role, "current_user") == 0)
+ {
+ roleid = GetUserId();
+ tuple = SearchSysCache1(AUTHOID, ObjectIdGetDatum(roleid));
+ if (!HeapTupleIsValid(tuple))
+ ereport(ERROR,
+ (errcode(ERRCODE_UNDEFINED_OBJECT),
+ errmsg("roleid %d does not exist", roleid)));
+ }
+ else
+ {
+ tuple = SearchSysCache1(AUTHNAME, PointerGetDatum(stmt->role));
+ if (!HeapTupleIsValid(tuple))
+ ereport(ERROR,
+ (errcode(ERRCODE_UNDEFINED_OBJECT),
+ errmsg("role \"%s\" does not exist", stmt->role)));
To:
if (strcmp(stmt->role, "current_user") == 0)
roleid = GetUserId();
else
roleid = get_role_oid(stmt->role, false);
tuple = SearchSysCache1(AUTHOID, ObjectIdGetDatum(roleid));
if (!HeapTupleIsValid(tuple))
ereport(ERROR,
(errcode(ERRCODE_UNDEFINED_OBJECT),
errmsg("roleid %d does not exist", roleid)));
I think this makes it a bit cleaner. It also reuses existing code as 'get_role_oid()' already does a valid role name check and will raise the proper error.
-Adam
On Mon, Oct 20, 2014 at 3:40 AM, Rushabh Lathia <rushabh.lathia@gmail.com> wrote:
I gone through patch and here is the review for this patch:
.) patch go applied on master branch with patch -p1 command
(git apply failed)
.) regression make check run fine
.) testcase coverage is missing in the patch
.) Over all coding/patch looks good.
Few comments:
1) Any particular reason for not adding SESSION_USER/USER also made usable
for this command ?
2) I think RoleId_or_curruser can be used for following role:
/* ALTER TABLE <name> OWNER TO RoleId */
| OWNER TO RoleId
3) In the documentation patch, added for CURRENT_USER but CURRENT_ROLE is
missing.On Fri, Oct 10, 2014 at 1:57 PM, Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> wrote:--Hello, on the way considering alter role set .., I found that
ALTER ROLE/USER cannot take CURRENT_USER as the role name.
In addition to that, documents of ALTER ROLE / USER are
inconsistent with each other in spite of ALTER USER is said to be
an alias for ALTER ROLE. Plus, ALTER USER cannot take ALL as user
name although ALTER ROLE can.
This patch does following things,
- ALTER USER/ROLE now takes CURRENT_USER as user name.
- Rewrite sysnopsis of the documents for ALTER USER and ALTER
ROLE so as to they have exactly same syntax.
- Modify syntax of ALTER USER so as to be an alias of ALTER ROLE.
- Added CURRENT_USER/CURRENT_ROLE syntax to both.
- Added ALL syntax as user name to ALTER USER.
- Added IN DATABASE syntax to ALTER USER.
x Integrating ALTER ROLE/USER syntax could not be done because of
shift/reduce error of bison.
x This patch contains no additional regressions. Is it needed?
SESSION_USER/USER also can be made usable for this command, but
this patch doesn't so (yet).
regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
--
Rushabh Lathia
Adam Brightwell - adam.brightwell@crunchydatasolutions.com
Database Engineer - www.crunchydatasolutions.com
В списке pgsql-hackers по дате отправления: