Make more use of RoleSpec struct

Most code was casting this through a generic Node.  By declaring
everything as RoleSpec appropriately, we can remove a bunch of casts and
ad-hoc node type checking.

Reviewed-by: Alvaro Herrera <alvherre@2ndquadrant.com>

diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c
index 3086021432ab3abed6660669ceff3415740319fa..fb6c276eaf86c1697772a4297a3f07bbf755bdb1 100644 (file)
--- a/src/backend/catalog/aclchk.c
+++ b/src/backend/catalog/aclchk.c
@@ -423,7 +423,7 @@ ExecuteGrantStmt(GrantStmt *stmt)
                grantee_uid = ACL_ID_PUBLIC;
                break;
            default:
-               grantee_uid = get_rolespec_oid((Node *) grantee, false);
+               grantee_uid = get_rolespec_oid(grantee, false);
                break;
        }
        istmt.grantees = lappend_oid(istmt.grantees, grantee_uid);
@@ -922,7 +922,7 @@ ExecAlterDefaultPrivilegesStmt(ParseState *pstate, AlterDefaultPrivilegesStmt *s
                grantee_uid = ACL_ID_PUBLIC;
                break;
            default:
-               grantee_uid = get_rolespec_oid((Node *) grantee, false);
+               grantee_uid = get_rolespec_oid(grantee, false);
                break;
        }
        iacls.grantees = lappend_oid(iacls.grantees, grantee_uid);
