From 37e23f44b4d2b45312f5da6a9c880a534d7331e9 Mon Sep 17 00:00:00 2001 From: ThomasC02 <147439284+ThomasC02@users.noreply.github.com> Date: Fri, 25 Apr 2025 18:13:41 -0400 Subject: [PATCH] Add Support for CASCADE/RESTRICT in REVOKE statements (#7958) Fixes #7105. DESCRIPTION: Fixes a bug that causes omitting CASCADE clause for the commands sent to workers for REVOKE commands on tables. --------- Co-authored-by: ThomasC02 Co-authored-by: Onur Tirtir Co-authored-by: Tiago Silva --- src/backend/distributed/commands/grant.c | 9 ++++ .../expected/grant_on_table_propagation.out | 49 +++++++++++++++++-- .../expected/grant_on_table_propagation_0.out | 49 +++++++++++++++++-- src/test/regress/expected/pg17.out | 6 +-- .../sql/grant_on_table_propagation.sql | 14 ++++-- 5 files changed, 115 insertions(+), 12 deletions(-) diff --git a/src/backend/distributed/commands/grant.c b/src/backend/distributed/commands/grant.c index 3cc18c155..c60afa197 100644 --- a/src/backend/distributed/commands/grant.c +++ b/src/backend/distributed/commands/grant.c @@ -182,6 +182,15 @@ PreprocessGrantStmt(Node *node, const char *queryString, appendStringInfo(&ddlString, "REVOKE %s%s ON %s FROM %s", grantOption, privsString.data, targetString.data, granteesString.data); + + if (grantStmt->behavior == DROP_CASCADE) + { + appendStringInfoString(&ddlString, " CASCADE"); + } + else + { + appendStringInfoString(&ddlString, " RESTRICT"); + } } DDLJob *ddlJob = palloc0(sizeof(DDLJob)); diff --git a/src/test/regress/expected/grant_on_table_propagation.out b/src/test/regress/expected/grant_on_table_propagation.out index 55a55fac3..4bceb19b8 100644 --- a/src/test/regress/expected/grant_on_table_propagation.out +++ b/src/test/regress/expected/grant_on_table_propagation.out @@ -862,6 +862,7 @@ SELECT citus_remove_node('localhost', :worker_2_port); (1 row) +SET citus.next_shard_id to 2000000; CREATE TABLE grant_table_propagated_after (id int primary key); SET citus.shard_replication_factor TO 1; SELECT create_distributed_table('grant_table_propagated_after', 'id'); @@ -902,8 +903,50 @@ SELECT id FROM grant_table_propagated_after; 57638 | (grant_on_table.grant_table_propagated_after,id,{grant_user_0=r/postgres}) (3 rows) --- cleanup -REVOKE SELECT (id) ON grant_table_propagated_after FROM grant_user_0; +-- cleanup and test revoke .. cascade/restrict +SET citus.log_remote_commands TO on; +set citus.grep_remote_commands = '%REVOKE%'; +REVOKE SELECT (id) ON grant_table_propagated_after FROM grant_user_0 CASCADE; +NOTICE: issuing REVOKE select (id ) ON grant_table_propagated_after FROM grant_user_0 CASCADE +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing REVOKE select (id ) ON grant_table_propagated_after FROM grant_user_0 CASCADE +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing SELECT worker_apply_shard_ddl_command (2000000, 'grant_on_table', 'REVOKE select (id ) ON grant_table_propagated_after FROM grant_user_0 CASCADE') +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing SELECT worker_apply_shard_ddl_command (2000001, 'grant_on_table', 'REVOKE select (id ) ON grant_table_propagated_after FROM grant_user_0 CASCADE') +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing SELECT worker_apply_shard_ddl_command (2000002, 'grant_on_table', 'REVOKE select (id ) ON grant_table_propagated_after FROM grant_user_0 CASCADE') +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing SELECT worker_apply_shard_ddl_command (2000003, 'grant_on_table', 'REVOKE select (id ) ON grant_table_propagated_after FROM grant_user_0 CASCADE') +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +REVOKE SELECT (id) ON grant_table_propagated_after FROM grant_user_0 RESTRICT; +NOTICE: issuing REVOKE select (id ) ON grant_table_propagated_after FROM grant_user_0 RESTRICT +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing REVOKE select (id ) ON grant_table_propagated_after FROM grant_user_0 RESTRICT +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing SELECT worker_apply_shard_ddl_command (2000000, 'grant_on_table', 'REVOKE select (id ) ON grant_table_propagated_after FROM grant_user_0 RESTRICT') +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing SELECT worker_apply_shard_ddl_command (2000001, 'grant_on_table', 'REVOKE select (id ) ON grant_table_propagated_after FROM grant_user_0 RESTRICT') +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing SELECT worker_apply_shard_ddl_command (2000002, 'grant_on_table', 'REVOKE select (id ) ON grant_table_propagated_after FROM grant_user_0 RESTRICT') +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing SELECT worker_apply_shard_ddl_command (2000003, 'grant_on_table', 'REVOKE select (id ) ON grant_table_propagated_after FROM grant_user_0 RESTRICT') +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +REVOKE SELECT (id) ON grant_table_propagated_after FROM grant_user_0; -- implicit RESTRICT +NOTICE: issuing REVOKE select (id ) ON grant_table_propagated_after FROM grant_user_0 RESTRICT +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing REVOKE select (id ) ON grant_table_propagated_after FROM grant_user_0 RESTRICT +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing SELECT worker_apply_shard_ddl_command (2000000, 'grant_on_table', 'REVOKE select (id ) ON grant_table_propagated_after FROM grant_user_0 RESTRICT') +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing SELECT worker_apply_shard_ddl_command (2000001, 'grant_on_table', 'REVOKE select (id ) ON grant_table_propagated_after FROM grant_user_0 RESTRICT') +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing SELECT worker_apply_shard_ddl_command (2000002, 'grant_on_table', 'REVOKE select (id ) ON grant_table_propagated_after FROM grant_user_0 RESTRICT') +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing SELECT worker_apply_shard_ddl_command (2000003, 'grant_on_table', 'REVOKE select (id ) ON grant_table_propagated_after FROM grant_user_0 RESTRICT') +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +RESET citus.grep_remote_commands; +RESET citus.log_remote_commands; :verify_grant_table ; nodeport | unnest --------------------------------------------------------------------- @@ -925,6 +968,6 @@ RESET client_min_messages; -- prevent useless messages on DROP objects. SET client_min_messages TO ERROR; DROP SCHEMA grant_on_table CASCADE; -DROP ROLE grant_user_0, grant_user_1; +DROP ROLE grant_user_0, grant_user_1, nogrant_user; RESET client_min_messages; RESET search_path; diff --git a/src/test/regress/expected/grant_on_table_propagation_0.out b/src/test/regress/expected/grant_on_table_propagation_0.out index adebacd19..c490a3080 100644 --- a/src/test/regress/expected/grant_on_table_propagation_0.out +++ b/src/test/regress/expected/grant_on_table_propagation_0.out @@ -862,6 +862,7 @@ SELECT citus_remove_node('localhost', :worker_2_port); (1 row) +SET citus.next_shard_id to 2000000; CREATE TABLE grant_table_propagated_after (id int primary key); SET citus.shard_replication_factor TO 1; SELECT create_distributed_table('grant_table_propagated_after', 'id'); @@ -902,8 +903,50 @@ SELECT id FROM grant_table_propagated_after; 57638 | (grant_on_table.grant_table_propagated_after,id,{grant_user_0=r/postgres}) (3 rows) --- cleanup -REVOKE SELECT (id) ON grant_table_propagated_after FROM grant_user_0; +-- cleanup and test revoke .. cascade/restrict +SET citus.log_remote_commands TO on; +set citus.grep_remote_commands = '%REVOKE%'; +REVOKE SELECT (id) ON grant_table_propagated_after FROM grant_user_0 CASCADE; +NOTICE: issuing REVOKE select (id ) ON grant_table_propagated_after FROM grant_user_0 CASCADE +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing REVOKE select (id ) ON grant_table_propagated_after FROM grant_user_0 CASCADE +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing SELECT worker_apply_shard_ddl_command (2000000, 'grant_on_table', 'REVOKE select (id ) ON grant_table_propagated_after FROM grant_user_0 CASCADE') +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing SELECT worker_apply_shard_ddl_command (2000001, 'grant_on_table', 'REVOKE select (id ) ON grant_table_propagated_after FROM grant_user_0 CASCADE') +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing SELECT worker_apply_shard_ddl_command (2000002, 'grant_on_table', 'REVOKE select (id ) ON grant_table_propagated_after FROM grant_user_0 CASCADE') +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing SELECT worker_apply_shard_ddl_command (2000003, 'grant_on_table', 'REVOKE select (id ) ON grant_table_propagated_after FROM grant_user_0 CASCADE') +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +REVOKE SELECT (id) ON grant_table_propagated_after FROM grant_user_0 RESTRICT; +NOTICE: issuing REVOKE select (id ) ON grant_table_propagated_after FROM grant_user_0 RESTRICT +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing REVOKE select (id ) ON grant_table_propagated_after FROM grant_user_0 RESTRICT +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing SELECT worker_apply_shard_ddl_command (2000000, 'grant_on_table', 'REVOKE select (id ) ON grant_table_propagated_after FROM grant_user_0 RESTRICT') +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing SELECT worker_apply_shard_ddl_command (2000001, 'grant_on_table', 'REVOKE select (id ) ON grant_table_propagated_after FROM grant_user_0 RESTRICT') +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing SELECT worker_apply_shard_ddl_command (2000002, 'grant_on_table', 'REVOKE select (id ) ON grant_table_propagated_after FROM grant_user_0 RESTRICT') +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing SELECT worker_apply_shard_ddl_command (2000003, 'grant_on_table', 'REVOKE select (id ) ON grant_table_propagated_after FROM grant_user_0 RESTRICT') +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +REVOKE SELECT (id) ON grant_table_propagated_after FROM grant_user_0; -- implicit RESTRICT +NOTICE: issuing REVOKE select (id ) ON grant_table_propagated_after FROM grant_user_0 RESTRICT +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing REVOKE select (id ) ON grant_table_propagated_after FROM grant_user_0 RESTRICT +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing SELECT worker_apply_shard_ddl_command (2000000, 'grant_on_table', 'REVOKE select (id ) ON grant_table_propagated_after FROM grant_user_0 RESTRICT') +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing SELECT worker_apply_shard_ddl_command (2000001, 'grant_on_table', 'REVOKE select (id ) ON grant_table_propagated_after FROM grant_user_0 RESTRICT') +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing SELECT worker_apply_shard_ddl_command (2000002, 'grant_on_table', 'REVOKE select (id ) ON grant_table_propagated_after FROM grant_user_0 RESTRICT') +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing SELECT worker_apply_shard_ddl_command (2000003, 'grant_on_table', 'REVOKE select (id ) ON grant_table_propagated_after FROM grant_user_0 RESTRICT') +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +RESET citus.grep_remote_commands; +RESET citus.log_remote_commands; :verify_grant_table ; nodeport | unnest --------------------------------------------------------------------- @@ -925,6 +968,6 @@ RESET client_min_messages; -- prevent useless messages on DROP objects. SET client_min_messages TO ERROR; DROP SCHEMA grant_on_table CASCADE; -DROP ROLE grant_user_0, grant_user_1; +DROP ROLE grant_user_0, grant_user_1, nogrant_user; RESET client_min_messages; RESET search_path; diff --git a/src/test/regress/expected/pg17.out b/src/test/regress/expected/pg17.out index 5c0dc73c6..edda22f26 100644 --- a/src/test/regress/expected/pg17.out +++ b/src/test/regress/expected/pg17.out @@ -473,11 +473,11 @@ DETAIL: on server regress_maintain@localhost:xxxxx connectionId: xxxxxxx RESET ROLE; SET citus.grep_remote_commands = '%maintain%'; REVOKE MAINTAIN ON dist_test FROM regress_maintain; -NOTICE: issuing REVOKE maintain ON dist_test FROM regress_maintain +NOTICE: issuing REVOKE maintain ON dist_test FROM regress_maintain RESTRICT DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx -NOTICE: issuing REVOKE maintain ON dist_test FROM regress_maintain +NOTICE: issuing REVOKE maintain ON dist_test FROM regress_maintain RESTRICT DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx -NOTICE: issuing SELECT worker_apply_shard_ddl_command (20240023, 'pg17', 'REVOKE maintain ON dist_test FROM regress_maintain') +NOTICE: issuing SELECT worker_apply_shard_ddl_command (20240023, 'pg17', 'REVOKE maintain ON dist_test FROM regress_maintain RESTRICT') DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx RESET citus.grep_remote_commands; SET ROLE regress_maintain; diff --git a/src/test/regress/sql/grant_on_table_propagation.sql b/src/test/regress/sql/grant_on_table_propagation.sql index 754870c83..42d77b4de 100644 --- a/src/test/regress/sql/grant_on_table_propagation.sql +++ b/src/test/regress/sql/grant_on_table_propagation.sql @@ -405,6 +405,7 @@ RESET client_min_messages; -- similar test but adding a node after the fact -- remove one of the worker nodes: SELECT citus_remove_node('localhost', :worker_2_port); +SET citus.next_shard_id to 2000000; CREATE TABLE grant_table_propagated_after (id int primary key); SET citus.shard_replication_factor TO 1; SELECT create_distributed_table('grant_table_propagated_after', 'id'); @@ -424,8 +425,15 @@ SELECT id FROM grant_table_propagated_after; :verify_grant_table ; :verify_grant_attributes ; --- cleanup -REVOKE SELECT (id) ON grant_table_propagated_after FROM grant_user_0; +-- cleanup and test revoke .. cascade/restrict +SET citus.log_remote_commands TO on; +set citus.grep_remote_commands = '%REVOKE%'; +REVOKE SELECT (id) ON grant_table_propagated_after FROM grant_user_0 CASCADE; +REVOKE SELECT (id) ON grant_table_propagated_after FROM grant_user_0 RESTRICT; +REVOKE SELECT (id) ON grant_table_propagated_after FROM grant_user_0; -- implicit RESTRICT + +RESET citus.grep_remote_commands; +RESET citus.log_remote_commands; :verify_grant_table ; :verify_grant_attributes ; @@ -439,6 +447,6 @@ RESET client_min_messages; -- prevent useless messages on DROP objects. SET client_min_messages TO ERROR; DROP SCHEMA grant_on_table CASCADE; -DROP ROLE grant_user_0, grant_user_1; +DROP ROLE grant_user_0, grant_user_1, nogrant_user; RESET client_min_messages; RESET search_path;