Fix WITH ADMIN FALSE propagation (#7191)

citusdata · Sep 11, 2023 · c1dc378 · c1dc378
1 parent d628a4c
commit c1dc378
Show file tree

Hide file tree

Showing 3 changed files with 69 additions and 1 deletion.
diff --git a/src/backend/distributed/deparser/deparse_role_stmts.c b/src/backend/distributed/deparser/deparse_role_stmts.c
@@ -15,6 +15,7 @@
 
 #include "pg_version_compat.h"
 
+#include "commands/defrem.h"
 #include "distributed/citus_ruleutils.h"
 #include "distributed/deparser.h"
 #include "distributed/listutils.h"
@@ -384,7 +385,10 @@ AppendGrantWithAdminOption(StringInfo buf, GrantRoleStmt *stmt)
 		DefElem *opt = NULL;
 		foreach_ptr(opt, stmt->opt)
 		{
-			if (strcmp(opt->defname, "admin") == 0)
+			bool admin_option = false;
+			char *optval = defGetString(opt);
+			if (strcmp(opt->defname, "admin") == 0 &&
+				parse_bool(optval, &admin_option) && admin_option)
 			{
 				appendStringInfo(buf, " WITH ADMIN OPTION");
 				break;

diff --git a/src/test/regress/expected/pg16.out b/src/test/regress/expected/pg16.out
@@ -971,6 +971,44 @@ LEFT JOIN ref_table ON TRUE;
             1.19
 (1 row)
 
+--
+-- PG16 added WITH ADMIN FALSE option to GRANT ROLE
+-- WITH ADMIN FALSE is the default, make sure we propagate correctly in Citus
+-- Relevant PG commit: https://github.com/postgres/postgres/commit/e3ce2de
+--
+CREATE ROLE role1;
+CREATE ROLE role2;
+SET citus.log_remote_commands TO on;
+SET citus.grep_remote_commands = '%GRANT%';
+-- default admin option is false
+GRANT role1 TO role2;
+NOTICE:  issuing GRANT role1 TO  role2;
+DETAIL:  on server postgres@localhost:xxxxx connectionId: xxxxxxx
+NOTICE:  issuing GRANT role1 TO  role2;
+DETAIL:  on server postgres@localhost:xxxxx connectionId: xxxxxxx
+REVOKE role1 FROM role2;
+-- should behave same as default
+GRANT role1 TO role2 WITH ADMIN FALSE;
+NOTICE:  issuing GRANT role1 TO  role2;
+DETAIL:  on server postgres@localhost:xxxxx connectionId: xxxxxxx
+NOTICE:  issuing GRANT role1 TO  role2;
+DETAIL:  on server postgres@localhost:xxxxx connectionId: xxxxxxx
+REVOKE role1 FROM role2;
+-- with admin option and with admin true are the same
+GRANT role1 TO role2 WITH ADMIN OPTION;
+NOTICE:  issuing GRANT role1 TO  role2 WITH ADMIN OPTION;
+DETAIL:  on server postgres@localhost:xxxxx connectionId: xxxxxxx
+NOTICE:  issuing GRANT role1 TO  role2 WITH ADMIN OPTION;
+DETAIL:  on server postgres@localhost:xxxxx connectionId: xxxxxxx
+REVOKE role1 FROM role2;
+GRANT role1 TO role2 WITH ADMIN TRUE;
+NOTICE:  issuing GRANT role1 TO  role2 WITH ADMIN OPTION;
+DETAIL:  on server postgres@localhost:xxxxx connectionId: xxxxxxx
+NOTICE:  issuing GRANT role1 TO  role2 WITH ADMIN OPTION;
+DETAIL:  on server postgres@localhost:xxxxx connectionId: xxxxxxx
+REVOKE role1 FROM role2;
+RESET citus.log_remote_commands;
+RESET citus.grep_remote_commands;
 \set VERBOSITY terse
 SET client_min_messages TO ERROR;
 DROP EXTENSION postgres_fdw CASCADE;

diff --git a/src/test/regress/sql/pg16.sql b/src/test/regress/sql/pg16.sql
@@ -565,6 +565,32 @@ SELECT PERCENTILE_DISC((2 > random_normal(stddev => 1, mean => 0))::int::numeric
 FROM dist_table
 LEFT JOIN ref_table ON TRUE;
 
+--
+-- PG16 added WITH ADMIN FALSE option to GRANT ROLE
+-- WITH ADMIN FALSE is the default, make sure we propagate correctly in Citus
+-- Relevant PG commit: https://github.com/postgres/postgres/commit/e3ce2de
+--
+
+CREATE ROLE role1;
+CREATE ROLE role2;
+
+SET citus.log_remote_commands TO on;
+SET citus.grep_remote_commands = '%GRANT%';
+-- default admin option is false
+GRANT role1 TO role2;
+REVOKE role1 FROM role2;
+-- should behave same as default
+GRANT role1 TO role2 WITH ADMIN FALSE;
+REVOKE role1 FROM role2;
+-- with admin option and with admin true are the same
+GRANT role1 TO role2 WITH ADMIN OPTION;
+REVOKE role1 FROM role2;
+GRANT role1 TO role2 WITH ADMIN TRUE;
+REVOKE role1 FROM role2;
+
+RESET citus.log_remote_commands;
+RESET citus.grep_remote_commands;
+
 \set VERBOSITY terse
 SET client_min_messages TO ERROR;
 DROP EXTENSION postgres_fdw CASCADE;