Merge branch 'main' into alter_database_additional_options

src/backend/distributed/commands/database.c

@@ -11,36 +11,51 @@
 
 #include "postgres.h"
 #include "miscadmin.h"
+#include "miscadmin.h"
 
+#include "access/heapam.h"
 #include "access/heapam.h"
 #include "access/htup_details.h"
 #include "access/xact.h"
 #include "catalog/objectaddress.h"
 #include "catalog/pg_collation.h"
+#include "catalog/pg_collation.h"
 #include "catalog/pg_database.h"
 #include "catalog/pg_database_d.h"
 #include "catalog/pg_tablespace.h"
+#include "catalog/pg_database_d.h"
+#include "catalog/pg_tablespace.h"
 #include "commands/dbcommands.h"
 #include "commands/defrem.h"
+#include "commands/defrem.h"
 #include "nodes/parsenodes.h"
 #include "utils/builtins.h"
 #include "utils/lsyscache.h"
 #include "utils/rel.h"
 #include "utils/relcache.h"
+#include "utils/builtins.h"
+#include "utils/lsyscache.h"
+#include "utils/rel.h"
+#include "utils/relcache.h"
 #include "utils/syscache.h"
 
+#include "distributed/adaptive_executor.h"
 #include "distributed/adaptive_executor.h"
 #include "distributed/commands.h"
 #include "distributed/commands/utility_hook.h"
 #include "distributed/deparse_shard_query.h"
+#include "distributed/deparse_shard_query.h"
 #include "distributed/deparser.h"
 #include "distributed/listutils.h"
 #include "distributed/metadata/distobject.h"
+#include "distributed/listutils.h"
+#include "distributed/metadata/distobject.h"
 #include "distributed/metadata_sync.h"
 #include "distributed/metadata_utility.h"
 #include "distributed/multi_executor.h"
 #include "distributed/relation_access_tracking.h"
 #include "distributed/worker_protocol.h"
+#include "distributed/worker_protocol.h"
 #include "distributed/worker_transaction.h"
 
 
@@ -137,13 +152,13 @@ RecreateAlterDatabaseOwnerStmt(Oid databaseOid)
  * get_database_owner returns the Oid of the role owning the database
  */
 static Oid
-get_database_owner(Oid db_oid)
+get_database_owner(Oid dbId)
 {
-	HeapTuple tuple = SearchSysCache1(DATABASEOID, ObjectIdGetDatum(db_oid));
+	HeapTuple tuple = SearchSysCache1(DATABASEOID, ObjectIdGetDatum(dbId));
 	if (!HeapTupleIsValid(tuple))
 	{
 		ereport(ERROR, (errcode(ERRCODE_UNDEFINED_DATABASE),
-						errmsg("database with OID %u does not exist", db_oid)));
+						errmsg("database with OID %u does not exist", dbId)));
 	}
 
 	Oid dba = ((Form_pg_database) GETSTRUCT(tuple))->datdba;
@@ -343,61 +358,13 @@ PreprocessAlterDatabaseSetStmt(Node *node, const char *queryString,
 }
 
 
