From 2e254130d1d3f16575f2d72952ab23b4e27d035a Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Wed, 28 Dec 2016 12:00:00 -0500 Subject: [PATCH] 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 --- src/backend/catalog/aclchk.c | 6 +++--- src/backend/commands/policy.c | 2 +- src/backend/commands/user.c | 12 +++++++----- src/backend/parser/gram.y | 17 +++++++++-------- src/backend/utils/adt/acl.c | 28 ++++++---------------------- src/include/nodes/parsenodes.h | 22 +++++++++++----------- src/include/utils/acl.h | 8 ++++---- 7 files changed, 41 insertions(+), 54 deletions(-) diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c index 3086021432..fb6c276eaf 100644 --- 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 6da3205c9e..3746bf1f9f 100644 --- 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 adc6b99b21..a3521e7757 100644 --- 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 834a00971a..08cf5b78f5 100644 --- 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 stmt schema_stmt @@ -339,7 +340,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); %type RowSecurityDefaultToRole RowSecurityOptionalToRole %type iso_level opt_encoding -%type grantee +%type grantee %type grantee_list %type privilege %type privileges privilege_list @@ -506,7 +507,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); %type NonReservedWord NonReservedWord_or_Sconst %type createdb_opt_name %type var_value zone_value -%type auth_ident RoleSpec opt_granted_by +%type auth_ident RoleSpec opt_granted_by %type unreserved_keyword type_func_name_keyword %type col_name_keyword reserved_keyword @@ -522,7 +523,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); %type constraints_set_list %type constraints_set_mode %type OptTableSpace OptConsTableSpace -%type OptTableSpaceOwner +%type OptTableSpaceOwner %type opt_check_option %type 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 025a99e55a..cc4e0b3f81 100644 --- 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 fc532fbd43..9d8ef775e4 100644 --- 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 fda75bb618..faadebd26a 100644 --- 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, -- 2.40.0