Re: alter user/role CURRENT_USER

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(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)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(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
>

--
Rushabh Lathia