From 6bff0ad924f641d432a486ad3ee190b8981b6c64 Mon Sep 17 00:00:00 2001 From: gurkanindibay Date: Thu, 1 Feb 2024 17:14:17 +0300 Subject: [PATCH] Fixes review issues --- .../distributed/commands/utility_hook.c | 23 +++++++++++-------- .../expected/grant_role_from_non_maindb.out | 4 ++-- .../sql/grant_role_from_non_maindb.sql | 6 ++--- 3 files changed, 17 insertions(+), 16 deletions(-) diff --git a/src/backend/distributed/commands/utility_hook.c b/src/backend/distributed/commands/utility_hook.c index e70afb69b..6e2f68748 100644 --- a/src/backend/distributed/commands/utility_hook.c +++ b/src/backend/distributed/commands/utility_hook.c @@ -100,8 +100,8 @@ * supported from non-main databases and whether it should be marked as * distributed explicitly (*). * - * We always have to mark such the objects created "as distributed" but while for - * some object types we can delegate this to main database, for some others we have + * (*) We always have to mark such objects as "distributed" but while for some + * object types we can delegate this to main database, for some others we have * to explicitly send a command to all nodes in this code-path to achieve this. */ typedef struct NonMainDbDistributedStatementInfo @@ -157,8 +157,8 @@ static void RunPreprocessMainDBCommand(Node *parsetree, const char *queryString) static void RunPostprocessMainDBCommand(Node *parsetree); static bool IsStatementSupportedInNonMainDb(Node *parsetree); static bool StatementRequiresMarkDistributedFromNonMainDb(Node *parsetree); -static ObjectInfo GetObjectInfo(Node *parsetree); static void MarkObjectDistributedInNonMainDb(Node *parsetree); +static ObjectInfo GetObjectInfo(Node *parsetree); /* * ProcessUtilityParseTree is a convenience method to create a PlannedStmt out of @@ -1673,23 +1673,26 @@ RunPostprocessMainDBCommand(Node *parsetree) /* - * GetObjectInfo returns the name and oid of the object in the given parsetree. + * GetObjectInfo returns ObjectInfo for the target object of given parsetree. */ static ObjectInfo GetObjectInfo(Node *parsetree) { - ObjectInfo info; if (IsA(parsetree, CreateRoleStmt)) { CreateRoleStmt *stmt = castNode(CreateRoleStmt, parsetree); - info.name = stmt->role; - info.id = get_role_oid(stmt->role, false); + ObjectInfo info = { + .name = stmt->role, + .id = get_role_oid(stmt->role, false) + }; + + return info; } /* Add else if branches for other statement types */ - return info; + elog(ERROR, "unsupported statement type"); } @@ -1713,7 +1716,7 @@ MarkObjectDistributedInNonMainDb(Node *parsetree) /* - * IsStatementSupportedIn2Pc returns true if the statement is supported from a + * IsStatementSupportedInNonMainDb returns true if the statement is supported from a * non-main database. */ static bool @@ -1735,7 +1738,7 @@ IsStatementSupportedInNonMainDb(Node *parsetree) /* - * DoesStatementRequireMarkDistributedFor2PC returns true if the statement should be marked + * StatementRequiresMarkDistributedFromNonMainDb returns true if the statement should be marked * as distributed when executed from a non-main database. */ static bool diff --git a/src/test/regress/expected/grant_role_from_non_maindb.out b/src/test/regress/expected/grant_role_from_non_maindb.out index fa88e4e2a..6dc0b6c60 100644 --- a/src/test/regress/expected/grant_role_from_non_maindb.out +++ b/src/test/regress/expected/grant_role_from_non_maindb.out @@ -127,12 +127,12 @@ $$); [{"member":"grant_role2pc_user2","role":"grant_role2pc_user1","grantor":"postgres","admin_option":true},{"member":"grant_role2pc_user3","role":"grant_role2pc_user2","grantor":"postgres","admin_option":true},{"member":"grant_role2pc_user5","role":"grant_role2pc_user2","grantor":"grant_role2pc_user3","admin_option":false},{"member":"grant_role2pc_user6","role":"grant_role2pc_user2","grantor":"grant_role2pc_user3","admin_option":false},{"member":"grant_role2pc_user7","role":"grant_role2pc_user2","grantor":"grant_role2pc_user3","admin_option":false}] (3 rows) -\c - - - :worker_1_port +\c grant_role2pc_db - - :worker_1_port BEGIN; grant grant_role2pc_user1 to grant_role2pc_user5 WITH ADMIN OPTION; grant grant_role2pc_user1 to grant_role2pc_user6; COMMIT; -\c - - - :master_port +\c regression - - :master_port select result FROM run_command_on_all_nodes($$ SELECT array_to_json(array_agg(row_to_json(t))) FROM ( diff --git a/src/test/regress/sql/grant_role_from_non_maindb.sql b/src/test/regress/sql/grant_role_from_non_maindb.sql index 5e698b6f4..b74b5092d 100644 --- a/src/test/regress/sql/grant_role_from_non_maindb.sql +++ b/src/test/regress/sql/grant_role_from_non_maindb.sql @@ -1,5 +1,3 @@ - - CREATE SCHEMA grant_role2pc; SET search_path TO grant_role2pc; set citus.enable_create_database_propagation to on; @@ -119,13 +117,13 @@ FROM ( ) t $$); -\c - - - :worker_1_port +\c grant_role2pc_db - - :worker_1_port BEGIN; grant grant_role2pc_user1 to grant_role2pc_user5 WITH ADMIN OPTION; grant grant_role2pc_user1 to grant_role2pc_user6; COMMIT; -\c - - - :master_port +\c regression - - :master_port select result FROM run_command_on_all_nodes($$ SELECT array_to_json(array_agg(row_to_json(t)))