Re: change alter user to be a true alias for alter role - Mailing list pgsql-hackers

Jov <amutu@amutu.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

*** src/backend/parser/gram.y~    Sat Aug 23 14:53:23 2014
--- src/backend/parser/gram.y    Sat Aug 23 18:10:56 2014
*************** static Node *makeRecursiveViewSelect(cha
*** 230,236 ****
          AlterFdwStmt AlterForeignServerStmt AlterGroupStmt
          AlterObjectSchemaStmt AlterOwnerStmt AlterSeqStmt AlterSystemStmt AlterTableStmt
          AlterTblSpcStmt AlterExtensionStmt AlterExtensionContentsStmt AlterForeignTableStmt
!         AlterCompositeTypeStmt AlterUserStmt AlterUserMappingStmt AlterUserSetStmt
          AlterRoleStmt AlterRoleSetStmt
          AlterDefaultPrivilegesStmt DefACLAction
          AnalyzeStmt ClosePortalStmt ClusterStmt CommentStmt
--- 230,236 ----
          AlterFdwStmt AlterForeignServerStmt AlterGroupStmt
          AlterObjectSchemaStmt AlterOwnerStmt AlterSeqStmt AlterSystemStmt AlterTableStmt
          AlterTblSpcStmt AlterExtensionStmt AlterExtensionContentsStmt AlterForeignTableStmt
!         AlterCompositeTypeStmt AlterUserMappingStmt
          AlterRoleStmt AlterRoleSetStmt
          AlterDefaultPrivilegesStmt DefACLAction
          AnalyzeStmt ClosePortalStmt ClusterStmt CommentStmt
*************** stmt :
*** 749,756 ****
              | AlterTSConfigurationStmt
              | AlterTSDictionaryStmt
              | AlterUserMappingStmt
-             | AlterUserSetStmt
-             | AlterUserStmt
              | AnalyzeStmt
              | CheckPointStmt
              | ClosePortalStmt
--- 749,754 ----
*************** CreateUserStmt:
*** 1020,1029 ****
   *
   * Alter a postgresql DBMS role
   *
   *****************************************************************************/

  AlterRoleStmt:
!             ALTER ROLE RoleId opt_with AlterOptRoleList
                   {
                      AlterRoleStmt *n = makeNode(AlterRoleStmt);
                      n->role = $3;
--- 1018,1029 ----
   *
   * Alter a postgresql DBMS role
   *
+  * ALTER USER is accepted interchangeably with ALTER ROLE.
+  *
   *****************************************************************************/

  AlterRoleStmt:
!             ALTER role_or_user RoleId opt_with AlterOptRoleList
                   {
                      AlterRoleStmt *n = makeNode(AlterRoleStmt);
                      n->role = $3;
*************** AlterRoleStmt:
*** 1033,1045 ****
                   }
          ;

  opt_in_database:
                 /* EMPTY */                    { $$ = NULL; }
              | IN_P DATABASE database_name    { $$ = $3; }
          ;

  AlterRoleSetStmt:
!             ALTER ROLE RoleId opt_in_database SetResetClause
                  {
                      AlterRoleSetStmt *n = makeNode(AlterRoleSetStmt);
                      n->role = $3;
--- 1033,1050 ----
                   }
          ;

+ role_or_user:
+             ROLE
+             | USER
+         ;
+
  opt_in_database:
                 /* EMPTY */                    { $$ = NULL; }
              | IN_P DATABASE database_name    { $$ = $3; }
          ;

  AlterRoleSetStmt:
!             ALTER role_or_user RoleId opt_in_database SetResetClause
                  {
                      AlterRoleSetStmt *n = makeNode(AlterRoleSetStmt);
                      n->role = $3;
*************** AlterRoleSetStmt:
*** 1047,1053 ****
                      n->setstmt = $5;
                      $$ = (Node *)n;
                  }
!             | ALTER ROLE ALL opt_in_database SetResetClause
                  {
                      AlterRoleSetStmt *n = makeNode(AlterRoleSetStmt);
                      n->role = NULL;
--- 1052,1058 ----
                      n->setstmt = $5;
                      $$ = (Node *)n;
                  }
!             | ALTER role_or_user ALL opt_in_database SetResetClause
                  {
                      AlterRoleSetStmt *n = makeNode(AlterRoleSetStmt);
                      n->role = NULL;
*************** AlterRoleSetStmt:
*** 1060,1095 ****

  /*****************************************************************************
   *
-  * Alter a postgresql DBMS user
-  *
-  *****************************************************************************/
-
- AlterUserStmt:
-             ALTER USER RoleId opt_with AlterOptRoleList
-                  {
-                     AlterRoleStmt *n = makeNode(AlterRoleStmt);
-                     n->role = $3;
-                     n->action = +1;    /* add, if there are members */
-                     n->options = $5;
-                     $$ = (Node *)n;
-                  }
-         ;
-
-
- AlterUserSetStmt:
-             ALTER USER RoleId SetResetClause
-                 {
-                     AlterRoleSetStmt *n = makeNode(AlterRoleSetStmt);
-                     n->role = $3;
-                     n->database = NULL;
-                     n->setstmt = $4;
-                     $$ = (Node *)n;
-                 }
-             ;
-
-
- /*****************************************************************************
-  *
   * Drop a postgresql DBMS role
   *
   * XXX Ideally this would have CASCADE/RESTRICT options, but since a role
--- 1065,1070 ----
*************** DropUserMappingStmt: DROP USER MAPPING F
*** 4463,4471 ****
   *        QUERY :
   *                ALTER USER MAPPING FOR auth_ident SERVER name OPTIONS
   *
   ****************************************************************************/

! AlterUserMappingStmt: ALTER USER MAPPING FOR auth_ident SERVER name alter_generic_options
                  {
                      AlterUserMappingStmt *n = makeNode(AlterUserMappingStmt);
                      n->username = $5;
--- 4438,4451 ----
   *        QUERY :
   *                ALTER USER MAPPING FOR auth_ident SERVER name OPTIONS
   *
+  * Note: we also accept "ALTER ROLE MAPPING", because distinguishing that
+  * case in the grammar would require us to distinguish ROLE from USER in all
+  * other ALTER cases, and we don't want to do that.  However, that spelling
+  * of the command is nonstandard and is not documented.
+  *
   ****************************************************************************/

! AlterUserMappingStmt: ALTER role_or_user MAPPING FOR auth_ident SERVER name alter_generic_options
                  {
                      AlterUserMappingStmt *n = makeNode(AlterUserMappingStmt);
                      n->username = $5;
*************** RenameStmt: ALTER AGGREGATE func_name ag
*** 7464,7479 ****
                      n->newname = $7;
                      $$ = (Node *)n;
                  }
!             | ALTER ROLE RoleId RENAME TO RoleId
!                 {
!                     RenameStmt *n = makeNode(RenameStmt);
!                     n->renameType = OBJECT_ROLE;
!                     n->subname = $3;
!                     n->newname = $6;
!                     n->missing_ok = false;
!                     $$ = (Node *)n;
!                 }
!             | ALTER USER RoleId RENAME TO RoleId
                  {
                      RenameStmt *n = makeNode(RenameStmt);
                      n->renameType = OBJECT_ROLE;
--- 7444,7450 ----
                      n->newname = $7;
                      $$ = (Node *)n;
                  }
!             | ALTER role_or_user RoleId RENAME TO RoleId
                  {
                      RenameStmt *n = makeNode(RenameStmt);
                      n->renameType = OBJECT_ROLE;