Hello,
At Thu, 30 Apr 2015 17:12:25 -0300, Alvaro Herrera alvhe...@2ndquadrant.com
wrote in 20150430201225.gv4...@alvh.no-ip.org
Kyotaro HORIGUCHI wrote:
Thank you for completing this and very sorry not to respond these
days.
I understood that it is committed after I noticed that
Kyotaro HORIGUCHI wrote:
Thank you for completing this and very sorry not to respond these
days.
I understood that it is committed after I noticed that rebasing
my code failed..
You'd do well to check your email, I guess :-)
Although after committed, I found some issues as I looked on
Thank you for completing this and very sorry not to respond these
days.
I understood that it is committed after I noticed that rebasing
my code failed..
Although after committed, I found some issues as I looked on
it. Please forgive me to comment it now after all this time.
Several changes in
Alvaro Herrera wrote:
With this patch applied, doing
\h ALTER ROLE
in psql looks quite odd: note how wide it has become. Maybe we should
be doing this differently? (Hmm, why don't we accept ALL in the first SET
line? Maybe that's just a mistake and the four lines should be all
identical
There is something odd here going on:
alvherre=# alter group test add user current_user;
ERROR: role test is a member of role (null)
Surely (null) is not the right name to be reporting there ...
Attached is a rebased patch, which also has some incomplete doc changes.
With this patch applied,
Kyotaro HORIGUCHI wrote:
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
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
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. Maybe we need a new production,
say RoleSpec, and
Thank you.
A new patch has been sent here but no review occurred, hence moving
this item to CF 2014-12.
regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
On Fri, Nov 14, 2014 at 5:39 PM, Kyotaro HORIGUCHI
horiguchi.kyot...@lab.ntt.co.jp wrote:
Hi, this is revised version.
Kyotaro HORIGUCHI wrote:
- Storage for new information
The new struct NameId stores an identifier which telling what it
logically is using the new enum NameIdTypes.
Hi, this is revised version.
Kyotaro HORIGUCHI wrote:
- Storage for new information
The new struct NameId stores an identifier which telling what it
logically is using the new enum NameIdTypes.
I think NameId is a bad name for this. My point is that NameId, as it
stands, might be
Kyotaro HORIGUCHI wrote:
- Storage for new information
The new struct NameId stores an identifier which telling what it
logically is using the new enum NameIdTypes.
I think NameId is a bad name for this. My point is that NameId, as it
stands, might be a name for anything, not just a role;
Hello, This is the new version. (WIP v2)
The first attachment is the patch and the second is test sql
script.
- Behavior changing
Almost all syntax taking role accepts CURRENT_USER and
SESSION_USER and they are distinguished from current_user and
session_user. The exceptions are follows.
-
All,
I agree that we should probably seperate the concerns here. Personally,
I like the idea of being able to say CURRENT_USER in utility commands
to refer to the current user where a role would normally be expected, as
I could see it simplifying things for some applications, but that's a
Hello,
At Tue, 28 Oct 2014 09:05:20 -0400, Stephen Frost sfr...@snowman.net wrote in
20141028130520.gl28...@tamriel.snowman.net
As well, the
originally proposed RoleId_or_curruser suffers from the same issue. I'm
going to go out on a limb here, but is it not possible to consider
Kyotaro,
Zero-length identifiers are rejected in scanner so RoleId cannot
be a valid pointer to a zero-length string. (NULL is used as
PUBLIC in auth_ident)
| postgres=# create role ;
| ERROR: zero-length delimited identifier at or near
| postgres=# create role U\00;
| ERROR: invalid
* Adam Brightwell (adam.brightw...@crunchydatasolutions.com) wrote:
| RoleId_or_curruser: RoleId{ $$ = $1; }
| | CURRENT_USER { $$ = \x00\x01;};
[...]
This is ugly but needs no additional struct member or special
logics. (Macros could make
Stephen Frost sfr...@snowman.net writes:
The other idea which comes to mind is- could we try to actually resolve
what the 'right' answer is here, instead of setting a special value and
then having to detect and fix it later?
No, absolutely not. Read the NOTES at the head of gram.y. Or if you
Adam Brightwell adam.brightw...@crunchydatasolutions.com writes:
FWIW, I found the following in the archives:
http://www.postgresql.org/message-id/15516.1038718...@sss.pgh.pa.us
Now this is from 2002 and it appears it wasn't necessary to change at the
time, but I haven't yet found anything
Hello, thank you all for many comments.
At the first, I removed changes for role-vs-user consistency and
remove all added role named other than current_user.
The followings are one-by-one answer for the comments so far,
please let me know if I missed anything.
- The necessity of the new
+RoleId:CURRENT_USER{ $$ =
current_user;}
+ | USER { $$ = current_user;}
+ | CURRENT_ROLE { $$ = current_user;}
+ | SESSION_USER { $$ =
* Adam Brightwell (adam.brightw...@crunchydatasolutions.com) wrote:
+RoleId:CURRENT_USER{ $$ =
current_user;}
+ | USER { $$ = current_user;}
+ | CURRENT_ROLE { $$ =
Stephen Frost sfr...@snowman.net wrote:
You can still double-quote reserved words and use them in general. What
we're talking about here are cases where a word can't be used even if
it's double-quoted, and we try really hard to keep those cases at an
absolute minimum. We should also really
On Tue, Oct 28, 2014 at 2:40 AM, Adam Brightwell
adam.brightw...@crunchydatasolutions.com wrote:
Taking a step back, I'm still not sure I understand the need for this
feature or the use case. It seems to have started as a potential fix to an
inconsistency between ALTER USER and ALTER ROLE
* Kevin Grittner (kgri...@ymail.com) wrote:
Stephen Frost sfr...@snowman.net wrote:
You can still double-quote reserved words and use them in general. What
we're talking about here are cases where a word can't be used even if
it's double-quoted, and we try really hard to keep those cases
Marti Raudsepp wrote:
On Fri, Oct 24, 2014 at 11:29 AM, Kyotaro HORIGUCHI
horiguchi.kyot...@lab.ntt.co.jp wrote:
- 0001-ALTER-ROLE-CURRENT_USER_v2.patch - the patch.
+RoleId:CURRENT_USER{ $$ =
current_user;}
+ | USER
All,
I just ran through the patch giving it a good once over, some items to
address/consider/discuss:
* Trailing whitespace.
* Why are you making changes in foreigncmds.c? These seem like unnecessary
changes. I see that you are trying to consolidate but this refactor seems
potentially out of
On Fri, Oct 24, 2014 at 11:29 AM, Kyotaro HORIGUCHI
horiguchi.kyot...@lab.ntt.co.jp wrote:
- 0001-ALTER-ROLE-CURRENT_USER_v2.patch - the patch.
+RoleId:CURRENT_USER{ $$ = current_user;}
+ | USER { $$ =
Marti Raudsepp wrote
On Fri, Oct 24, 2014 at 11:29 AM, Kyotaro HORIGUCHI
lt;
horiguchi.kyotaro@.co
gt; wrote:
But should ALTER USER ALL and ALTER ROLE ALL really do the same thing?
A user is a role with the LOGIN option. Every user is a role, but not
every role is a user. I suspect that
Thanks Kyotaro,
I just did quickly looked at the patch and it does cover more syntax then
earlier patch. But still if doesn't not cover the all the part of syntax
where
we can use CURRENT_USER/CURRENT_ROLE/USER/SESSION_USER. For example:
-- Not working
alter default privileges for role
Hi, here is the revised patch.
Attached files are the followings
- 0001-ALTER-ROLE-CURRENT_USER_v2.patch - the patch.
- testset.tar.bz2 - test set. Run by typing 'make check' as a
superuser of the running postgreSQL server. It creates testdb
and some roles.
Documents are not edited
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
Hello,
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,
+
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
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),
+
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
36 matches
Mail list logo
