Re: alter user/role CURRENT_USER

From: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
To: tgl(at)sss(dot)pgh(dot)pa(dot)us
Cc: adam(dot)brightwell(at)crunchydatasolutions(dot)com, sfrost(at)snowman(dot)net, marti(at)juffo(dot)org, rushabh(dot)lathia(at)gmail(dot)com, pgsql-hackers(at)postgresql(dot)org
Subject: Re: alter user/role CURRENT_USER
Date: 2014-11-10 10:23:33
Message-ID: 20141110.192333.47702319.horiguchi.kyotaro@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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.

- CREATE USER/GROUP <roleid>
- ALTER ROLE/GROUP/USER <roleid> RENAME TO <newname>

These syntax still do not accept the keywords like CURRENT_USER
and special names like "public" at all, but accepts
"current_user". The error message is changed as follows.

| postgres=# create user current_user;
| ERROR: role name should not be a keyword nor reserved name.
| LINE 1: create user current_user;
| ^

# Some other messages may changed...

USER and CURRENT_ROLE haven't been extended to other syntax. The
former still can be used only in CREATE/ALTER/DROP USER MAPPING
and the latter cannot be used out of function expressions.

- Storage for new information

The new struct NameId stores an identifier which telling what it
logically is using the new enum NameIdTypes.

This is still be a bit suffered by the difference between
CURRENT_USER and PUBLIC but now it makes distinction between
current_user and "current_user". User oid does not have the room
for representing the difference among PUBLIC, NONE and 'not
found' as the result of get_nameid_oid(), so members of NameId is
exposed in foreigncmds.c and it gets a bit uglier.

- Changes of related structs and grammar.

Type of role member is changed to NameId in some of parser
structs. AlterTableCmd.name has many other usage so I added new
member NameId *newowner for exclusive usage.

Type of OWNER clause of CREATE TABLESPACE is changed to RoleId. I
suppose there's no particular reason that the non-terminal was
"name".

Usage of "public" and "none" had been blocked for CREATE/RENAME
USER in user.c but now it is blocked in gram.y

- New function to resolve NameId

New function get_nameid_oid() is added. It is an alternative of
get_role_oid which can handle current_user and "current_user"
properly. get_role_oid() still be used in many places having no
direct relation to syntax.

- Others

No doc provided for now.

regards,

> > Adam Brightwell <adam(dot)brightwell(at)crunchydatasolutions(dot)com> writes:
> > > FWIW, I found the following in the archives:
> >
> > > http://www.postgresql.org/message-id/15516.1038718413@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 else related (it's a big archive).
> > > Though, as I understand it, PUBLIC is now non-reserved as of SQL:2011 which
> > > might make a compelling argument to leave it as is?
> >
> > The current spec does list PUBLIC as a non-reserved keyword, but it also
> > says (5.4 "Names and identifiers" syntax rules)
> >
> > 20) No <authorization identifier> shall specify "PUBLIC".
> >
> > which, oddly enough, seems to license us to handle "PUBLIC" the way
> > we are doing. OTOH, it lists CURRENT_USER as a reserved word, suggesting
> > that they don't think the same type of hack should be used for that.
> >
> > I'd be inclined to leave the grammar as such alone (ie CURRENT_USER is
> > a keyword, PUBLIC isn't). Changing that has more downside than upside,
> > and we do have justification in the spec for treating the two cases
> > differently. However, I agree that we should fix the subsequent
> > processing so that "current_user" is not confused with CURRENT_USER.
>
> Sure, maybe.
>
> - PUBLIC should be left as it is, as an supecial identifier
> which cannot be used even if quoted under some syntax.
>
> - CURRENT_USER should be a kayword as it is, but we shouldn't
> inhibit it from being used as an identifier if
> quoted. SESSION_USER and USER should be treated in the same way.
>
> We don't want to use something other than string (prefixed by
> zero-byte) as a matter of course:). And resolving the name to
> roleid instantly in gram.y is not allowed for the reason shown
> upthread.
>
> So it is necessary to add a new member for the struct, say
> "int special_role;:... Let me have more sane name for it :(
>
> - USER and CURRENT_ROLE are not needed for the syntax other than
> them which already uses them.
>
> I will work on this way. Let me know if something is not acceptable.

--
Kyotaro Horiguchi
NTT Open Source Software Center

Attachment Content-Type Size
0001-ALTER-USER-CURRENT_USER-v2.patch text/x-patch 38.0 KB
unknown_filename text/plain 17.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kouhei Kaigai 2014-11-10 10:48:49 Re: [v9.5] Custom Plan API
Previous Message furuyao 2014-11-10 10:19:30 Re: pg_receivexlog --status-interval add fsync feedback