Merge branch 'main' into alter_db_set_prop

.github/workflows/packaging-test-pipelines.yml

@@ -125,7 +125,6 @@ jobs:
           - debian-bullseye-all
           - ubuntu-focal-all
           - ubuntu-jammy-all
-          - ubuntu-kinetic-all
 
         POSTGRES_VERSION: ${{ fromJson(needs.get_postgres_versions_from_file.outputs.pg_versions) }}
 

src/backend/distributed/commands/database.c

@@ -213,6 +213,7 @@ PreprocessAlterDatabaseRefreshCollStmt(Node *node, const char *queryString,
 
 #endif
 
+
 /*
  * PreprocessAlterDatabaseSetStmt is executed before the statement is applied to the local
  * postgres instance.
@@ -241,3 +242,4 @@ PreprocessAlterDatabaseSetStmt(Node *node, const char *queryString,
 
 	return NodeDDLTaskList(NON_COORDINATOR_NODES, commands);
 }
+

src/backend/distributed/commands/distribute_object_ops.c

@@ -479,6 +479,7 @@ static DistributeObjectOps Database_Set = {
 	.markDistributed = false,
 };
 
+
 static DistributeObjectOps Domain_Alter = {
 	.deparse = DeparseAlterDomainStmt,
 	.qualify = QualifyAlterDomainStmt,
@@ -1335,6 +1336,7 @@ GetDistributeObjectOps(Node *node)
 			return &Database_Set;
 		}
 
+
 		case T_AlterDomainStmt:
 		{
 			return &Domain_Alter;

src/backend/distributed/commands/role.c

@@ -78,6 +78,7 @@ static const char * WrapQueryInAlterRoleIfExistsCall(const char *query, RoleSpec
 static VariableSetStmt * MakeVariableSetStmt(const char *config);
 static int ConfigGenericNameCompare(const void *lhs, const void *rhs);
 static List * RoleSpecToObjectAddress(RoleSpec *role, bool missing_ok);
+static bool IsGrantRoleWithInheritOrSetOption(GrantRoleStmt *stmt);
 
 /* controlled via GUC */
 bool EnableCreateRolePropagation = true;
@@ -1141,6 +1142,19 @@ PreprocessGrantRoleStmt(Node *node, const char *queryString,
 		return NIL;
 	}
 
+	if (IsGrantRoleWithInheritOrSetOption(stmt))
+	{
+		if (EnableUnsupportedFeatureMessages)
+		{
+			ereport(NOTICE, (errmsg("not propagating GRANT/REVOKE commands with specified"
+									" INHERIT/SET options to worker nodes"),
+							 errhint(
+								 "Connect to worker nodes directly to manually run the same"
+								 " GRANT/REVOKE command after disabling DDL propagation.")));
+		}
+		return NIL;
+	}
+
 	/*
 	 * Postgres don't seem to use the grantor. Even dropping the grantor doesn't
 	 * seem to affect the membership. If this changes, we might need to add grantors
@@ -1190,6 +1204,27 @@ PostprocessGrantRoleStmt(Node *node, const char *queryString)
 }
 
 
+/*
+ * IsGrantRoleWithInheritOrSetOption returns true if the given
+ * GrantRoleStmt has inherit or set option specified in its options
+ */
+static bool
+IsGrantRoleWithInheritOrSetOption(GrantRoleStmt *stmt)
+{
+#if PG_VERSION_NUM >= PG_VERSION_16
+	DefElem *opt = NULL;
+	foreach_ptr(opt, stmt->opt)
+	{
+		if (strcmp(opt->defname, "inherit") == 0 || strcmp(opt->defname, "set") == 0)
+		{
+			return true;
+		}
+	}
+#endif
+	return false;
+}
+
+
 /*
  * ConfigGenericNameCompare compares two config_generic structs based on their
  * name fields. If the name fields contain the same strings two structs are

src/backend/distributed/deparser/deparse_database_stmts.c

@@ -25,6 +25,7 @@
 #include "distributed/log_utils.h"
 #include "parser/parse_type.h"
 
+
 static void AppendAlterDatabaseOwnerStmt(StringInfo buf, AlterOwnerStmt *stmt);
 static void AppendAlterDatabaseStmt(StringInfo buf, AlterDatabaseStmt *stmt);
 
@@ -85,16 +86,6 @@ AppendGrantOnDatabaseStmt(StringInfo buf, GrantStmt *stmt)
 	AppendGrantSharedSuffix(buf, stmt);
 }
 
-
-static void
-AppendDefElemIsTemplate(StringInfo buf, DefElem *def)
-{
-	appendStringInfo(buf, " %s  %s", quote_identifier(def->defname),
-					 quote_literal_cstr(strVal(def->arg)));
-}
-
-
-static void
 AppendDefElemConnLimit(StringInfo buf, DefElem *def)
 {
 	appendStringInfo(buf, " CONNECTION LIMIT %ld", (long int) defGetNumeric(def));
@@ -109,13 +100,14 @@ AppendAlterDatabaseStmt(StringInfo buf, AlterDatabaseStmt *stmt)
 	if (stmt->options)
 	{
 		ListCell *cell = NULL;
-		appendStringInfo(buf, "WITH");
+		appendStringInfo(buf, "WITH ");
 		foreach(cell, stmt->options)
 		{
 			DefElem *def = castNode(DefElem, lfirst(cell));
 			if (strcmp(def->defname, "is_template") == 0)
 			{
-				AppendDefElemIsTemplate(buf, def);
+				appendStringInfo(buf, "IS_TEMPLATE %s",
+								 quote_literal_cstr(strVal(def->arg)));
 			}
 			else if (strcmp(def->defname, "connection_limit") == 0)
 			{
@@ -187,7 +179,6 @@ DeparseAlterDatabaseRefreshCollStmt(Node *node)
 
 #endif
 
-
 static void
 AppendAlterDatabaseSetStmt(StringInfo buf, AlterDatabaseSetStmt *stmt)
 {
@@ -211,3 +202,4 @@ DeparseAlterDatabaseSetStmt(Node *node)
 
 	return str.data;
 }
+

src/include/distributed/deparser.h

@@ -227,6 +227,7 @@ extern char * DeparseAlterDatabaseStmt(Node *node);
 extern char * DeparseAlterDatabaseRefreshCollStmt(Node *node);
 extern char * DeparseAlterDatabaseSetStmt(Node *node);
 
+
 /* forward declaration for deparse_publication_stmts.c */
 extern char * DeparseCreatePublicationStmt(Node *stmt);
 extern char * DeparseCreatePublicationStmtExtended(Node *node,

src/test/regress/expected/alter_database_propagation.out

@@ -6,29 +6,30 @@ set citus.grep_remote_commands = '%ALTER DATABASE%';
 alter database regression ALLOW_CONNECTIONS false;
 ERROR:  ALLOW_CONNECTIONS is not supported
 alter database regression with CONNECTION LIMIT 100;
-NOTICE:  issuing ALTER DATABASE regression WITH CONNECTION LIMIT 100;
+
+NOTICE:  issuing ALTER DATABASE regression WITH  CONNECTION LIMIT 100;
 DETAIL:  on server postgres@localhost:xxxxx connectionId: xxxxxxx
-NOTICE:  issuing ALTER DATABASE regression WITH CONNECTION LIMIT 100;
+NOTICE:  issuing ALTER DATABASE regression WITH  CONNECTION LIMIT 100;
 DETAIL:  on server postgres@localhost:xxxxx connectionId: xxxxxxx
 alter database regression with IS_TEMPLATE true CONNECTION LIMIT 50;
-NOTICE:  issuing ALTER DATABASE regression WITH is_template  'true' CONNECTION LIMIT 50;
+NOTICE:  issuing ALTER DATABASE regression WITH IS_TEMPLATE 'true' CONNECTION LIMIT 50;
 DETAIL:  on server postgres@localhost:xxxxx connectionId: xxxxxxx
-NOTICE:  issuing ALTER DATABASE regression WITH is_template  'true' CONNECTION LIMIT 50;
+NOTICE:  issuing ALTER DATABASE regression WITH IS_TEMPLATE 'true' CONNECTION LIMIT 50;
 DETAIL:  on server postgres@localhost:xxxxx connectionId: xxxxxxx
 alter database regression with CONNECTION LIMIT -1;
-NOTICE:  issuing ALTER DATABASE regression WITH CONNECTION LIMIT -1;
+NOTICE:  issuing ALTER DATABASE regression WITH  CONNECTION LIMIT -1;
 DETAIL:  on server postgres@localhost:xxxxx connectionId: xxxxxxx
-NOTICE:  issuing ALTER DATABASE regression WITH CONNECTION LIMIT -1;
+NOTICE:  issuing ALTER DATABASE regression WITH  CONNECTION LIMIT -1;
 DETAIL:  on server postgres@localhost:xxxxx connectionId: xxxxxxx
 alter database regression with IS_TEMPLATE true;
-NOTICE:  issuing ALTER DATABASE regression WITH is_template  'true';
+NOTICE:  issuing ALTER DATABASE regression WITH IS_TEMPLATE 'true';
 DETAIL:  on server postgres@localhost:xxxxx connectionId: xxxxxxx
-NOTICE:  issuing ALTER DATABASE regression WITH is_template  'true';
+NOTICE:  issuing ALTER DATABASE regression WITH IS_TEMPLATE 'true';
 DETAIL:  on server postgres@localhost:xxxxx connectionId: xxxxxxx
 alter database regression with IS_TEMPLATE false;
-NOTICE:  issuing ALTER DATABASE regression WITH is_template  'false';
+NOTICE:  issuing ALTER DATABASE regression WITH IS_TEMPLATE 'false';
 DETAIL:  on server postgres@localhost:xxxxx connectionId: xxxxxxx
-NOTICE:  issuing ALTER DATABASE regression WITH is_template  'false';
+NOTICE:  issuing ALTER DATABASE regression WITH IS_TEMPLATE 'false';
 DETAIL:  on server postgres@localhost:xxxxx connectionId: xxxxxxx
 -- this statement will get error since we don't have a multiple database support for now
 alter database regression rename to regression2;

src/test/regress/expected/pg16.out

@@ -1009,6 +1009,95 @@ DETAIL:  on server postgres@localhost:xxxxx connectionId: xxxxxxx
 REVOKE role1 FROM role2;
 RESET citus.log_remote_commands;
 RESET citus.grep_remote_commands;
+--
+-- PG16 added new options to GRANT ROLE
+-- inherit: https://github.com/postgres/postgres/commit/e3ce2de
+-- set: https://github.com/postgres/postgres/commit/3d14e17
+-- We don't propagate for now in Citus
+--
+GRANT role1 TO role2 WITH INHERIT FALSE;
+NOTICE:  not propagating GRANT/REVOKE commands with specified INHERIT/SET options to worker nodes
+HINT:  Connect to worker nodes directly to manually run the same GRANT/REVOKE command after disabling DDL propagation.
+REVOKE role1 FROM role2;
+GRANT role1 TO role2 WITH INHERIT TRUE;
+NOTICE:  not propagating GRANT/REVOKE commands with specified INHERIT/SET options to worker nodes
+HINT:  Connect to worker nodes directly to manually run the same GRANT/REVOKE command after disabling DDL propagation.
+REVOKE role1 FROM role2;
+GRANT role1 TO role2 WITH INHERIT OPTION;
+NOTICE:  not propagating GRANT/REVOKE commands with specified INHERIT/SET options to worker nodes
+HINT:  Connect to worker nodes directly to manually run the same GRANT/REVOKE command after disabling DDL propagation.
+REVOKE role1 FROM role2;
+GRANT role1 TO role2 WITH SET FALSE;
+NOTICE:  not propagating GRANT/REVOKE commands with specified INHERIT/SET options to worker nodes
+HINT:  Connect to worker nodes directly to manually run the same GRANT/REVOKE command after disabling DDL propagation.
+REVOKE role1 FROM role2;
+GRANT role1 TO role2 WITH SET TRUE;
+NOTICE:  not propagating GRANT/REVOKE commands with specified INHERIT/SET options to worker nodes
+HINT:  Connect to worker nodes directly to manually run the same GRANT/REVOKE command after disabling DDL propagation.
+REVOKE role1 FROM role2;
+GRANT role1 TO role2 WITH SET OPTION;
+NOTICE:  not propagating GRANT/REVOKE commands with specified INHERIT/SET options to worker nodes
+HINT:  Connect to worker nodes directly to manually run the same GRANT/REVOKE command after disabling DDL propagation.
+REVOKE role1 FROM role2;
+-- connect to worker node
+GRANT role1 TO role2 WITH ADMIN OPTION, INHERIT FALSE, SET FALSE;
+NOTICE:  not propagating GRANT/REVOKE commands with specified INHERIT/SET options to worker nodes
+HINT:  Connect to worker nodes directly to manually run the same GRANT/REVOKE command after disabling DDL propagation.
+SELECT roleid::regrole::text AS role, member::regrole::text,
+admin_option, inherit_option, set_option FROM pg_auth_members
+WHERE roleid::regrole::text = 'role1' ORDER BY 1, 2;
+ role  | member | admin_option | inherit_option | set_option
+---------------------------------------------------------------------
+ role1 | role2  | t            | f              | f
+(1 row)
+
+\c - - - :worker_1_port
+SELECT roleid::regrole::text AS role, member::regrole::text,
+admin_option, inherit_option, set_option FROM pg_auth_members
+WHERE roleid::regrole::text = 'role1' ORDER BY 1, 2;
+ role | member | admin_option | inherit_option | set_option
+---------------------------------------------------------------------
+(0 rows)
+
+SET citus.enable_ddl_propagation TO off;
+GRANT role1 TO role2 WITH ADMIN OPTION, INHERIT FALSE, SET FALSE;
+RESET citus.enable_ddl_propagation;
+SELECT roleid::regrole::text AS role, member::regrole::text,
+admin_option, inherit_option, set_option FROM pg_auth_members
+WHERE roleid::regrole::text = 'role1' ORDER BY 1, 2;
+ role  | member | admin_option | inherit_option | set_option
+---------------------------------------------------------------------
+ role1 | role2  | t            | f              | f
+(1 row)
+
+\c - - - :master_port
+REVOKE role1 FROM role2;
+-- test REVOKES as well
+GRANT role1 TO role2;
+REVOKE SET OPTION FOR role1 FROM role2;
+NOTICE:  not propagating GRANT/REVOKE commands with specified INHERIT/SET options to worker nodes
+HINT:  Connect to worker nodes directly to manually run the same GRANT/REVOKE command after disabling DDL propagation.
+REVOKE INHERIT OPTION FOR role1 FROM role2;
+NOTICE:  not propagating GRANT/REVOKE commands with specified INHERIT/SET options to worker nodes
+HINT:  Connect to worker nodes directly to manually run the same GRANT/REVOKE command after disabling DDL propagation.
+DROP ROLE role1, role2;
+-- test that everything works fine for roles that are not propagated
+SET citus.enable_ddl_propagation TO off;
+CREATE ROLE role3;
+CREATE ROLE role4;
+CREATE ROLE role5;
+RESET citus.enable_ddl_propagation;
+-- by default, admin option is false, inherit is true, set is true
+GRANT role3 TO role4;
+GRANT role3 TO role5 WITH ADMIN TRUE, INHERIT FALSE, SET FALSE;
+SELECT roleid::regrole::text AS role, member::regrole::text, admin_option, inherit_option, set_option FROM pg_auth_members WHERE roleid::regrole::text = 'role3' ORDER BY 1, 2;
+ role  | member | admin_option | inherit_option | set_option
+---------------------------------------------------------------------
+ role3 | role4  | f            | t              | t
+ role3 | role5  | t            | f              | f
+(2 rows)
+
+DROP ROLE role3, role4, role5;
 \set VERBOSITY terse
 SET client_min_messages TO ERROR;
 DROP EXTENSION postgres_fdw CASCADE;

src/test/regress/sql/alter_database_propagation.sql

@@ -1,7 +1,8 @@
 set citus.log_remote_commands = true;
 set citus.grep_remote_commands = '%ALTER DATABASE%';
 
---since ALLOW_CONNECTIONS alter option should be executed in a different database
+
+-- since ALLOW_CONNECTIONS alter option should be executed in a different database
 -- and since we don't have a multiple database support for now,
 -- this statement will get error
 alter database regression ALLOW_CONNECTIONS false;
@@ -55,13 +56,4 @@ alter database regression set lock_timeout from current;
 alter database regression set lock_timeout to DEFAULT;
 alter database regression RESET lock_timeout;
 
-
-
-
-
-
-
-
-
-
 set citus.log_remote_commands = false;

src/test/regress/sql/pg16.sql

@@ -591,6 +591,69 @@ REVOKE role1 FROM role2;
 RESET citus.log_remote_commands;
 RESET citus.grep_remote_commands;
 
+--
+-- PG16 added new options to GRANT ROLE
+-- inherit: https://github.com/postgres/postgres/commit/e3ce2de
+-- set: https://github.com/postgres/postgres/commit/3d14e17
+-- We don't propagate for now in Citus
+--
+GRANT role1 TO role2 WITH INHERIT FALSE;
+REVOKE role1 FROM role2;
+GRANT role1 TO role2 WITH INHERIT TRUE;
+REVOKE role1 FROM role2;
+GRANT role1 TO role2 WITH INHERIT OPTION;
+REVOKE role1 FROM role2;
+GRANT role1 TO role2 WITH SET FALSE;
+REVOKE role1 FROM role2;
+GRANT role1 TO role2 WITH SET TRUE;
+REVOKE role1 FROM role2;
+GRANT role1 TO role2 WITH SET OPTION;
+REVOKE role1 FROM role2;
+
+-- connect to worker node
+GRANT role1 TO role2 WITH ADMIN OPTION, INHERIT FALSE, SET FALSE;
+
+SELECT roleid::regrole::text AS role, member::regrole::text,
+admin_option, inherit_option, set_option FROM pg_auth_members
+WHERE roleid::regrole::text = 'role1' ORDER BY 1, 2;
+
+\c - - - :worker_1_port
+
+SELECT roleid::regrole::text AS role, member::regrole::text,
+admin_option, inherit_option, set_option FROM pg_auth_members
+WHERE roleid::regrole::text = 'role1' ORDER BY 1, 2;
+
+SET citus.enable_ddl_propagation TO off;
+GRANT role1 TO role2 WITH ADMIN OPTION, INHERIT FALSE, SET FALSE;
+RESET citus.enable_ddl_propagation;
+
+SELECT roleid::regrole::text AS role, member::regrole::text,
+admin_option, inherit_option, set_option FROM pg_auth_members
+WHERE roleid::regrole::text = 'role1' ORDER BY 1, 2;
+
+\c - - - :master_port
+REVOKE role1 FROM role2;
+
+-- test REVOKES as well
+GRANT role1 TO role2;
+REVOKE SET OPTION FOR role1 FROM role2;
+REVOKE INHERIT OPTION FOR role1 FROM role2;
+
+DROP ROLE role1, role2;
+
+-- test that everything works fine for roles that are not propagated
+SET citus.enable_ddl_propagation TO off;
+CREATE ROLE role3;
+CREATE ROLE role4;
+CREATE ROLE role5;
+RESET citus.enable_ddl_propagation;
+-- by default, admin option is false, inherit is true, set is true
+GRANT role3 TO role4;
+GRANT role3 TO role5 WITH ADMIN TRUE, INHERIT FALSE, SET FALSE;
+SELECT roleid::regrole::text AS role, member::regrole::text, admin_option, inherit_option, set_option FROM pg_auth_members WHERE roleid::regrole::text = 'role3' ORDER BY 1, 2;
+
+DROP ROLE role3, role4, role5;
+
 \set VERBOSITY terse
 SET client_min_messages TO ERROR;
 DROP EXTENSION postgres_fdw CASCADE;