make more use of RoleSpec struct

From 6f65b08e55d0a1005536c0661c7f857dac29d3bf Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Tue, 27 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.
---
 src/backend/catalog/aclchk.c   |  6 +++---
 src/backend/commands/policy.c  |  2 +-
 src/backend/commands/user.c    | 10 +++++-----
 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, 39 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..5aaf6cf088 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,7 +1396,7 @@ roleSpecsToIds(List *memberNames)
 
 	foreach(l, memberNames)
 	{
-		Node	   *rolespec = (Node *) lfirst(l);
+		RoleSpec   *rolespec = (RoleSpec *) lfirst(l);
 		Oid			roleid;
 
 		roleid = get_rolespec_oid(rolespec, false);
@@ -1493,7 +1493,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 +1508,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 +1619,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 <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 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.11.0