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