Re: alter user/role CURRENT_USER

From: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
To: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
Cc: tgl(at)sss(dot)pgh(dot)pa(dot)us, 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: 2015-03-02 22:54:22
Message-ID: 20150302225422.GI3291@alvh.no-ip.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Alvaro Herrera wrote:
> Kyotaro HORIGUCHI wrote:

> Thanks for doing the fiddly work here. Attached is a new version of
> this patch. I simplified some things, including removing those rules
> you added to RoleId. It seems to me that this problem:
>
> > RoleId in the patch still has rule components for CURRENT_USER,
> > SESSION_USER, and CURRENT_ROLE. Without them, the parser prints
> > an error ununderstandable to users.
> >
> > | =# alter role current_user rename to "PuBlic";
> > | ERROR: syntax error at or near "rename"
> > | LINE 1: alter role current_user rename to "PuBlic";
> > | ^
>
> can be fixed without complicating the rest of the stuff simply by using
> RoleSpec instead of RoleId and doing the error checks at the RenameStmt
> production.

I tried that but it's way too messy, so I readded them.

> I couldn't find any further problems with this version of the code,
> though I also noticed that a lot of things are not being tested in the
> regression tests, such as "create user public" or "alter user none". It
> would be good to have tests for such cases, to avoid breaking them
> accidentally. If you can spare some time to submit test cases for such
> commands, I would be thankful.

I later noticed that you had already submitted a test.sql file, so I
adopted it as rolenames.sql and added it to the schedule files. I still
have to read through the results and make sure they make sense, so the
expected file is not in this patch.

I made some more changes to the code; unless the tests uncover something
ugly, the code in this patch is what will be committed.

> I'm pretty sure, thought I haven't tried yet, that we can now remove the
> PrivGrantee node completely.

That's done in the attached.

Documentation is still missing. Are you submitting doc changes soon? I
would like to get this committed.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachment Content-Type Size
0001-ALTER-USER-CURRENT_USER-v6.patch text/x-diff 74.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jeremy Harris 2015-03-02 23:04:31 Re: Abbreviated keys for text cost model fix
Previous Message Fabien COELHO 2015-03-02 22:41:28 Re: add modulo (%) operator to pgbench