From a2875009acef72e9724edbe3f5c435f50bd15b73 Mon Sep 17 00:00:00 2001 From: David Christensen Date: Tue, 19 Oct 2021 12:23:50 -0500 Subject: [PATCH] Support CREATE ROLE IF NOT EXISTS for roles, users, and groups By choosing to implement CREATE ROLE IF NOT EXISTS instead of CREATE OR REPLACE ROLE, we can return an existing oid while not throwing an error when using this form of the CREATE ROLE statement. This does mean that if writing code which expects a specific state for the ROLE with specific attributes, you will likely want to separate these out into two separate statements: CREATE ROLE IF NOT EXISTS my_role; ALTER ROLE my_role WITH INHERIT SUPERUSER CREATEDB; Instead of: CREATE ROLE IF NOT EXISTS my_role WITH INHERIT SUPERUSER CREATEDB; While the second form will DWYM in cases where the role does not exist, it will not change the existing object to reflect the role permissions passed in, and will instead keep the existing permissions. This is a conscious choice at this point. What this *does* mean is that you can avoid issues caused by needing to (e.g.) DROP and immediately CREATE a user/role when the intent is just "make sure this role exists". --- doc/src/sgml/ref/create_group.sgml | 2 +- doc/src/sgml/ref/create_role.sgml | 13 +++++- doc/src/sgml/ref/create_user.sgml | 2 +- src/backend/commands/user.c | 23 ++++++++-- src/backend/nodes/copyfuncs.c | 1 + src/backend/nodes/equalfuncs.c | 1 + src/backend/parser/gram.y | 36 +++++++++++++-- src/include/nodes/parsenodes.h | 1 + src/test/regress/expected/roleattributes.out | 48 ++++++++++++++++++++ src/test/regress/sql/roleattributes.sql | 28 ++++++++++++ 10 files changed, 144 insertions(+), 11 deletions(-) diff --git a/doc/src/sgml/ref/create_group.sgml b/doc/src/sgml/ref/create_group.sgml index d124c98eb5..299873bb71 100644 --- a/doc/src/sgml/ref/create_group.sgml +++ b/doc/src/sgml/ref/create_group.sgml @@ -21,7 +21,7 @@ PostgreSQL documentation -CREATE GROUP name [ [ WITH ] option [ ... ] ] +CREATE GROUP [ IF NOT EXISTS ] name [ [ WITH ] option [ ... ] ] where option can be: diff --git a/doc/src/sgml/ref/create_role.sgml b/doc/src/sgml/ref/create_role.sgml index b6a4ea1f72..a21e2d6fe6 100644 --- a/doc/src/sgml/ref/create_role.sgml +++ b/doc/src/sgml/ref/create_role.sgml @@ -21,7 +21,7 @@ PostgreSQL documentation -CREATE ROLE name [ [ WITH ] option [ ... ] ] +CREATE ROLE [ IF NOT EXISTS ] name [ [ WITH ] option [ ... ] ] where option can be: @@ -74,6 +74,17 @@ in sync when changing the above synopsis! Parameters + + IF NOT EXISTS + + + Do not throw an error if the role with the same name already exists. A notice + is issued in this case. Note that there is no guarantee that the existing role + is anything like the one that would have been created. + + + + name diff --git a/doc/src/sgml/ref/create_user.sgml b/doc/src/sgml/ref/create_user.sgml index 48d2089238..9749db8233 100644 --- a/doc/src/sgml/ref/create_user.sgml +++ b/doc/src/sgml/ref/create_user.sgml @@ -21,7 +21,7 @@ PostgreSQL documentation -CREATE USER name [ [ WITH ] option [ ... ] ] +CREATE USER [ IF NOT EXISTS ] name [ [ WITH ] option [ ... ] ] where option can be: diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c index aa69821be4..044fc8107d 100644 --- a/src/backend/commands/user.c +++ b/src/backend/commands/user.c @@ -304,11 +304,24 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt) pg_authid_rel = table_open(AuthIdRelationId, RowExclusiveLock); pg_authid_dsc = RelationGetDescr(pg_authid_rel); - if (OidIsValid(get_role_oid(stmt->role, true))) - ereport(ERROR, - (errcode(ERRCODE_DUPLICATE_OBJECT), - errmsg("role \"%s\" already exists", - stmt->role))); + roleid = get_role_oid(stmt->role, true); + + if (OidIsValid(roleid)) + { + if (stmt->if_not_exists) + { + table_close(pg_authid_rel, NoLock); + ereport(NOTICE, + (errmsg("role \"%s\" already exists, skipping", + stmt->role))); + return roleid; + } + else + ereport(ERROR, + (errcode(ERRCODE_DUPLICATE_OBJECT), + errmsg("role \"%s\" already exists", + stmt->role))); + } /* Convert validuntil to internal form */ if (validUntil) diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index 70e9e54d3e..71cc4bcf79 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -4515,6 +4515,7 @@ _copyCreateRoleStmt(const CreateRoleStmt *from) COPY_SCALAR_FIELD(stmt_type); COPY_STRING_FIELD(role); COPY_NODE_FIELD(options); + COPY_SCALAR_FIELD(if_not_exists); return newnode; } diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index 19eff20102..7e3331e0b5 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -2129,6 +2129,7 @@ _equalCreateRoleStmt(const CreateRoleStmt *a, const CreateRoleStmt *b) COMPARE_SCALAR_FIELD(stmt_type); COMPARE_STRING_FIELD(role); COMPARE_NODE_FIELD(options); + COMPARE_SCALAR_FIELD(if_not_exists); return true; } diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 08f1bf1031..7292b251ae 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -1048,12 +1048,22 @@ CallStmt: CALL func_application *****************************************************************************/ CreateRoleStmt: - CREATE ROLE RoleId opt_with OptRoleList + CREATE ROLE IF_P NOT EXISTS RoleId opt_with OptRoleList + { + CreateRoleStmt *n = makeNode(CreateRoleStmt); + n->stmt_type = ROLESTMT_ROLE; + n->role = $6; + n->options = $8; + n->if_not_exists = true; + $$ = (Node *)n; + } + | CREATE ROLE RoleId opt_with OptRoleList { CreateRoleStmt *n = makeNode(CreateRoleStmt); n->stmt_type = ROLESTMT_ROLE; n->role = $3; n->options = $5; + n->if_not_exists = false; $$ = (Node *)n; } ; @@ -1204,12 +1214,22 @@ CreateOptRoleElem: *****************************************************************************/ CreateUserStmt: - CREATE USER RoleId opt_with OptRoleList + CREATE USER IF_P NOT EXISTS RoleId opt_with OptRoleList + { + CreateRoleStmt *n = makeNode(CreateRoleStmt); + n->stmt_type = ROLESTMT_USER; + n->role = $6; + n->options = $8; + n->if_not_exists = true; + $$ = (Node *)n; + } + | CREATE USER RoleId opt_with OptRoleList { CreateRoleStmt *n = makeNode(CreateRoleStmt); n->stmt_type = ROLESTMT_USER; n->role = $3; n->options = $5; + n->if_not_exists = false; $$ = (Node *)n; } ; @@ -1343,12 +1363,22 @@ DropRoleStmt: *****************************************************************************/ CreateGroupStmt: - CREATE GROUP_P RoleId opt_with OptRoleList + CREATE GROUP_P IF_P NOT EXISTS RoleId opt_with OptRoleList + { + CreateRoleStmt *n = makeNode(CreateRoleStmt); + n->stmt_type = ROLESTMT_GROUP; + n->role = $6; + n->options = $8; + n->if_not_exists = true; + $$ = (Node *)n; + } + | CREATE GROUP_P RoleId opt_with OptRoleList { CreateRoleStmt *n = makeNode(CreateRoleStmt); n->stmt_type = ROLESTMT_GROUP; n->role = $3; n->options = $5; + n->if_not_exists = false; $$ = (Node *)n; } ; diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 3138877553..509a996a78 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -2621,6 +2621,7 @@ typedef struct CreateRoleStmt RoleStmtType stmt_type; /* ROLE/USER/GROUP */ char *role; /* role name */ List *options; /* List of DefElem nodes */ + bool if_not_exists; /* Skip role creation if exists */ } CreateRoleStmt; typedef struct AlterRoleStmt diff --git a/src/test/regress/expected/roleattributes.out b/src/test/regress/expected/roleattributes.out index 5e6969b173..d8d6194414 100644 --- a/src/test/regress/expected/roleattributes.out +++ b/src/test/regress/expected/roleattributes.out @@ -247,3 +247,51 @@ DROP ROLE regress_test_def_replication; DROP ROLE regress_test_replication; DROP ROLE regress_test_def_bypassrls; DROP ROLE regress_test_bypassrls; +-- CREATE ROLE IF NOT EXISTS +CREATE ROLE regress_test_exists_role; +CREATE ROLE regress_test_exists_role; +ERROR: role "regress_test_exists_role" already exists +CREATE ROLE IF NOT EXISTS regress_test_exists_role; +DROP ROLE regress_test_exists_role; +CREATE ROLE regress_test_exists_role WITH NOINHERIT; +CREATE ROLE IF NOT EXISTS regress_test_exists_role WITH INHERIT; +SELECT rolname, rolinherit FROM pg_authid WHERE rolname = 'regress_test_exists_role'; + rolname | rolinherit +--------------------------+------------ + regress_test_exists_role | f +(1 row) + +ALTER ROLE regress_test_exists_role WITH INHERIT; +SELECT rolname, rolinherit FROM pg_authid WHERE rolname = 'regress_test_exists_role'; + rolname | rolinherit +--------------------------+------------ + regress_test_exists_role | t +(1 row) + +DROP ROLE regress_test_exists_role; +CREATE USER regress_test_exists_user; +CREATE USER regress_test_exists_user; +ERROR: role "regress_test_exists_user" already exists +CREATE USER IF NOT EXISTS regress_test_exists_user; +DROP USER regress_test_exists_user; +CREATE USER regress_test_exists_user WITH NOINHERIT; +CREATE USER IF NOT EXISTS regress_test_exists_user WITH INHERIT; +SELECT rolname, rolinherit FROM pg_authid WHERE rolname = 'regress_test_exists_user'; + rolname | rolinherit +--------------------------+------------ + regress_test_exists_user | f +(1 row) + +ALTER USER regress_test_exists_user WITH INHERIT; +SELECT rolname, rolinherit FROM pg_authid WHERE rolname = 'regress_test_exists_user'; + rolname | rolinherit +--------------------------+------------ + regress_test_exists_user | t +(1 row) + +DROP USER regress_test_exists_user; +CREATE GROUP regress_test_exists_group; +CREATE GROUP regress_test_exists_group; +ERROR: role "regress_test_exists_group" already exists +CREATE GROUP IF NOT EXISTS regress_test_exists_group; +DROP GROUP regress_test_exists_group; diff --git a/src/test/regress/sql/roleattributes.sql b/src/test/regress/sql/roleattributes.sql index c961b2d730..b6f79c25cd 100644 --- a/src/test/regress/sql/roleattributes.sql +++ b/src/test/regress/sql/roleattributes.sql @@ -96,3 +96,31 @@ DROP ROLE regress_test_def_replication; DROP ROLE regress_test_replication; DROP ROLE regress_test_def_bypassrls; DROP ROLE regress_test_bypassrls; + +-- CREATE ROLE IF NOT EXISTS +CREATE ROLE regress_test_exists_role; +CREATE ROLE regress_test_exists_role; +CREATE ROLE IF NOT EXISTS regress_test_exists_role; +DROP ROLE regress_test_exists_role; +CREATE ROLE regress_test_exists_role WITH NOINHERIT; +CREATE ROLE IF NOT EXISTS regress_test_exists_role WITH INHERIT; +SELECT rolname, rolinherit FROM pg_authid WHERE rolname = 'regress_test_exists_role'; +ALTER ROLE regress_test_exists_role WITH INHERIT; +SELECT rolname, rolinherit FROM pg_authid WHERE rolname = 'regress_test_exists_role'; +DROP ROLE regress_test_exists_role; + +CREATE USER regress_test_exists_user; +CREATE USER regress_test_exists_user; +CREATE USER IF NOT EXISTS regress_test_exists_user; +DROP USER regress_test_exists_user; +CREATE USER regress_test_exists_user WITH NOINHERIT; +CREATE USER IF NOT EXISTS regress_test_exists_user WITH INHERIT; +SELECT rolname, rolinherit FROM pg_authid WHERE rolname = 'regress_test_exists_user'; +ALTER USER regress_test_exists_user WITH INHERIT; +SELECT rolname, rolinherit FROM pg_authid WHERE rolname = 'regress_test_exists_user'; +DROP USER regress_test_exists_user; + +CREATE GROUP regress_test_exists_group; +CREATE GROUP regress_test_exists_group; +CREATE GROUP IF NOT EXISTS regress_test_exists_group; +DROP GROUP regress_test_exists_group; -- 2.30.1 (Apple Git-130)