@@ -1012,7 +1012,7 @@ ExecAlterDefaultPrivilegesStmt(ParseState *pstate, AlterDefaultPrivilegesStmt *s
        {
            RoleSpec   *rolespec = lfirst(rolecell);
 
-           iacls.roleid = get_rolespec_oid((Node *) rolespec, false);
+           iacls.roleid = get_rolespec_oid(rolespec, false);
 
            /*
             * We insist that calling user be a member of each target role. If

diff --git a/src/backend/commands/policy.c b/src/backend/commands/policy.c
index 6da3205c9e15d6039ee36f5db6b8c0bdcdcd9edb..3746bf1f9f13fbb812fc10d8e6ba6a232bf46cf6 100644 (file)
--- a/src/backend/commands/policy.c
+++ b/src/backend/commands/policy.c
@@ -177,7 +177,7 @@ policy_role_list_to_array(List *roles, int *num_roles)
        }
        else
            role_oids[i++] =
-               ObjectIdGetDatum(get_rolespec_oid((Node *) spec, false));
+               ObjectIdGetDatum(get_rolespec_oid(spec, false));
    }
 
    return role_oids;

diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c
index adc6b99b215d16847a6c41071e89faf4e303a455..a3521e77570e9d16ede9b44f4ff9e328b65cedb1 100644 (file)
--- a/src/backend/commands/user.c
+++ b/src/backend/commands/user.c
@@ -449,7 +449,7 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
    foreach(item, addroleto)
    {
        RoleSpec   *oldrole = lfirst(item);
-       HeapTuple   oldroletup = get_rolespec_tuple((Node *) oldrole);
+       HeapTuple   oldroletup = get_rolespec_tuple(oldrole);
        Oid         oldroleid = HeapTupleGetOid(oldroletup);
        char       *oldrolename = NameStr(((Form_pg_authid) GETSTRUCT(oldroletup))->rolname);
 
@@ -1396,9 +1396,11 @@ roleSpecsToIds(List *memberNames)
 
    foreach(l, memberNames)
    {
-       Node       *rolespec = (Node *) lfirst(l);
+       RoleSpec   *rolespec = (RoleSpec *) lfirst(l);
        Oid         roleid;
 
+       Assert(IsA(rolespec, RoleSpec));
+
        roleid = get_rolespec_oid(rolespec, false);
        result = lappend_oid(result, roleid);
    }
@@ -1493,7 +1495,7 @@ AddRoleMems(const char *rolename, Oid roleid,
            ereport(ERROR,
                    (errcode(ERRCODE_INVALID_GRANT_OPERATION),
                     (errmsg("role \"%s\" is a member of role \"%s\"",
-                       rolename, get_rolespec_name((Node *) memberRole)))));
+                       rolename, get_rolespec_name(memberRole)))));
 
        /*
         * Check if entry for this role/member already exists; if so, give
@@ -1508,7 +1510,7 @@ AddRoleMems(const char *rolename, Oid roleid,
        {
            ereport(NOTICE,
                    (errmsg("role \"%s\" is already a member of role \"%s\"",
-                        get_rolespec_name((Node *) memberRole), rolename)));
+                        get_rolespec_name(memberRole), rolename)));
            ReleaseSysCache(authmem_tuple);
            continue;
        }
@@ -1619,7 +1621,7 @@ DelRoleMems(const char *rolename, Oid roleid,
        {
            ereport(WARNING,
                    (errmsg("role \"%s\" is not a member of role \"%s\"",
-                        get_rolespec_name((Node *) memberRole), rolename)));
+                        get_rolespec_name(memberRole), rolename)));
            continue;
        }
 

diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 834a00971a93fccd3f3516196a3a9f6e3425308b..08cf5b78f5c5c1398e2e0af52b56a29ed69477a7 100644 (file)
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -144,7 +144,7 @@ static Node *makeBitStringConst(char *str, int location);
 static Node *makeNullAConst(int location);
 static Node *makeAConst(Value *v, int location);
 static Node *makeBoolAConst(bool state, int location);
-static Node *makeRoleSpec(RoleSpecType type, int location);
+static RoleSpec *makeRoleSpec(RoleSpecType type, int location);
 static void check_qualified_name(List *names, core_yyscan_t yyscanner);
 static List *check_func_name(List *names, core_yyscan_t yyscanner);
 static List *check_indirection(List *indirection, core_yyscan_t yyscanner);
@@ -231,6 +231,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
    PartitionElem       *partelem;
    PartitionSpec       *partspec;
    PartitionRangeDatum *partrange_datum;
+   RoleSpec            *rolespec;
 }
 
 %type <node>   stmt schema_stmt
@@ -339,7 +340,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
 %type <list>   RowSecurityDefaultToRole RowSecurityOptionalToRole
 
 %type <str>        iso_level opt_encoding
-%type <node>   grantee
+%type <rolespec> grantee
 %type <list>   grantee_list
 %type <accesspriv> privilege
 %type <list>   privileges privilege_list
@@ -506,7 +507,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
 %type <str>        NonReservedWord NonReservedWord_or_Sconst
 %type <str>        createdb_opt_name
 %type <node>   var_value zone_value
-%type <node>   auth_ident RoleSpec opt_granted_by
+%type <rolespec> auth_ident RoleSpec opt_granted_by
 
 %type <keyword> unreserved_keyword type_func_name_keyword
 %type <keyword> col_name_keyword reserved_keyword
@@ -522,7 +523,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
 %type <list>   constraints_set_list
 %type <boolean> constraints_set_mode
 %type <str>        OptTableSpace OptConsTableSpace
-%type <node>   OptTableSpaceOwner
+%type <rolespec> OptTableSpaceOwner
 %type <ival>   opt_check_option
 
 %type <str>        opt_provider security_label
@@ -13918,10 +13919,10 @@ RoleSpec: NonReservedWord
                        }
                        else
                        {
-                           n = (RoleSpec *) makeRoleSpec(ROLESPEC_CSTRING, @1);
+                           n = makeRoleSpec(ROLESPEC_CSTRING, @1);
                            n->rolename = pstrdup($1);
                        }
-                       $$ = (Node *) n;
+                       $$ = n;
                    }
            | CURRENT_USER
                    {
@@ -14644,7 +14645,7 @@ makeBoolAConst(bool state, int location)
 /* makeRoleSpec
  * Create a RoleSpec with the given type
  */
-static Node *
+static RoleSpec *
 makeRoleSpec(RoleSpecType type, int location)
 {
    RoleSpec *spec = makeNode(RoleSpec);
@@ -14652,7 +14653,7 @@ makeRoleSpec(RoleSpecType type, int location)
    spec->roletype = type;
    spec->location = location;
 
-   return (Node *) spec;
+   return spec;
 }
 
 /* check_qualified_name --- check the result of qualified_name production

diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c
index 025a99e55afa434861f53e627fd687fdf45c366e..cc4e0b3f815b3dcb663f4e677b68008f598c2a5a 100644 (file)
--- a/src/backend/utils/adt/acl.c
+++ b/src/backend/utils/adt/acl.c
@@ -5143,15 +5143,10 @@ get_role_oid_or_public(const char *rolname)
  * case must check the case separately.
  */
 Oid
-get_rolespec_oid(const Node *node, bool missing_ok)
+get_rolespec_oid(const RoleSpec *role, bool missing_ok)
 {
-   RoleSpec   *role;
    Oid         oid;
 
-   if (!IsA(node, RoleSpec))
-       elog(ERROR, "invalid node type %d", node->type);
-
-   role = (RoleSpec *) node;
    switch (role->roletype)
    {
        case ROLESPEC_CSTRING:
@@ -5186,15 +5181,10 @@ get_rolespec_oid(const Node *node, bool missing_ok)
  * Caller must ReleaseSysCache when done with the result tuple.
  */
 HeapTuple
-get_rolespec_tuple(const Node *node)
+get_rolespec_tuple(const RoleSpec *role)
 {
-   RoleSpec   *role;
    HeapTuple   tuple;
 
-   role = (RoleSpec *) node;
-   if (!IsA(node, RoleSpec))
-       elog(ERROR, "invalid node type %d", node->type);
-
    switch (role->roletype)
    {
        case ROLESPEC_CSTRING:
@@ -5235,13 +5225,13 @@ get_rolespec_tuple(const Node *node)
  * Given a RoleSpec, returns a palloc'ed copy of the corresponding role's name.
  */
 char *
-get_rolespec_name(const Node *node)
+get_rolespec_name(const RoleSpec *role)
 {
    HeapTuple   tp;
    Form_pg_authid authForm;
    char       *rolename;
 
-   tp = get_rolespec_tuple(node);
+   tp = get_rolespec_tuple(role);
    authForm = (Form_pg_authid) GETSTRUCT(tp);
    rolename = pstrdup(NameStr(authForm->rolname));
    ReleaseSysCache(tp);
@@ -5257,17 +5247,11 @@ get_rolespec_name(const Node *node)
  * message is provided.
  */
 void
-check_rolespec_name(const Node *node, const char *detail_msg)
+check_rolespec_name(const RoleSpec *role, const char *detail_msg)
 {
-   RoleSpec   *role;
-
-   if (!node)
+   if (!role)
        return;
 
-   role = (RoleSpec *) node;
-
-   Assert(IsA(node, RoleSpec));
-
    if (role->roletype != ROLESPEC_CSTRING)
        return;
 

diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index fc532fbd432cbf35dcc1ae870e1d065b18c70755..9d8ef775e4f223471298c27bc48a39719acbc187 100644 (file)
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -1542,7 +1542,7 @@ typedef struct CreateSchemaStmt
 {
    NodeTag     type;
    char       *schemaname;     /* the name of the schema to create */
-   Node       *authrole;       /* the owner of the created schema */
+   RoleSpec   *authrole;       /* the owner of the created schema */
    List       *schemaElts;     /* schema components (list of parsenodes) */
    bool        if_not_exists;  /* just do nothing if schema already exists? */
 } CreateSchemaStmt;
@@ -1647,7 +1647,7 @@ typedef struct AlterTableCmd  /* one subcommand of an ALTER TABLE */
    AlterTableType subtype;     /* Type of table alteration to apply */
    char       *name;           /* column, constraint, or trigger to act on,
                                 * or tablespace */
-   Node       *newowner;       /* RoleSpec */
+   RoleSpec   *newowner;
    Node       *def;            /* definition of new column, index,
                                 * constraint, or parent table */
    DropBehavior behavior;      /* RESTRICT or CASCADE for DROP cases */
@@ -1766,7 +1766,7 @@ typedef struct GrantRoleStmt
    List       *grantee_roles;  /* list of member roles to add/delete */
    bool        is_grant;       /* true = GRANT, false = REVOKE */
    bool        admin_opt;      /* with admin option */
-   Node       *grantor;        /* set grantor to other than current role */
+   RoleSpec   *grantor;        /* set grantor to other than current role */
    DropBehavior behavior;      /* drop behavior (for REVOKE) */
 } GrantRoleStmt;
 
@@ -1981,7 +1981,7 @@ typedef struct CreateTableSpaceStmt
 {
    NodeTag     type;
    char       *tablespacename;
-   Node       *owner;
+   RoleSpec   *owner;
    char       *location;
    List       *options;
 } CreateTableSpaceStmt;
@@ -2107,7 +2107,7 @@ typedef struct CreateForeignTableStmt
 typedef struct CreateUserMappingStmt
 {
    NodeTag     type;
-   Node       *user;           /* user role */
+   RoleSpec   *user;           /* user role */
    char       *servername;     /* server name */
    List       *options;        /* generic options to server */
 } CreateUserMappingStmt;
@@ -2115,7 +2115,7 @@ typedef struct CreateUserMappingStmt
 typedef struct AlterUserMappingStmt
 {
    NodeTag     type;
-   Node       *user;           /* user role */
+   RoleSpec   *user;           /* user role */
    char       *servername;     /* server name */
    List       *options;        /* generic options to server */
 } AlterUserMappingStmt;
@@ -2123,7 +2123,7 @@ typedef struct AlterUserMappingStmt
 typedef struct DropUserMappingStmt
 {
    NodeTag     type;
-   Node       *user;           /* user role */
+   RoleSpec   *user;           /* user role */
    char       *servername;     /* server name */
    bool        missing_ok;     /* ignore missing mappings */
 } DropUserMappingStmt;
@@ -2288,7 +2288,7 @@ typedef struct CreateRoleStmt
 typedef struct AlterRoleStmt
 {
    NodeTag     type;
-   Node       *role;           /* role */
+   RoleSpec   *role;           /* role */
    List       *options;        /* List of DefElem nodes */
    int         action;         /* +1 = add members, -1 = drop members */
 } AlterRoleStmt;
@@ -2296,7 +2296,7 @@ typedef struct AlterRoleStmt
 typedef struct AlterRoleSetStmt
 {
    NodeTag     type;
-   Node       *role;           /* role */
+   RoleSpec   *role;           /* role */
    char       *database;       /* database name, or NULL */
    VariableSetStmt *setstmt;   /* SET or RESET subcommand */
 } AlterRoleSetStmt;
@@ -2687,7 +2687,7 @@ typedef struct AlterOwnerStmt
    RangeVar   *relation;       /* in case it's a table */
    List       *object;         /* in case it's some other object */
    List       *objarg;         /* argument types, if applicable */
-   Node       *newowner;       /* the new owner */
+   RoleSpec   *newowner;       /* the new owner */
 } AlterOwnerStmt;
 
 
@@ -3171,7 +3171,7 @@ typedef struct ReassignOwnedStmt
 {
    NodeTag     type;
    List       *roles;
-   Node       *newrole;
+   RoleSpec   *newrole;
 } ReassignOwnedStmt;
 
 /*

diff --git a/src/include/utils/acl.h b/src/include/utils/acl.h
index fda75bb6180297fa9eee9fdcb51782da6e7e63ee..faadebd26a9aad5b5d9aeac734ce1d293ff9ac01 100644 (file)
--- a/src/include/utils/acl.h
+++ b/src/include/utils/acl.h
@@ -231,10 +231,10 @@ extern bool is_admin_of_role(Oid member, Oid role);
 extern void check_is_member_of_role(Oid member, Oid role);
 extern Oid get_role_oid(const char *rolename, bool missing_ok);
 extern Oid get_role_oid_or_public(const char *rolename);
-extern Oid get_rolespec_oid(const Node *node, bool missing_ok);
-extern void check_rolespec_name(const Node *node, const char *detail_msg);
-extern HeapTuple get_rolespec_tuple(const Node *node);
-extern char *get_rolespec_name(const Node *node);
+extern Oid get_rolespec_oid(const RoleSpec *role, bool missing_ok);
+extern void check_rolespec_name(const RoleSpec *role, const char *detail_msg);
+extern HeapTuple get_rolespec_tuple(const RoleSpec *role);
+extern char *get_rolespec_name(const RoleSpec *role);
 
 extern void select_best_grantor(Oid roleId, AclMode privileges,
                    const Acl *acl, Oid ownerId,