oddity with ALTER ROLE/USER

Started by Joe Conwayalmost 7 years ago4 messages

mail@joeconway.com

almost 7 years ago

1 attachment(s)

I noticed that ALTER ROLE/USER succeeds even when called without any
options:

postgres=# alter user foo;
ALTER ROLE
postgres=# alter role foo;
ALTER ROLE
postgres=# alter group foo;
ERROR: syntax error at or near ";"
LINE 1: alter group foo;

That seems odd, does nothing useful, and is inconsistent with, for
example, ALTER GROUP as shown above.

Proposed patch attached.

Comments/thoughts?

Joe

--
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development

Attachments:

alter-role-00.patchtext/x-patch; name=alter-role-00.patchDownload

diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c
index 8340948..a71217e 100644
*** a/src/backend/commands/user.c
--- b/src/backend/commands/user.c
*************** AlterRole(AlterRoleStmt *stmt)
*** 549,554 ****
--- 549,559 ----
  	check_rolespec_name(stmt->role,
  						"Cannot alter reserved roles.");
  
+ 	if (list_length(stmt->options) == 0)
+ 		ereport(ERROR,
+ 				(errcode(ERRCODE_SYNTAX_ERROR),
+ 				 errmsg("no options specified")));
+ 
  	/* Extract options from the statement node tree */
  	foreach(option, stmt->options)
  	{

Tom Lane

tgl@sss.pgh.pa.us

almost 7 years ago

In reply to: Joe Conway (#1)

Re: oddity with ALTER ROLE/USER

Joe Conway <mail@joeconway.com> writes:

I noticed that ALTER ROLE/USER succeeds even when called without any
options:

postgres=# alter user foo;
ALTER ROLE
postgres=# alter role foo;
ALTER ROLE
postgres=# alter group foo;
ERROR: syntax error at or near ";"
LINE 1: alter group foo;

That seems odd, does nothing useful, and is inconsistent with, for
example, ALTER GROUP as shown above.

Proposed patch attached.

If you want to make it act like alter group, why not make it act
like alter group? That is, the way to fix this is to change the
grammar so that AlterOptRoleList doesn't permit an expansion with
zero list elements.

Having said that, I can't get excited about changing this at all.
Nobody will thank us for it, and someone might complain.

regards, tom lane

Joe Conway

mail@joeconway.com

almost 7 years ago

In reply to: Tom Lane (#2)

Re: oddity with ALTER ROLE/USER

On 2/22/19 4:19 PM, Tom Lane wrote:

Joe Conway <mail@joeconway.com> writes:

I noticed that ALTER ROLE/USER succeeds even when called without any
options:

postgres=# alter user foo;
ALTER ROLE
postgres=# alter role foo;
ALTER ROLE
postgres=# alter group foo;
ERROR: syntax error at or near ";"
LINE 1: alter group foo;

That seems odd, does nothing useful, and is inconsistent with, for
example, ALTER GROUP as shown above.

Proposed patch attached.

If you want to make it act like alter group, why not make it act
like alter group? That is, the way to fix this is to change the
grammar so that AlterOptRoleList doesn't permit an expansion with
zero list elements.

I considered that but liked the more specific error message.

Having said that, I can't get excited about changing this at all.
Nobody will thank us for it, and someone might complain.

The other route is change the documentation to reflect reality I guess.

Joe

--
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development

Stephen Frost

sfrost@snowman.net

almost 7 years ago

In reply to: Tom Lane (#2)

Re: oddity with ALTER ROLE/USER

Greetings,

* Tom Lane (tgl@sss.pgh.pa.us) wrote:

Joe Conway <mail@joeconway.com> writes:

I noticed that ALTER ROLE/USER succeeds even when called without any
options:

postgres=# alter user foo;
ALTER ROLE
postgres=# alter role foo;
ALTER ROLE
postgres=# alter group foo;
ERROR: syntax error at or near ";"
LINE 1: alter group foo;

That seems odd, does nothing useful, and is inconsistent with, for
example, ALTER GROUP as shown above.

Proposed patch attached.

If you want to make it act like alter group, why not make it act
like alter group? That is, the way to fix this is to change the
grammar so that AlterOptRoleList doesn't permit an expansion with
zero list elements.

Having said that, I can't get excited about changing this at all.
Nobody will thank us for it, and someone might complain.

Is there no chance that someone's issueing such an ALTER ROLE foo;
statement and thinking it's actually doing something? I suppose it's
pretty unlikely, but we do complain in some cases where we've been asked
to do something and we end up not actually doing anything for various
reasons ("no privileges were granted..."), though that's only a warning.
Maybe that'd be useful to have here though? At least then we're making
it clear to the user that nothing is happening and we don't break
existing things.

Thanks!

Stephen