From f3427932ed3e82bbacdda21805cbb08e7dae75cd Mon Sep 17 00:00:00 2001 From: gindibay Date: Wed, 22 Nov 2023 12:13:35 +0300 Subject: [PATCH] Fixes review notes --- .../commands/distribute_object_ops.c | 4 +- src/backend/distributed/commands/owned.c | 20 +++++++- src/include/distributed/commands.h | 3 +- ...{reassure_owned.out => reassign_owned.out} | 50 +++++++++++-------- src/test/regress/multi_1_schedule | 2 +- ...{reassure_owned.sql => reassign_owned.sql} | 27 ++++++---- 6 files changed, 70 insertions(+), 36 deletions(-) rename src/test/regress/expected/{reassure_owned.out => reassign_owned.out} (58%) rename src/test/regress/sql/{reassure_owned.sql => reassign_owned.sql} (69%) diff --git a/src/backend/distributed/commands/distribute_object_ops.c b/src/backend/distributed/commands/distribute_object_ops.c index 901762ebe..4a26f5b4f 100644 --- a/src/backend/distributed/commands/distribute_object_ops.c +++ b/src/backend/distributed/commands/distribute_object_ops.c @@ -278,8 +278,8 @@ static DistributeObjectOps Any_CreateRole = { static DistributeObjectOps Any_ReassignOwned = { .deparse = DeparseReassignOwnedStmt, .qualify = NULL, - .preprocess = PreprocessReassignOwnedStmt, - .postprocess = NULL, + .preprocess = NULL, + .postprocess = PostprocessReassignOwnedStmt, .operationType = DIST_OPS_DROP, .address = NULL, .markDistributed = false, diff --git a/src/backend/distributed/commands/owned.c b/src/backend/distributed/commands/owned.c index ba90cf8fa..216e3e4a1 100644 --- a/src/backend/distributed/commands/owned.c +++ b/src/backend/distributed/commands/owned.c @@ -90,9 +90,25 @@ PreprocessDropOwnedStmt(Node *node, const char *queryString, } +/* + * Post-processes a REASSIGN OWNED statement. + * + * This function takes a Node pointer representing a REASSIGN OWNED statement, + * and performs any necessary post-processing after the statement has been executed. + * In this method, we propagate all the roles that are being reassigned. + * Therefore, we don't filter the roles based on whether they are distributed or not. + * If roles are not distributed and those roles are not present in the nodes + * where the statement is being propagated, then the statement will fail. + * + * Parameters: + * stmt: A pointer to a Node representing a REASSIGN OWNED statement. + * queryString: The original SQL command string from the client. + * + * Returns: + * List of SQL statements to be executed. + */ List * -PreprocessReassignOwnedStmt(Node *node, const char *queryString, - ProcessUtilityContext processUtilityContext) +PostprocessReassignOwnedStmt(Node *node, const char *queryString) { ReassignOwnedStmt *stmt = castNode(ReassignOwnedStmt, node); List *allReassignRoles = stmt->roles; diff --git a/src/include/distributed/commands.h b/src/include/distributed/commands.h index 02eadacc5..eeaa4870e 100644 --- a/src/include/distributed/commands.h +++ b/src/include/distributed/commands.h @@ -427,8 +427,7 @@ extern List * CreateExtensionStmtObjectAddress(Node *stmt, bool missing_ok, bool /* owned.c - forward declarations */ extern List * PreprocessDropOwnedStmt(Node *node, const char *queryString, ProcessUtilityContext processUtilityContext); -extern List * PreprocessReassignOwnedStmt(Node *node, const char *queryString, - ProcessUtilityContext processUtilityContext); +extern List * PostprocessReassignOwnedStmt(Node *node, const char *queryString); /* policy.c - forward declarations */ extern List * CreatePolicyCommands(Oid relationId); diff --git a/src/test/regress/expected/reassure_owned.out b/src/test/regress/expected/reassign_owned.out similarity index 58% rename from src/test/regress/expected/reassure_owned.out rename to src/test/regress/expected/reassign_owned.out index 580bfdd30..6a0fcf68b 100644 --- a/src/test/regress/expected/reassure_owned.out +++ b/src/test/regress/expected/reassign_owned.out @@ -1,8 +1,11 @@ CREATE ROLE role1; -CREATE ROLE role2; -GRANT CREATE ON SCHEMA public TO role1; +create ROLE role2; +CREATE ROLE role3; +GRANT CREATE ON SCHEMA public TO role1,role2; SET ROLE role1; CREATE TABLE public.test_table (col1 int); +set role role2; +CREATE TABLE public.test_table2 (col2 int); RESET ROLE; select create_distributed_table('test_table', 'col1'); create_distributed_table @@ -10,6 +13,12 @@ select create_distributed_table('test_table', 'col1'); (1 row) +select create_distributed_table('test_table2', 'col2'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + SELECT result from run_command_on_all_nodes( $$ SELECT jsonb_agg(to_jsonb(q2.*)) FROM ( @@ -20,23 +29,23 @@ SELECT result from run_command_on_all_nodes( FROM pg_tables WHERE - tablename = 'test_table' + tablename in ('test_table', 'test_table2') ) q2 $$ ) ORDER BY result; - result + result --------------------------------------------------------------------- - [{"tablename": "test_table", "schemaname": "public", "tableowner": "role1"}] - [{"tablename": "test_table", "schemaname": "public", "tableowner": "role1"}] - [{"tablename": "test_table", "schemaname": "public", "tableowner": "role1"}] + [{"tablename": "test_table", "schemaname": "public", "tableowner": "role1"}, {"tablename": "test_table2", "schemaname": "public", "tableowner": "role2"}] + [{"tablename": "test_table", "schemaname": "public", "tableowner": "role1"}, {"tablename": "test_table2", "schemaname": "public", "tableowner": "role2"}] + [{"tablename": "test_table", "schemaname": "public", "tableowner": "role1"}, {"tablename": "test_table2", "schemaname": "public", "tableowner": "role2"}] (3 rows) set citus.log_remote_commands to on; set citus.grep_remote_commands = '%REASSIGN OWNED BY%'; -REASSIGN OWNED BY role1 TO role2; -NOTICE: issuing REASSIGN OWNED BY role1 TO role2 +REASSIGN OWNED BY role1,role2 TO role3; +NOTICE: issuing REASSIGN OWNED BY role1, role2 TO role3 DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx -NOTICE: issuing REASSIGN OWNED BY role1 TO role2 +NOTICE: issuing REASSIGN OWNED BY role1, role2 TO role3 DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx SET citus.log_remote_commands = false; SELECT result from run_command_on_all_nodes( @@ -49,19 +58,20 @@ SELECT result from run_command_on_all_nodes( FROM pg_tables WHERE - tablename = 'test_table' + tablename in ('test_table', 'test_table2') ) q2 $$ ) ORDER BY result; - result + result --------------------------------------------------------------------- - [{"tablename": "test_table", "schemaname": "public", "tableowner": "role2"}] - [{"tablename": "test_table", "schemaname": "public", "tableowner": "role2"}] - [{"tablename": "test_table", "schemaname": "public", "tableowner": "role2"}] + [{"tablename": "test_table", "schemaname": "public", "tableowner": "role3"}, {"tablename": "test_table2", "schemaname": "public", "tableowner": "role3"}] + [{"tablename": "test_table", "schemaname": "public", "tableowner": "role3"}, {"tablename": "test_table2", "schemaname": "public", "tableowner": "role3"}] + [{"tablename": "test_table", "schemaname": "public", "tableowner": "role3"}, {"tablename": "test_table2", "schemaname": "public", "tableowner": "role3"}] (3 rows) +--clear resources SET citus.log_remote_commands = true; -DROP OWNED BY role1, role2; +DROP OWNED BY role1, role2,role3; SET citus.log_remote_commands = false; SELECT result from run_command_on_all_nodes( $$ @@ -73,7 +83,7 @@ SELECT result from run_command_on_all_nodes( FROM pg_tables WHERE - tablename = 'test_table' + tablename in ('test_table', 'test_table2') ) q2 $$ ) ORDER BY result; @@ -86,8 +96,8 @@ WHERE SET citus.log_remote_commands = true; set citus.grep_remote_commands = '%DROP ROLE%'; -drop role role1, role2 ; -NOTICE: issuing DROP ROLE role1, role2 +drop role role1, role2,role3 ; +NOTICE: issuing DROP ROLE role1, role2, role3 DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx -NOTICE: issuing DROP ROLE role1, role2 +NOTICE: issuing DROP ROLE role1, role2, role3 DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx diff --git a/src/test/regress/multi_1_schedule b/src/test/regress/multi_1_schedule index 0bb482856..4e5e88fa7 100644 --- a/src/test/regress/multi_1_schedule +++ b/src/test/regress/multi_1_schedule @@ -56,7 +56,7 @@ test: grant_on_database_propagation test: alter_database_propagation test: citus_shards -test: reassure_owned +test: reassign_owned # ---------- # multi_citus_tools tests utility functions written for citus tools diff --git a/src/test/regress/sql/reassure_owned.sql b/src/test/regress/sql/reassign_owned.sql similarity index 69% rename from src/test/regress/sql/reassure_owned.sql rename to src/test/regress/sql/reassign_owned.sql index a45f174da..2fdd853a1 100644 --- a/src/test/regress/sql/reassure_owned.sql +++ b/src/test/regress/sql/reassign_owned.sql @@ -1,12 +1,19 @@ CREATE ROLE role1; -CREATE ROLE role2; -GRANT CREATE ON SCHEMA public TO role1; +create ROLE role2; +CREATE ROLE role3; + +GRANT CREATE ON SCHEMA public TO role1,role2; SET ROLE role1; CREATE TABLE public.test_table (col1 int); + +set role role2; +CREATE TABLE public.test_table2 (col2 int); RESET ROLE; select create_distributed_table('test_table', 'col1'); +select create_distributed_table('test_table2', 'col2'); + SELECT result from run_command_on_all_nodes( $$ @@ -18,14 +25,14 @@ SELECT result from run_command_on_all_nodes( FROM pg_tables WHERE - tablename = 'test_table' + tablename in ('test_table', 'test_table2') ) q2 $$ ) ORDER BY result; set citus.log_remote_commands to on; set citus.grep_remote_commands = '%REASSIGN OWNED BY%'; -REASSIGN OWNED BY role1 TO role2; +REASSIGN OWNED BY role1,role2 TO role3; SET citus.log_remote_commands = false; @@ -39,15 +46,15 @@ SELECT result from run_command_on_all_nodes( FROM pg_tables WHERE - tablename = 'test_table' + tablename in ('test_table', 'test_table2') ) q2 $$ ) ORDER BY result; - +--clear resources SET citus.log_remote_commands = true; -DROP OWNED BY role1, role2; +DROP OWNED BY role1, role2,role3; SET citus.log_remote_commands = false; SELECT result from run_command_on_all_nodes( @@ -60,11 +67,13 @@ SELECT result from run_command_on_all_nodes( FROM pg_tables WHERE - tablename = 'test_table' + tablename in ('test_table', 'test_table2') ) q2 $$ ) ORDER BY result; + + SET citus.log_remote_commands = true; set citus.grep_remote_commands = '%DROP ROLE%'; -drop role role1, role2 ; +drop role role1, role2,role3 ;