-/*
- * This function validates the options provided for the CREATE DATABASE command.
- * It iterates over each option in the stmt->options list and checks if it's supported.
- * If an unsupported option is found, or if a supported option has an invalid value,
- * it raises an error.
- *
- * Parameters:
- * stmt: A CreatedbStmt struct representing a CREATE DATABASE command.
- *       The options field is a list of DefElem structs, each representing an option.
- *
- * Currently, this function checks for the following:
- * - The "oid" option is not supported.
- * - The "template" option is only supported with the value "template1".
- * - The "strategy" option is only supported with the value "wal_log".
- *
- * If any of these checks fail, the function calls ereport to raise an error.
- */
-static void
-EnsureSupportedCreateDatabaseCommand(CreatedbStmt *stmt)
-{
-	DefElem *option = NULL;
-	foreach_ptr(option, stmt->options)
-	{
-		if (strcmp(option->defname, "oid") == 0)
-		{
-			ereport(ERROR,
-					errmsg("CREATE DATABASE option \"%s\" is not supported",
-						   option->defname));
-		}
-
-		char *optionValue = defGetString(option);
-
-		if (strcmp(option->defname, "template") == 0 && strcmp(optionValue,
-															   "template1") != 0)
-		{
-			ereport(ERROR, errmsg("Only template1 is supported as template "
-								  "parameter for CREATE DATABASE"));
-		}
-
-		if (strcmp(option->defname, "strategy") == 0 && strcmp(optionValue, "wal_log") !=
-			0)
-		{
-			ereport(ERROR, errmsg("Only wal_log is supported as strategy "
-								  "parameter for CREATE DATABASE"));
-		}
-	}
-}
-
-
 /*
  * PostprocessAlterDatabaseStmt is executed before the statement is applied to the local
- * postgres instance.
+ * Postgres instance.
  *
- * In this stage, we can perform validations and prepare the commands that need to
+ * In this stage, we perform validations that we want to ensure before delegating to
- * be run on all workers to create the database.
+ * previous utility hooks because it might not be convenient to throw an error in an
+ * implicit transaction that creates a database.
  */
 List *
 PreprocessCreateDatabaseStmt(Node *node, const char *queryString,
@@ -410,7 +377,6 @@ PreprocessCreateDatabaseStmt(Node *node, const char *queryString,
 
 	EnsureCoordinator();
 
-	/*validate the statement*/
 	CreatedbStmt *stmt = castNode(CreatedbStmt, node);
 	EnsureSupportedCreateDatabaseCommand(stmt);
 
@@ -420,7 +386,7 @@ PreprocessCreateDatabaseStmt(Node *node, const char *queryString,
 
 /*
  * PostprocessCreateDatabaseStmt is executed after the statement is applied to the local
- * postgres instance. In this stage we can prepare the commands that need to be run on
+ * postgres instance. In this stage we prepare the commands that need to be run on
  * all workers to create the database. Since the CREATE DATABASE statement gives error
  * in a transaction block, we need to use NontransactionalNodeDDLTaskList to send the
  * CREATE DATABASE statement to the workers.
@@ -436,6 +402,16 @@ PostprocessCreateDatabaseStmt(Node *node, const char *queryString)
 
 	EnsureCoordinator();
 
+	/*
+	 * Given that CREATE DATABASE doesn't support "IF NOT EXISTS" and we're
+	 * in the post-process, database must exist, hence missingOk = false.
+	 */
+	bool missingOk = false;
+	bool isPostProcess = true;
+	List *addresses = GetObjectAddressListFromParseTree(node, missingOk,
+														isPostProcess);
+	EnsureAllObjectDependenciesExistOnAllNodes(addresses);
+
 	char *createDatabaseCommand = DeparseTreeNode(node);
 
 	List *commands = list_make3(DISABLE_DDL_PROPAGATION,
@@ -492,20 +468,6 @@ PreprocessDropDatabaseStmt(Node *node, const char *queryString,
 }
 
 
-/*
- * GetDatabaseAddressFromDatabaseName gets the database name and returns the ObjectAddress
- * of the database.
- */
-static ObjectAddress *
-GetDatabaseAddressFromDatabaseName(char *databaseName, bool missingOk)
-{
-	Oid databaseOid = get_database_oid(databaseName, missingOk);
-	ObjectAddress *dbObjectAddress = palloc0(sizeof(ObjectAddress));
-	ObjectAddressSet(*dbObjectAddress, DatabaseRelationId, databaseOid);
-	return dbObjectAddress;
-}
-
-
 /*
  * DropDatabaseStmtObjectAddress gets the ObjectAddress of the database that is the
  * object of the DropdbStmt.
@@ -534,6 +496,65 @@ CreateDatabaseStmtObjectAddress(Node *node, bool missingOk, bool isPostprocess)
 }
 
 
+/*
+ * EnsureSupportedCreateDatabaseCommand validates the options provided for the CREATE
+ * DATABASE command.
+ *
+ * Parameters:
+ * stmt: A CreatedbStmt struct representing a CREATE DATABASE command.
+ *       The options field is a list of DefElem structs, each representing an option.
+ *
+ * Currently, this function checks for the following:
+ * - The "oid" option is not supported.
+ * - The "template" option is only supported with the value "template1".
+ * - The "strategy" option is only supported with the value "wal_log".
+ */
+void
+EnsureSupportedCreateDatabaseCommand(CreatedbStmt *stmt)
+{
+	DefElem *option = NULL;
+	foreach_ptr(option, stmt->options)
+	{
+		if (strcmp(option->defname, "oid") == 0)
+		{
+			ereport(ERROR,
+					errmsg("CREATE DATABASE option \"%s\" is not supported",
+						   option->defname));
+		}
+
+		char *optionValue = defGetString(option);
+
+		if (strcmp(option->defname, "template") == 0 &&
+			strcmp(optionValue, "template1") != 0)
+		{
+			ereport(ERROR, errmsg("Only template1 is supported as template "
+								  "parameter for CREATE DATABASE"));
+		}
+
+		if (strcmp(option->defname, "strategy") == 0 &&
+			strcmp(optionValue, "wal_log") != 0)
+		{
+			ereport(ERROR, errmsg("Only wal_log is supported as strategy "
+								  "parameter for CREATE DATABASE"));
+		}
+	}
+}
+
+
+/*
+ * GetDatabaseAddressFromDatabaseName gets the database name and returns the ObjectAddress
+ * of the database.
+ */
+static ObjectAddress *
+GetDatabaseAddressFromDatabaseName(char *databaseName, bool missingOk)
+{
+	Oid databaseOid = get_database_oid(databaseName, missingOk);
+	ObjectAddress *dbObjectAddress = palloc0(sizeof(ObjectAddress));
+	ObjectAddressSet(*dbObjectAddress, DatabaseRelationId, databaseOid);
+	return dbObjectAddress;
+}
+
+
 /*
  * GetTablespaceName gets the tablespace oid and returns the tablespace name.
  */
@@ -721,89 +742,34 @@ GenerateCreateDatabaseStatementFromPgDatabase(Form_pg_database databaseForm)
 
 
 /*
- * GrantOnDatabaseDDLCommands returns a list of sql statements to idempotently apply a
+ * CreateDatabaseDDLCommand returns a CREATE DATABASE command to create given
- * GRANT on distributed databases.
+ * database
- */
-List *
-GenerateGrantDatabaseCommandList(void)
-{
-	List *grantCommands = NIL;
-
-	Relation pgDatabaseRel = table_open(DatabaseRelationId, AccessShareLock);
-	TableScanDesc scan = table_beginscan_catalog(pgDatabaseRel, 0, NULL);
-
-	HeapTuple tuple = NULL;
-	while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL)
-	{
-		Form_pg_database databaseForm = (Form_pg_database) GETSTRUCT(tuple);
-
-		ObjectAddress *dbAddress = GetDatabaseAddressFromDatabaseName(
-			NameStr(databaseForm->datname), false);
-
-		/* skip databases that are not distributed */
-		if (!IsAnyObjectDistributed(list_make1(dbAddress)))
-		{
-			continue;
-		}
-
-		List *dbGrants = GrantOnDatabaseDDLCommands(databaseForm->oid);
-
-		/* append dbGrants into grantCommands*/
-		grantCommands = list_concat(grantCommands, dbGrants);
-	}
-
-	heap_endscan(scan);
-	table_close(pgDatabaseRel, AccessShareLock);
-
-	return grantCommands;
-}
-
-
-/*
- * GenerateCreateDatabaseCommandList returns a list of CREATE DATABASE statements
- * for all the databases.
  *
- * Commands in the list are wrapped by citus_internal_database_command() UDF
+ * Command is wrapped by citus_internal_database_command() UDF
- * to avoid from transaction block restrictions that apply to database commands
+ * to avoid from transaction block restrictions that apply to database commands.
  */
-List *
+char *
-GenerateCreateDatabaseCommandList(void)
+CreateDatabaseDDLCommand(Oid dbId)
 {
-	List *commands = NIL;
+	HeapTuple tuple = SearchSysCache1(DATABASEOID, ObjectIdGetDatum(dbId));
-
+	if (!HeapTupleIsValid(tuple))
-	Relation pgDatabaseRel = table_open(DatabaseRelationId, AccessShareLock);
-	TableScanDesc scan = table_beginscan_catalog(pgDatabaseRel, 0, NULL);
-
-	HeapTuple tuple = NULL;
-	while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL)
 	{
-		Form_pg_database databaseForm = (Form_pg_database) GETSTRUCT(tuple);
+		ereport(ERROR, (errcode(ERRCODE_UNDEFINED_DATABASE),
-
+						errmsg("database with OID %u does not exist", dbId)));
-		ObjectAddress *dbAddress = GetDatabaseAddressFromDatabaseName(
-			NameStr(databaseForm->datname), false);
-
-		/* skip databases that are not distributed */
-		if (!IsAnyObjectDistributed(list_make1(dbAddress)))
-		{
-			continue;
-		}
-
-		char *createStmt = GenerateCreateDatabaseStatementFromPgDatabase(databaseForm);
-
-		StringInfo outerDbStmt = makeStringInfo();
-
-		/* Generate the CREATE DATABASE statement */
-		appendStringInfo(outerDbStmt,
-						 "SELECT pg_catalog.citus_internal_database_command(%s)",
-						 quote_literal_cstr(
-							 createStmt));
-
-		/* Add the statement to the list of commands */
-		commands = lappend(commands, outerDbStmt->data);
 	}
 
-	heap_endscan(scan);
+	Form_pg_database databaseForm = (Form_pg_database) GETSTRUCT(tuple);
-	table_close(pgDatabaseRel, AccessShareLock);
 
-	return commands;
+	char *createStmt = GenerateCreateDatabaseStatementFromPgDatabase(databaseForm);
+
+	StringInfo outerDbStmt = makeStringInfo();
+
+	/* Generate the CREATE DATABASE statement */
+	appendStringInfo(outerDbStmt,
+					 "SELECT pg_catalog.citus_internal_database_command(%s)",
+					 quote_literal_cstr(createStmt));
+
+	ReleaseSysCache(tuple);
+
+	return outerDbStmt->data;
 }

src/backend/distributed/commands/dependencies.c

@@ -457,16 +457,37 @@ GetDependencyCreateDDLCommands(const ObjectAddress *dependency)
 
 		case OCLASS_DATABASE:
 		{
-			List *databaseDDLCommands = NIL;
+			/*
-
+			 * For the database where Citus is installed, only propagate the ownership of the
-			/* only propagate the ownership of the database when the feature is on */
+			 * database, only when the feature is on.
-			if (EnableAlterDatabaseOwner)
+			 *
+			 * This is because this database must exist on all nodes already so we shouldn't
+			 * need to "CREATE" it on other nodes. However, we still need to correctly reflect
+			 * its owner on other nodes too.
+			 */
+			if (dependency->objectId == MyDatabaseId && EnableAlterDatabaseOwner)
 			{
-				List *ownerDDLCommands = DatabaseOwnerDDLCommands(dependency);
+				return DatabaseOwnerDDLCommands(dependency);
-				databaseDDLCommands = list_concat(databaseDDLCommands, ownerDDLCommands);
 			}
 
-			return databaseDDLCommands;
+			/*
+			 * For the other databases, create the database on all nodes, only when the feature
+			 * is on.
+			 */
+			if (dependency->objectId != MyDatabaseId && EnableCreateDatabasePropagation)
+			{
+				char *databaseDDLCommand = CreateDatabaseDDLCommand(dependency->objectId);
+
+				List *ddlCommands = list_make1(databaseDDLCommand);
+
+				List *grantDDLCommands = GrantOnDatabaseDDLCommands(dependency->objectId);
+
+				ddlCommands = list_concat(ddlCommands, grantDDLCommands);
+
+				return ddlCommands;
+			}
+
+			return NIL;
 		}
 
 		case OCLASS_PROC:

src/backend/distributed/deparser/deparse_database_stmts.c

@@ -13,17 +13,19 @@
 
 #include "pg_version_compat.h"
 #include "catalog/namespace.h"
+#include "commands/defrem.h"
 #include "lib/stringinfo.h"
 #include "nodes/parsenodes.h"
+#include "parser/parse_type.h"
 #include "utils/builtins.h"
 
 #include "commands/defrem.h"
 #include "distributed/deparser.h"
+#include "distributed/commands.h"
 #include "distributed/citus_ruleutils.h"
 #include "distributed/deparser.h"
 #include "distributed/listutils.h"
 #include "distributed/log_utils.h"
-#include "parser/parse_type.h"
 
 
 static void AppendAlterDatabaseOwnerStmt(StringInfo buf, AlterOwnerStmt *stmt);
@@ -289,26 +291,25 @@ DeparseAlterDatabaseSetStmt(Node *node)
 static void
 AppendCreateDatabaseStmt(StringInfo buf, CreatedbStmt *stmt)
 {
+	/*
+	 * Make sure that we don't try to deparse something that this
+	 * function doesn't expect.
+	 */
+	EnsureSupportedCreateDatabaseCommand(stmt);
+
 	appendStringInfo(buf,
 					 "CREATE DATABASE %s",
 					 quote_identifier(stmt->dbname));
 
 	DefElem *option = NULL;
-
 	foreach_ptr(option, stmt->options)
 	{
-		/*ValidateCreateDatabaseOptions(option); */
-
 		DefElemOptionToStatement(buf, option, create_database_option_formats,
 								 lengthof(create_database_option_formats));
 	}
 }
 
 
-/*
- * Converts a CreatedbStmt structure into a SQL command string.
- * Used in the deparsing of Create database statement.
- */
 char *
 DeparseCreateDatabaseStmt(Node *node)
 {
@@ -331,20 +332,15 @@ AppendDropDatabaseStmt(StringInfo buf, DropdbStmt *stmt)
 					 ifExistsStatement,
 					 quote_identifier(stmt->dbname));
 
-	DefElem *option = NULL;
+	if (list_length(stmt->options) > 1)
-
-
-	foreach_ptr(option, stmt->options)
 	{
-		/* if it is the first option then append with "WITH" else append with "," */
+		/* FORCE is the only option that can be provided for this command */
-		if (option == linitial(stmt->options))
+		elog(ERROR, "got unexpected number of options for DROP DATABASE");
-		{
+	}
-			appendStringInfo(buf, " WITH ( ");
+	else if (list_length(stmt->options) == 1)
-		}
+	{
-		else
+		DefElem *option = linitial(stmt->options);
-		{
+		appendStringInfo(buf, " WITH ( ");
-			appendStringInfo(buf, ", ");
-		}
 
 		if (strcmp(option->defname, "force") == 0)
 		{
@@ -352,24 +348,17 @@ AppendDropDatabaseStmt(StringInfo buf, DropdbStmt *stmt)
 		}
 		else
 		{
+			/* FORCE is the only option that can be provided for this command */
 			ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR),
 							errmsg("unrecognized DROP DATABASE option \"%s\"",
 								   option->defname)));
 		}
 
