Here is a small cleanup patch to make more use of the RoleSpec
struct/node throughout the parser to avoid casts and make the code more
self-documenting.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From 6f65b08e55d0a1005536c0661c7f857dac29d3bf Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
