From 99e9786c7feed4c60aff8ef867a8704d52e0d3dc Mon Sep 17 00:00:00 2001 From: gindibay Date: Thu, 14 Sep 2023 02:11:11 +0300 Subject: [PATCH] Adds alter role rename --- .../commands/distribute_object_ops.c | 10 ++--- src/backend/distributed/commands/role.c | 45 +++++++++++++++++++ .../distributed/commands/utility_hook.c | 12 ----- .../distributed/deparser/deparse_role_stmts.c | 16 +++++++ src/include/distributed/commands.h | 8 ++++ src/include/distributed/deparser.h | 1 + .../regress/sql/alter_role_propagation.sql | 30 +++---------- 7 files changed, 81 insertions(+), 41 deletions(-) diff --git a/src/backend/distributed/commands/distribute_object_ops.c b/src/backend/distributed/commands/distribute_object_ops.c index c0fe607ef..a0a306e8d 100644 --- a/src/backend/distributed/commands/distribute_object_ops.c +++ b/src/backend/distributed/commands/distribute_object_ops.c @@ -152,12 +152,12 @@ static DistributeObjectOps Any_AlterRole = { }; static DistributeObjectOps Any_AlterRoleRename = { - .deparse = DeparseAlterRoleStmt, + .deparse = DeparseRenameRoleStmt, .qualify = NULL, - .preprocess = NULL, - .postprocess = PostprocessAlterRoleStmt, + .preprocess = PreprocessAlterRoleRenameStmt, + .postprocess = NULL, .operationType = DIST_OPS_ALTER, - .address = AlterRoleStmtObjectAddress, + .address = RenameRoleStmtObjectAddress, .markDistributed = false, }; @@ -2072,7 +2072,7 @@ GetDistributeObjectOps(Node *node) case OBJECT_ROLE: { - return &Role_Rename; + return &Any_AlterRoleRename; } case OBJECT_ROUTINE: diff --git a/src/backend/distributed/commands/role.c b/src/backend/distributed/commands/role.c index 63ede986d..4d7fcf4b3 100644 --- a/src/backend/distributed/commands/role.c +++ b/src/backend/distributed/commands/role.c @@ -1306,3 +1306,48 @@ EnsureSequentialModeForRoleDDL(void) "use only one connection for all future commands"))); SetLocalMultiShardModifyModeToSequential(); } + +/* + * PreprocessAlterDatabaseSetStmt is executed before the statement is applied to the local + * postgres instance. + * + * In this stage we can prepare the commands that need to be run on all workers to grant + * on databases. + */ +List * +PreprocessAlterRoleRenameStmt(Node *node, const char *queryString, + ProcessUtilityContext processUtilityContext) +{ + if (!ShouldPropagate()) + { + return NIL; + } + + RenameStmt *stmt = castNode(RenameStmt, node); + Assert(stmt->renameType == OBJECT_ROLE); + + + EnsureCoordinator(); + + char *sql = DeparseTreeNode((Node *) stmt); + + List *commands = list_make3(DISABLE_DDL_PROPAGATION, + (void *) sql, + ENABLE_DDL_PROPAGATION); + + return NodeDDLTaskList(NON_COORDINATOR_NODES, commands); +} + + +List * +RenameRoleStmtObjectAddress(Node *node, bool missing_ok, bool isPostprocess) +{ + RenameStmt *stmt = castNode(RenameStmt, node); + Assert(stmt->renameType == OBJECT_ROLE); + + Oid roleOid = get_role_oid(stmt->subname, missing_ok); + ObjectAddress *address = palloc0(sizeof(ObjectAddress)); + ObjectAddressSet(*address, AuthIdRelationId, roleOid); + + return list_make1(address); +} diff --git a/src/backend/distributed/commands/utility_hook.c b/src/backend/distributed/commands/utility_hook.c index 10e424623..900f36d10 100644 --- a/src/backend/distributed/commands/utility_hook.c +++ b/src/backend/distributed/commands/utility_hook.c @@ -818,18 +818,6 @@ ProcessUtilityInternal(PlannedStmt *pstmt, } } - if (IsA(parsetree, RenameStmt) && ((RenameStmt *) parsetree)->renameType == - OBJECT_ROLE && EnableAlterRolePropagation) - { - if (EnableUnsupportedFeatureMessages) - { - ereport(NOTICE, (errmsg( - "not propagating ALTER ROLE ... RENAME TO commands " - "to worker nodes"), - errhint("Connect to worker nodes directly to manually " - "rename the role"))); - } - } } if (IsA(parsetree, CreateStmt)) diff --git a/src/backend/distributed/deparser/deparse_role_stmts.c b/src/backend/distributed/deparser/deparse_role_stmts.c index 8cdc6ef9e..5243c2fe6 100644 --- a/src/backend/distributed/deparser/deparse_role_stmts.c +++ b/src/backend/distributed/deparser/deparse_role_stmts.c @@ -341,6 +341,22 @@ AppendRoleList(StringInfo buf, List *roleList) } +char * +DeparseRenameRoleStmt(Node *node) +{ + RenameStmt *stmt = castNode(RenameStmt, node); + StringInfoData str = { 0 }; + initStringInfo(&str); + + Assert(stmt->renameType == OBJECT_ROLE); + + appendStringInfo(&str, "ALTER ROLE %s RENAME TO %s;", + quote_identifier(stmt->subname), quote_identifier(stmt->newname)); + + return str.data; +} + + /* * DeparseGrantRoleStmt builds and returns a string representing of the * GrantRoleStmt for application on a remote server. diff --git a/src/include/distributed/commands.h b/src/include/distributed/commands.h index 429016f9f..d329ba79d 100644 --- a/src/include/distributed/commands.h +++ b/src/include/distributed/commands.h @@ -479,6 +479,10 @@ extern List * PreprocessRenameAttributeStmt(Node *stmt, const char *queryString, extern List * PostprocessAlterRoleStmt(Node *stmt, const char *queryString); extern List * PreprocessAlterRoleSetStmt(Node *stmt, const char *queryString, ProcessUtilityContext processUtilityContext); + +extern List * PreprocessAlterRoleRenameStmt(Node *stmt, const char *queryString, + ProcessUtilityContext processUtilityContext); + extern List * GenerateAlterRoleSetCommandForRole(Oid roleid); extern List * AlterRoleStmtObjectAddress(Node *node, bool missing_ok, bool isPostprocess); @@ -494,6 +498,10 @@ extern List * PostprocessGrantRoleStmt(Node *stmt, const char *queryString); extern List * GenerateCreateOrAlterRoleCommand(Oid roleOid); extern List * CreateRoleStmtObjectAddress(Node *stmt, bool missing_ok, bool isPostprocess); + +extern List * RenameRoleStmtObjectAddress(Node *stmt, bool missing_ok, bool + isPostprocess); + extern void UnmarkRolesDistributed(List *roles); extern List * FilterDistributedRoles(List *roles); diff --git a/src/include/distributed/deparser.h b/src/include/distributed/deparser.h index 6ffe33cc2..95d948bc9 100644 --- a/src/include/distributed/deparser.h +++ b/src/include/distributed/deparser.h @@ -201,6 +201,7 @@ extern void QualifyAlterFunctionDependsStmt(Node *stmt); /* forward declarations for deparse_role_stmts.c */ extern char * DeparseAlterRoleStmt(Node *stmt); extern char * DeparseAlterRoleSetStmt(Node *stmt); +extern char * DeparseRenameRoleStmt(Node *stmt); extern List * MakeSetStatementArguments(char *configurationName, char *configurationValue); diff --git a/src/test/regress/sql/alter_role_propagation.sql b/src/test/regress/sql/alter_role_propagation.sql index 044f93d62..198b8cae9 100644 --- a/src/test/regress/sql/alter_role_propagation.sql +++ b/src/test/regress/sql/alter_role_propagation.sql @@ -79,6 +79,7 @@ SELECT run_command_on_workers('SHOW enable_hashagg'); -- check that ALTER ROLE SET is not propagated when scoped to a different database -- also test case sensitivity CREATE DATABASE "REGRESSION"; + ALTER ROLE CURRENT_USER IN DATABASE "REGRESSION" SET public.myguc TO "Hello from coordinator only"; SELECT d.datname, r.setconfig FROM pg_db_role_setting r LEFT JOIN pg_database d ON r.setdatabase=d.oid WHERE r.setconfig::text LIKE '%Hello from coordinator only%'; SELECT run_command_on_workers($$SELECT json_agg((d.datname, r.setconfig)) FROM pg_db_role_setting r LEFT JOIN pg_database d ON r.setdatabase=d.oid WHERE r.setconfig::text LIKE '%Hello from coordinator only%'$$); @@ -133,40 +134,21 @@ SELECT run_command_on_workers($$SELECT row(rolname, rolsuper, rolinherit, rolcr alter user test1 with password NULL superuser inherit createrole createdb login replication bypassrls connection limit 10 valid until '2019-01-01'; SELECT run_command_on_workers($$SELECT row(rolname, rolsuper, rolinherit, rolcreaterole, rolcreatedb, rolcanlogin, rolreplication, rolbypassrls, rolconnlimit, EXTRACT (year FROM rolvaliduntil)) FROM pg_authid WHERE rolname = 'test1'$$); - +SET citus.enable_alter_role_set_propagation TO on; SET citus.log_remote_commands = true; --- Set a custom value for the search_path parameter -ALTER USER test1 SET search_path TO public, schema2; +set citus.grep_remote_commands = '%ALTER ROLE%'; --- Reset the search_path parameter to its default value -ALTER USER test1 SET search_path TO DEFAULT; - --- Set a custom value for the timezone parameter ALTER USER test1 SET timezone TO 'America/New_York'; - --- Reset the timezone parameter to its default value -ALTER USER test1 SET timezone TO DEFAULT; - --- Set a custom value for the work_mem parameter ALTER USER test1 SET work_mem TO '64MB'; - --- Reset the work_mem parameter to its default value -ALTER USER test1 SET work_mem TO DEFAULT; - --- Set a custom value for the max_connections parameter -ALTER USER test1 SET max_connections TO 100; - --- Reset the max_connections parameter to its default value -ALTER USER test1 SET max_connections TO DEFAULT; - --- Set a custom float value for the random_page_cost parameter ALTER USER test1 SET random_page_cost TO 1.5; + + alter user test1 rename to test2; drop user if exists test2; -drop user test1; +SET citus.log_remote_commands = false; DROP TABLE test_search_path; DROP SCHEMA alter_role, ",CitUs,.TeeN!?", test_sp CASCADE;