Re: change alter user to be a true alias for alter role

the doc say:

> ALTER USER is now an alias for ALTER ROLE<http://www.postgresql.org/docs/devel/static/sql-alterrole.html>
> .

but alter user lack the following format:

> ALTER ROLE name [ IN DATABASE database_name ] SET configuration_parameter{ TO | = } {
> value | DEFAULT }
> ALTER ROLE { name | ALL } [ IN DATABASE database_name ] SET
> configuration_parameter FROM CURRENT
> ALTER ROLE { name | ALL } [ IN DATABASE database_name ] RESET
> configuration_parameter
> ALTER ROLE { name | ALL } [ IN DATABASE database_name ] RESET ALL

attach patch add the per database set for alter user:
ALTER
USER
name [ IN DATABASE database_name ] SET configuration_parameter { TO | = }
{ value | DEFAULT }
ALTER
USER
{ name | ALL } [ IN DATABASE database_name ] SET configuration_parameter FROM
CURRENT
ALTER
USER
{ name | ALL } [ IN DATABASE database_name ] RESET configuration_parameter
ALTER
USER
{ name | ALL } [ IN DATABASE database_name ] RESET ALL

this make alter user a true alias for alter role.

Jov
blog: http:amutu.com/blog <http://amutu.com/blog>

Jov <amutu(at)amutu(dot)com> wrote:

> attach patch add the per database set for alter user

Please add to the open CommitFest so that the patch doesn't "fall
through the cracks":

https://commitfest.postgresql.org/action/commitfest_view/open

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 01/20/2014 03:31 PM, Jov wrote:
> the doc say:
>
> ALTER USER is now an alias for ALTER ROLE
> <http://www.postgresql.org/docs/devel/static/sql-alterrole.html>.
>
>
> but alter user lack the following format:
>
> [...]
>
> this make alter user a true alias for alter role.

This is a straightforward patch that does exactly what it says on the
tin. I have marked it ready for committer.
--
Vik

Jov <amutu(at)amutu(dot)com> writes:
> the doc say:
>> ALTER USER is now an alias for ALTER ROLE<http://www.postgresql.org/docs/devel/static/sql-alterrole.html>

> but alter user lack the following format:
> ...

If we're going to have a policy that these commands be exactly equivalent,
it seems like this patch is just sticking a finger into the dike. It does
nothing to prevent anyone from making the same mistake again in future.

What about collapsing both sets of productions into one, along the lines
of

role_or_user: ROLE | USER;

AlterRoleSetStmt:
ALTER role_or_user RoleId opt_in_database SetResetClause

(and similarly to the latter for every existing ALTER ROLE variant).

Because MAPPING is an unreserved keyword, I think that this approach
might force us to also change ALTER USER MAPPING to ALTER role_or_user
MAPPING, which is not contemplated by the SQL standard. But hey,
it would satisfy the principle of least surprise no? Anyway we don't
have to document that that would work.

regards, tom lane

On 06/19/2014 07:18 PM, Tom Lane wrote:
> Jov <amutu(at)amutu(dot)com> writes:
>> the doc say:
>>> ALTER USER is now an alias for ALTER ROLE<http://www.postgresql.org/docs/devel/static/sql-alterrole.html>
>
>> but alter user lack the following format:
>> ...
>
> If we're going to have a policy that these commands be exactly equivalent,
> it seems like this patch is just sticking a finger into the dike. It does
> nothing to prevent anyone from making the same mistake again in future.
>
> What about collapsing both sets of productions into one, along the lines
> of
>
> role_or_user: ROLE | USER;
>
> AlterRoleSetStmt:
> ALTER role_or_user RoleId opt_in_database SetResetClause
>
> (and similarly to the latter for every existing ALTER ROLE variant).

I thought about suggesting that, and it seems that I should have. I
also thought about suggesting adding GROUP as an alias, too. That's
probably not as good of an idea.

> Because MAPPING is an unreserved keyword, I think that this approach
> might force us to also change ALTER USER MAPPING to ALTER role_or_user
> MAPPING, which is not contemplated by the SQL standard. But hey,
> it would satisfy the principle of least surprise no? Anyway we don't
> have to document that that would work.

That's a small price to pay, so I'm all for accepting it. I agree that
it doesn't need to be documented.
--
Vik

