Re: alter user/role CURRENT_USER

Hello,

At Tue, 28 Oct 2014 09:05:20 -0400, Stephen Frost <sfrost(at)snowman(dot)net> wrote in <20141028130520(dot)GL28859(at)tamriel(dot)snowman(dot)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
> > "current_user", etc. reserved in the same sense that we do with PUBLIC and
> > NONE and disallow users/roles from being created as them?
>
> Maybe we could in some future release, but we can't for back-branches so
> I'm afraid we're going to have to figure out how to fix this to work
> regardless.

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 Unicode escape value at or near "\00""

As a dirty and quick hack we can use some integer values prfixed
by single zero byte to represent special roles such as
CURRENT_USER.

| RoleId_or_curruser: RoleId { $$ = $1; }
| | CURRENT_USER { $$ = "\x00\x01";};
....
| Oid ResolveRoleId(const char *name, bool missing_ok)
| {
| Oid roleid;
|
| if (name[0] == 0 && name[1] == 1)
| roleid = GetUserId();

This is ugly but needs no additional struct member or special
logics. (Macros could make them look better.)

> > 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 syntax (which I think I
> > could see some value in). However, I think it has been taken beyond just
> > resolving the inconsistency and started to cross over into feature creep.
> > Is the intent simply to resolve inconsistencies between what is now an
> > alias of another command? Or is it to add new functionality? I think the
> > original proposal needs to be revisited and more time needs to be spent
> > defining the scope and purpose of this patch.
>
> 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
> new feature and independent of making role-vs-user cases more
> consistent.

I agree that role-vs-user compatibility is another problem.

In a sense, the "CURRENT_USER" problem is also a separate problem
but simplly spreading current "current_user" mechanism (which
cannot allow using the words literally even if double-quoted) out
to other commands is a kind of pandemic so it should be fixed
before making current_user usable in other commands.

From a view of comptibility (specification stability), fixing
this problem could break someone's application if he/she uses
"current_user" in the meaning of CURRENT_USER intentionally but I
think it is least likely.

As another problem, in the defenition of grantee, there is the
following comment.

| /* This hack lets us avoid reserving PUBLIC as a keyword*/
| if (strcmp($1, "public") == 0)

Please let me know the reason to avoid registering new keyword
making the word unusable as an literal identifier, if any?

PUBLIC and NONE ought to can be identifiers but in reality
unusable because they are handled as keywords in literal
form. Thses are fixed by making them real keywords.

So if there's no particular reason, I will register new keywords
"PUBLIC" and "NONE" as another patch.

# However, I don't think that considerable number of people want
# to do that..

On the other hand, "pg_*" as shcmea names and operators are cases
simply of special names, which cannot be identifiers from the
first so it should be as it is, I think.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center