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,