On 06/19/2014 07:18 PM, Tom Lane wrote:
> Jov <amutu(at)amutu(dot)com> writes:
>> the doc say:
>>> ALTER USER is now an alias for ALTER ROLE<http://www.postgresql.org/docs/devel/static/sql-alterrole.html>
>
>> but alter user lack the following format:
>> ...
>
> If we're going to have a policy that these commands be exactly equivalent,
> it seems like this patch is just sticking a finger into the dike. It does
> nothing to prevent anyone from making the same mistake again in future.
>
> What about collapsing both sets of productions into one, along the lines
> of
>
> role_or_user: ROLE | USER;
>
> AlterRoleSetStmt:
> ALTER role_or_user RoleId opt_in_database SetResetClause
>
> (and similarly to the latter for every existing ALTER ROLE variant).
>
> Because MAPPING is an unreserved keyword, I think that this approach
> might force us to also change ALTER USER MAPPING to ALTER role_or_user
> MAPPING, which is not contemplated by the SQL standard. But hey,
> it would satisfy the principle of least surprise no? Anyway we don't
> have to document that that would work.

After a week of silence from Jov, I decided to do this myself since it
didn't seem very hard.

Many frustrating hours of trying to understand why I'm getting
shift/reduce conflicts by the hundreds later, I've decided to give up
for now.
--
Vik

At 2014-06-27 16:11:21 +0200, vik(dot)fearing(at)dalibo(dot)com wrote:
>
> After a week of silence from Jov, I decided to do this myself since it
> didn't seem very hard.
>
> Many frustrating hours of trying to understand why I'm getting
> shift/reduce conflicts by the hundreds later, I've decided to give up
> for now.

Jov, do you expect to be able to work on the patch along the lines Tom
suggested and resubmit during this CF?

If not, since Vik gave up on it, it seems to me that it would be best to
mark it returned with feedback (which I'll do in a couple of days if I
don't hear anything to the contrary).

-- Abhijit

Sorry for the late resp,I will try to update the patch.

Jov
blog: http:amutu.com/blog <http://amutu.com/blog>

2014-07-02 4:17 GMT+08:00 Abhijit Menon-Sen <ams(at)2ndquadrant(dot)com>:

> At 2014-06-27 16:11:21 +0200, vik(dot)fearing(at)dalibo(dot)com wrote:
> >
> > After a week of silence from Jov, I decided to do this myself since it
> > didn't seem very hard.
> >
> > Many frustrating hours of trying to understand why I'm getting
> > shift/reduce conflicts by the hundreds later, I've decided to give up
> > for now.
>
> Jov, do you expect to be able to work on the patch along the lines Tom
> suggested and resubmit during this CF?
>
> If not, since Vik gave up on it, it seems to me that it would be best to
> mark it returned with feedback (which I'll do in a couple of days if I
> don't hear anything to the contrary).
>
> -- Abhijit
>

I make the v2 of the patch,use Tom's advice.

But I can't make ROLE and USER in the keyword list,it is hard to solve the
conflict,or rewrite many gram rules.
the problem is :

role_or_user : ROLE | USER;
xx_keyword:...| ROLE|...|USER..;

this two rules produce conflict.So in v2,I remove ROLE and USER from the
xx_keyword rule now?
Any idea?

Jov
blog: http:amutu.com/blog <http://amutu.com/blog>

2014-07-09 18:27 GMT+08:00 Jov <amutu(at)amutu(dot)com>:

> Sorry for the late resp,I will try to update the patch.
>
> Jov
> blog: http:amutu.com/blog <http://amutu.com/blog>
>
>
> 2014-07-02 4:17 GMT+08:00 Abhijit Menon-Sen <ams(at)2ndquadrant(dot)com>:
>
> At 2014-06-27 16:11:21 +0200, vik(dot)fearing(at)dalibo(dot)com wrote:
>> >
>> > After a week of silence from Jov, I decided to do this myself since it
>> > didn't seem very hard.
>> >
>> > Many frustrating hours of trying to understand why I'm getting
>> > shift/reduce conflicts by the hundreds later, I've decided to give up
>> > for now.
>>
>> Jov, do you expect to be able to work on the patch along the lines Tom
>> suggested and resubmit during this CF?
>>
>> If not, since Vik gave up on it, it seems to me that it would be best to
>> mark it returned with feedback (which I'll do in a couple of days if I
>> don't hear anything to the contrary).
>>
>> -- Abhijit
>>
>
>

Jov <amutu(at)amutu(dot)com> writes:
> I make the v2 of the patch,use Tom's advice.
> But I can't make ROLE and USER in the keyword list,it is hard to solve the
> conflict,or rewrite many gram rules.

This patch seems to be trying to make ROLE and USER interchangeable
*everywhere* in the grammar, which is moving the goalposts quite a long
way to little purpose. The idea seems rather misguided to me, since
certainly CREATE USER and CREATE ROLE will never be exact synonyms.
I think it's sufficient to solve the original complaint about them not
being interchangeable in ALTER.

The patch as given is broken anyway: it removes both those keywords
from the keyword lists, which effectively makes them fully reserved,
in fact worse than fully reserved.

What I had in mind was more like the attached, which I've not tested
but bison does compile it without complaint. I've not looked at the
documentation end of it, either.

regards, tom lane