Thank you for reviewing, 2014 13:10:57 +0530, Rushabh Lathia <rushabh.lat...@gmail.com> wrote in <cagpqqf0kdfajizx0vca_-wazwu+xj5mdnl-hgg1sez9aw3c...@mail.gmail.com> > 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 ?
No particular reson. This patch was the first step and if this is the adequate way and adding them is desirable, I will add them. > 2) I think RoleId_or_curruser can be used for following role: > > /* ALTER TABLE <name> OWNER TO RoleId */ > | OWNER TO RoleId Good catch. I'll try it. > 3) In the documentation patch, added for CURRENT_USER but CURRENT_ROLE is > missing. Mmm.. I didn't added CURRENT_ROLE in the previous patch. I suppose CURRENT_ROLE is an implicit alias of CURRENT_USER because it is not explained in the page below in spite of existing in syntax. http://www.postgresql.org/docs/9.4/static/functions-info.html and it is currently usable only in expressions, so the following SQL failed and all syntax using auth_ident will fail. postgres=# CREATE USER MAPPING FOR CURRENT_ROLE SERVER s1; ERROR: syntax error at or near "current_role" share/doc/html/sql-createusermapping.html | Synopsis | | CREATE USER MAPPING FOR { user_name | USER | CURRENT_USER | PUBLIC } I don't know why the 'USER' is added here, but anyway I can add 'CURRENT_ROLE' there in gram.y but it seems not necessary to add it to document. Ok, I'll modify this patch so that, - Make CURRENT_USER/ROLE usable in TABLE OWNER TO. and since adding CURRENT_ROLE is the another thing, I'll do the following things as additional patch. - Add USER, CURRENT_ROLE and SESSION_* for the place where CURRENT_USER is usable now. auth_ident and RoleId_or_curruser. 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