-		/* if it is the last  option then append with ")" */
+		appendStringInfo(buf, " )");
-		if (option == llast(stmt->options))
-		{
-			appendStringInfo(buf, " )");
-		}
 	}
 }
 
 
-/*
- * Converts a DropdbStmt structure into a SQL command string.
- * Used in the deparsing of drop database statement.
- */
 char *
 DeparseDropDatabaseStmt(Node *node)
 {

src/backend/distributed/metadata/dependency.c

@@ -698,7 +698,6 @@ SupportedDependencyByCitus(const ObjectAddress *address)
 
 		case OCLASS_DATABASE:
 		{
-			/* only to propagate its owner */
 			return true;
 		}
 

src/backend/distributed/metadata/metadata_sync.c

@@ -2049,6 +2049,10 @@ GenerateGrantOnSchemaQueriesFromAclItem(Oid schemaOid, AclItem *aclItem)
 }
 
 
+/*
+ * GrantOnDatabaseDDLCommands creates a list of ddl command for replicating the permissions
+ * of roles on databases.
+ */
 List *
 GrantOnDatabaseDDLCommands(Oid databaseOid)
 {
@@ -2079,6 +2083,10 @@ GrantOnDatabaseDDLCommands(Oid databaseOid)
 }
 
 
+/*
+ * GenerateGrantOnDatabaseFromAclItem generates a query string for replicating a users permissions
+ * on a database.
+ */
 List *
 GenerateGrantOnDatabaseFromAclItem(Oid databaseOid, AclItem *aclItem)
 {
@@ -4010,7 +4018,7 @@ citus_internal_database_command(PG_FUNCTION_ARGS)
 					  GUC_ACTION_LOCAL, true, 0, false);
 
 	/*
-	 * createdb()  uses ParseState to report the error position for the
+	 * createdb() uses ParseState to report the error position for the
 	 * input command and the position is reported to be 0 when it's provided as NULL.
 	 * We're okay with that because we don't expect this UDF to be called with an incorrect
 	 * DDL command.

src/backend/distributed/sql/citus--12.1-1--12.2-1.sql

@@ -1,4 +1,5 @@
 -- citus--12.1-1--12.2-1
 -- bump version to 12.2-1
+
 #include "udfs/citus_internal_database_command/12.2-1.sql"
 #include "udfs/citus_add_rebalance_strategy/12.2-1.sql"

src/backend/distributed/sql/downgrades/citus--12.2-1--12.1-1.sql

@@ -1,4 +1,6 @@
 -- citus--12.2-1--12.1-1
+
 DROP FUNCTION pg_catalog.citus_internal_database_command(text);
+
 #include "../udfs/citus_add_rebalance_strategy/10.1-1.sql"
 

src/backend/distributed/sql/udfs/citus_internal_database_command/12.2-1.sql

@@ -1,5 +1,5 @@
 --
--- citus_internal_database_command creates a database according to the given command.
+-- citus_internal_database_command run given database command without transaction block restriction.
 
 CREATE OR REPLACE FUNCTION pg_catalog.citus_internal_database_command(command text)
  RETURNS void

src/backend/distributed/sql/udfs/citus_internal_database_command/latest.sql

@@ -1,5 +1,5 @@
 --
--- citus_internal_database_command creates a database according to the given command.
+-- citus_internal_database_command run given database command without transaction block restriction.
 
 CREATE OR REPLACE FUNCTION pg_catalog.citus_internal_database_command(command text)
  RETURNS void

src/include/distributed/commands.h

@@ -251,6 +251,9 @@ extern List * PreprocessAlterDatabaseRenameStmt(Node *node, const char *queryStr
 												ProcessUtilityContext
 												processUtilityContext);
 
+extern void EnsureSupportedCreateDatabaseCommand(CreatedbStmt *stmt);
+extern char * CreateDatabaseDDLCommand(Oid dbId);
+
 
 /* domain.c - forward declarations */
 extern List * CreateDomainStmtObjectAddress(Node *node, bool missing_ok, bool

src/test/regress/expected/create_drop_database_propagation.out

@@ -209,7 +209,19 @@ SELECT result FROM run_command_on_all_nodes(
 CREATE USER "role-needs\!escape";
 CREATE DATABASE "db-needs\!escape" owner "role-needs\!escape" tablespace "ts-needs\!escape";
 -- Rename it to make check_database_on_all_nodes happy.
-ALTER DATABASE "db-needs\!escape" RENAME TO db_needs_escape;
+-- Today we don't support ALTER DATABASE .. RENAME TO .., so need to propagate it manually.
+SELECT result FROM run_command_on_all_nodes(
+  $$
+  ALTER DATABASE "db-needs\!escape" RENAME TO db_needs_escape
+  $$
+);
+     result
+---------------------------------------------------------------------
+ ALTER DATABASE
+ ALTER DATABASE
+ ALTER DATABASE
+(3 rows)
+
 SELECT * FROM public.check_database_on_all_nodes('db_needs_escape') ORDER BY node_type;
       node_type       |                                                                                                                                                                                                                                   result
 ---------------------------------------------------------------------
@@ -541,7 +553,7 @@ SELECT result from run_command_on_all_nodes(
 
 SELECT result from run_command_on_all_nodes(
   $$
-  revoke connect,temp,temporary,create  on database db_role_grants_test_non_distributed from public
+  revoke connect,temp,temporary,create on database db_role_grants_test_non_distributed from public
   $$
 ) ORDER BY result;
                     result
@@ -560,7 +572,7 @@ select 1 from citus_remove_node('localhost', :worker_2_port);
 (1 row)
 
 CREATE DATABASE db_role_grants_test;
-revoke connect,temp,temporary,create  on database db_role_grants_test from public;
+revoke connect,temp,temporary,create on database db_role_grants_test from public;
 SET citus.log_remote_commands = true;
 set citus.grep_remote_commands = '%CREATE ROLE%';
 CREATE ROLE db_role_grants_test_role_missing_on_node_2;
@@ -573,13 +585,13 @@ set citus.grep_remote_commands = '%GRANT%';
 grant CONNECT,TEMPORARY,CREATE on DATABASE db_role_grants_test to db_role_grants_test_role_exists_on_node_2;
 NOTICE:  issuing GRANT connect, temporary, create ON DATABASE db_role_grants_test TO db_role_grants_test_role_exists_on_node_2;
 DETAIL:  on server postgres@localhost:xxxxx connectionId: xxxxxxx
-grant CONNECT,TEMPORARY,CREATE  on DATABASE db_role_grants_test to db_role_grants_test_role_missing_on_node_2;
+grant CONNECT,TEMPORARY,CREATE on DATABASE db_role_grants_test to db_role_grants_test_role_missing_on_node_2;
 NOTICE:  issuing GRANT connect, temporary, create ON DATABASE db_role_grants_test TO db_role_grants_test_role_missing_on_node_2;
 DETAIL:  on server postgres@localhost:xxxxx connectionId: xxxxxxx
 grant CONNECT,TEMPORARY,CREATE on DATABASE db_role_grants_test_non_distributed to db_role_grants_test_role_exists_on_node_2;
 NOTICE:  issuing GRANT connect, temporary, create ON DATABASE db_role_grants_test_non_distributed TO db_role_grants_test_role_exists_on_node_2;
 DETAIL:  on server postgres@localhost:xxxxx connectionId: xxxxxxx
-grant CONNECT,TEMPORARY,CREATE  on DATABASE db_role_grants_test_non_distributed to db_role_grants_test_role_missing_on_node_2;
+grant CONNECT,TEMPORARY,CREATE on DATABASE db_role_grants_test_non_distributed to db_role_grants_test_role_missing_on_node_2;
 NOTICE:  issuing GRANT connect, temporary, create ON DATABASE db_role_grants_test_non_distributed TO db_role_grants_test_role_missing_on_node_2;
 DETAIL:  on server postgres@localhost:xxxxx connectionId: xxxxxxx
 -- check the privileges before add_node for database db_role_grants_test,
@@ -882,7 +894,7 @@ SELECT result from run_command_on_all_nodes(
  t
 (3 rows)
 
-grant connect,temp,temporary,create  on database db_role_grants_test to public;
+grant connect,temp,temporary,create on database db_role_grants_test to public;
 DROP DATABASE db_role_grants_test;
 SELECT result from run_command_on_all_nodes(
   $$
@@ -898,6 +910,40 @@ SELECT result from run_command_on_all_nodes(
 
 DROP ROLE db_role_grants_test_role_exists_on_node_2;
 DROP ROLE db_role_grants_test_role_missing_on_node_2;
+select 1 from citus_remove_node('localhost', :worker_2_port);
+ ?column?
+---------------------------------------------------------------------
+        1
+(1 row)
+
+set citus.enable_create_role_propagation TO off;
+create role non_propagated_role;
+NOTICE:  not propagating CREATE ROLE/USER commands to other nodes
+HINT:  Connect to other nodes directly to manually create all necessary users and roles.
+set citus.enable_create_role_propagation TO on;
+set citus.enable_create_database_propagation TO on;
+-- Make sure that we propagate non_propagated_role because it's a dependency of test_db.
+-- And hence it becomes a distributed object.
+create database test_db OWNER non_propagated_role;
+create role propagated_role;
+grant connect on database test_db to propagated_role;
+SELECT 1 FROM citus_add_node('localhost', :worker_2_port);
+ ?column?
+---------------------------------------------------------------------
+        1
+(1 row)
+
+SELECT * FROM public.check_database_on_all_nodes('test_db') ORDER BY node_type;
+      node_type       |                                                                                                                                                                                                                                                                                   result
+---------------------------------------------------------------------
+ coordinator (local)  | {"database_properties": {"datacl": ["=Tc/non_propagated_role", "non_propagated_role=CTc/non_propagated_role", "propagated_role=c/non_propagated_role"], "datname": "test_db", "datctype": "C", "encoding": "UTF8", "datcollate": "C", "tablespace": "pg_default", "daticurules": null, "datallowconn": true, "datconnlimit": -1, "daticulocale": null, "datistemplate": false, "database_owner": "non_propagated_role", "datcollversion": null, "datlocprovider": "c"}, "pg_dist_object_record_for_db_exists": true, "stale_pg_dist_object_record_for_a_db_exists": false}
+ worker node (remote) | {"database_properties": {"datacl": ["=Tc/non_propagated_role", "non_propagated_role=CTc/non_propagated_role", "propagated_role=c/non_propagated_role"], "datname": "test_db", "datctype": "C", "encoding": "UTF8", "datcollate": "C", "tablespace": "pg_default", "daticurules": null, "datallowconn": true, "datconnlimit": -1, "daticulocale": null, "datistemplate": false, "database_owner": "non_propagated_role", "datcollversion": null, "datlocprovider": "c"}, "pg_dist_object_record_for_db_exists": true, "stale_pg_dist_object_record_for_a_db_exists": false}
+ worker node (remote) | {"database_properties": {"datacl": ["=Tc/non_propagated_role", "non_propagated_role=CTc/non_propagated_role", "propagated_role=c/non_propagated_role"], "datname": "test_db", "datctype": "C", "encoding": "UTF8", "datcollate": "C", "tablespace": "pg_default", "daticurules": null, "datallowconn": true, "datconnlimit": -1, "daticulocale": null, "datistemplate": false, "database_owner": "non_propagated_role", "datcollversion": null, "datlocprovider": "c"}, "pg_dist_object_record_for_db_exists": true, "stale_pg_dist_object_record_for_a_db_exists": false}
+(3 rows)
+
+REVOKE CONNECT ON DATABASE test_db FROM propagated_role;
+DROP DATABASE test_db;
+DROP ROLE propagated_role, non_propagated_role;
 --clean up resources created by this test
 -- DROP TABLESPACE is not supported, so we need to drop it manually.
 SELECT result FROM run_command_on_all_nodes(

src/test/regress/multi_1_schedule

@@ -38,7 +38,6 @@ test: create_single_shard_table
 test: create_drop_database_propagation
 test: create_drop_database_propagation_pg15
 test: create_drop_database_propagation_pg16
-
 # don't parallelize single_shard_table_udfs to make sure colocation ids are sequential
 test: single_shard_table_udfs
 test: schema_based_sharding

src/test/regress/sql/create_drop_database_propagation.sql

@@ -129,8 +129,12 @@ CREATE USER "role-needs\!escape";
 CREATE DATABASE "db-needs\!escape" owner "role-needs\!escape" tablespace "ts-needs\!escape";
 
 -- Rename it to make check_database_on_all_nodes happy.
-ALTER DATABASE "db-needs\!escape" RENAME TO db_needs_escape;
+-- Today we don't support ALTER DATABASE .. RENAME TO .., so need to propagate it manually.
-
+SELECT result FROM run_command_on_all_nodes(
+  $$
+  ALTER DATABASE "db-needs\!escape" RENAME TO db_needs_escape
+  $$
+);
 
 SELECT * FROM public.check_database_on_all_nodes('db_needs_escape') ORDER BY node_type;
 
@@ -292,8 +296,6 @@ drop database distributed_db;
 set citus.enable_create_database_propagation TO off;
 drop database non_distributed_db;
 
-
-
 -- test role grants on DATABASE in metadata sync
 
 SELECT result from run_command_on_all_nodes(
@@ -304,22 +306,19 @@ SELECT result from run_command_on_all_nodes(
 
 SELECT result from run_command_on_all_nodes(
   $$
-  revoke connect,temp,temporary,create  on database db_role_grants_test_non_distributed from public
+  revoke connect,temp,temporary,create on database db_role_grants_test_non_distributed from public
   $$
 ) ORDER BY result;
 
 SET citus.enable_create_database_propagation TO on;
 
-
-
 CREATE ROLE db_role_grants_test_role_exists_on_node_2;
 
-
 select 1 from citus_remove_node('localhost', :worker_2_port);
 
 CREATE DATABASE db_role_grants_test;
 
-revoke connect,temp,temporary,create  on database db_role_grants_test from public;
+revoke connect,temp,temporary,create on database db_role_grants_test from public;
 
 SET citus.log_remote_commands = true;
 set citus.grep_remote_commands = '%CREATE ROLE%';
@@ -328,17 +327,13 @@ CREATE ROLE db_role_grants_test_role_missing_on_node_2;
 RESET citus.log_remote_commands ;
 RESET citus.grep_remote_commands;
 
-
-
 SET citus.log_remote_commands = true;
 set citus.grep_remote_commands = '%GRANT%';
 grant CONNECT,TEMPORARY,CREATE on DATABASE db_role_grants_test to db_role_grants_test_role_exists_on_node_2;
-grant CONNECT,TEMPORARY,CREATE  on DATABASE db_role_grants_test to db_role_grants_test_role_missing_on_node_2;
+grant CONNECT,TEMPORARY,CREATE on DATABASE db_role_grants_test to db_role_grants_test_role_missing_on_node_2;
-
-
 
 grant CONNECT,TEMPORARY,CREATE on DATABASE db_role_grants_test_non_distributed to db_role_grants_test_role_exists_on_node_2;
-grant CONNECT,TEMPORARY,CREATE  on DATABASE db_role_grants_test_non_distributed to db_role_grants_test_role_missing_on_node_2;
+grant CONNECT,TEMPORARY,CREATE on DATABASE db_role_grants_test_non_distributed to db_role_grants_test_role_missing_on_node_2;
 
 -- check the privileges before add_node for database db_role_grants_test,
 --  role db_role_grants_test_role_exists_on_node_2
@@ -370,7 +365,6 @@ SELECT result from run_command_on_all_nodes(
   $$
 ) ORDER BY result;
 
-
 SELECT result from run_command_on_all_nodes(
   $$
   select has_database_privilege('db_role_grants_test_role_missing_on_node_2','db_role_grants_test', 'TEMPORARY')
@@ -412,7 +406,6 @@ SELECT result from run_command_on_all_nodes(
   $$
 ) ORDER BY result;
 
-
 SELECT result from run_command_on_all_nodes(
   $$
   select has_database_privilege('db_role_grants_test_role_missing_on_node_2','db_role_grants_test_non_distributed', 'TEMPORARY')
@@ -425,13 +418,11 @@ SELECT result from run_command_on_all_nodes(
   $$
 ) ORDER BY result;
 
-
 RESET citus.log_remote_commands;
 RESET citus.grep_remote_commands;
 
 select 1 from citus_add_node('localhost', :worker_2_port);
 
-
 -- check the privileges after add_node for database db_role_grants_test,
 --  role db_role_grants_test_role_exists_on_node_2
 
@@ -462,7 +453,6 @@ SELECT result from run_command_on_all_nodes(
   $$
 ) ORDER BY result;
 
-
 SELECT result from run_command_on_all_nodes(
   $$
   select has_database_privilege('db_role_grants_test_role_missing_on_node_2','db_role_grants_test', 'TEMPORARY')
@@ -504,7 +494,6 @@ SELECT result from run_command_on_all_nodes(
   $$
 ) ORDER BY result;
 
-
 SELECT result from run_command_on_all_nodes(
   $$
   select has_database_privilege('db_role_grants_test_role_missing_on_node_2','db_role_grants_test_non_distributed', 'TEMPORARY')
@@ -517,7 +506,7 @@ SELECT result from run_command_on_all_nodes(
   $$
 ) ORDER BY result;
 
-grant connect,temp,temporary,create  on database db_role_grants_test to public;
+grant connect,temp,temporary,create on database db_role_grants_test to public;
 
 DROP DATABASE db_role_grants_test;
 
@@ -529,8 +518,28 @@ SELECT result from run_command_on_all_nodes(
 DROP ROLE db_role_grants_test_role_exists_on_node_2;
 DROP ROLE db_role_grants_test_role_missing_on_node_2;
 
+select 1 from citus_remove_node('localhost', :worker_2_port);
 
+set citus.enable_create_role_propagation TO off;
+create role non_propagated_role;
+set citus.enable_create_role_propagation TO on;
 
+set citus.enable_create_database_propagation TO on;
+
+-- Make sure that we propagate non_propagated_role because it's a dependency of test_db.
+-- And hence it becomes a distributed object.
+create database test_db OWNER non_propagated_role;
+
+create role propagated_role;
+grant connect on database test_db to propagated_role;
+
+SELECT 1 FROM citus_add_node('localhost', :worker_2_port);
+
+SELECT * FROM public.check_database_on_all_nodes('test_db') ORDER BY node_type;
+
+REVOKE CONNECT ON DATABASE test_db FROM propagated_role;
+DROP DATABASE test_db;
+DROP ROLE propagated_role, non_propagated_role;
 
 --clean up resources created by this test