diff --git a/src/backend/distributed/commands/database.c b/src/backend/distributed/commands/database.c index 28b4465bc..ec3e47518 100644 --- a/src/backend/distributed/commands/database.c +++ b/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 - * be run on all workers to create the database. + * In this stage, we perform validations that we want to ensure before delegating to + * 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 - * GRANT on distributed databases. - */ -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. + * CreateDatabaseDDLCommand returns a CREATE DATABASE command to create given + * database * - * Commands in the list are wrapped by citus_internal_database_command() UDF - * to avoid from transaction block restrictions that apply to database commands + * Command is wrapped by citus_internal_database_command() UDF + * to avoid from transaction block restrictions that apply to database commands. */ -List * -GenerateCreateDatabaseCommandList(void) +char * +CreateDatabaseDDLCommand(Oid dbId) { - List *commands = 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) + HeapTuple tuple = SearchSysCache1(DATABASEOID, ObjectIdGetDatum(dbId)); + if (!HeapTupleIsValid(tuple)) { - 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; - } - - 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); + ereport(ERROR, (errcode(ERRCODE_UNDEFINED_DATABASE), + errmsg("database with OID %u does not exist", dbId))); } - heap_endscan(scan); - table_close(pgDatabaseRel, AccessShareLock); + Form_pg_database databaseForm = (Form_pg_database) GETSTRUCT(tuple); - 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; } diff --git a/src/backend/distributed/commands/dependencies.c b/src/backend/distributed/commands/dependencies.c index e309ee86c..6ce699cf5 100644 --- a/src/backend/distributed/commands/dependencies.c +++ b/src/backend/distributed/commands/dependencies.c @@ -457,16 +457,37 @@ GetDependencyCreateDDLCommands(const ObjectAddress *dependency) case OCLASS_DATABASE: { - List *databaseDDLCommands = NIL; - - /* only propagate the ownership of the database when the feature is on */ - if (EnableAlterDatabaseOwner) + /* + * For the database where Citus is installed, only propagate the ownership of the + * database, only when the feature is on. + * + * 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); - databaseDDLCommands = list_concat(databaseDDLCommands, ownerDDLCommands); + return DatabaseOwnerDDLCommands(dependency); } - 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: diff --git a/src/backend/distributed/deparser/deparse_database_stmts.c b/src/backend/distributed/deparser/deparse_database_stmts.c index 72a2d0cf1..04a0779ef 100644 --- a/src/backend/distributed/deparser/deparse_database_stmts.c +++ b/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; - - - foreach_ptr(option, stmt->options) + if (list_length(stmt->options) > 1) { - /* if it is the first option then append with "WITH" else append with "," */ - if (option == linitial(stmt->options)) - { - appendStringInfo(buf, " WITH ( "); - } - else - { - appendStringInfo(buf, ", "); - } + /* FORCE is the only option that can be provided for this command */ + elog(ERROR, "got unexpected number of options for DROP DATABASE"); + } + else if (list_length(stmt->options) == 1) + { + DefElem *option = linitial(stmt->options); + appendStringInfo(buf, " WITH ( "); 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 ")" */ - if (option == llast(stmt->options)) - { - appendStringInfo(buf, " )"); - } + appendStringInfo(buf, " )"); } } -/* - * Converts a DropdbStmt structure into a SQL command string. - * Used in the deparsing of drop database statement. - */ char * DeparseDropDatabaseStmt(Node *node) { diff --git a/src/backend/distributed/metadata/dependency.c b/src/backend/distributed/metadata/dependency.c index 989e957af..be2ceb3e3 100644 --- a/src/backend/distributed/metadata/dependency.c +++ b/src/backend/distributed/metadata/dependency.c @@ -698,7 +698,6 @@ SupportedDependencyByCitus(const ObjectAddress *address) case OCLASS_DATABASE: { - /* only to propagate its owner */ return true; } diff --git a/src/backend/distributed/metadata/metadata_sync.c b/src/backend/distributed/metadata/metadata_sync.c index ce2201903..ea6fd6dea 100644 --- a/src/backend/distributed/metadata/metadata_sync.c +++ b/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. diff --git a/src/backend/distributed/sql/citus--12.1-1--12.2-1.sql b/src/backend/distributed/sql/citus--12.1-1--12.2-1.sql index 578a182ef..63c4a527f 100644 --- a/src/backend/distributed/sql/citus--12.1-1--12.2-1.sql +++ b/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" diff --git a/src/backend/distributed/sql/downgrades/citus--12.2-1--12.1-1.sql b/src/backend/distributed/sql/downgrades/citus--12.2-1--12.1-1.sql index 4120c4f9e..d568cb034 100644 --- a/src/backend/distributed/sql/downgrades/citus--12.2-1--12.1-1.sql +++ b/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" diff --git a/src/backend/distributed/sql/udfs/citus_internal_database_command/12.2-1.sql b/src/backend/distributed/sql/udfs/citus_internal_database_command/12.2-1.sql index b20f6278e..9f6d873cc 100644 --- a/src/backend/distributed/sql/udfs/citus_internal_database_command/12.2-1.sql +++ b/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 diff --git a/src/backend/distributed/sql/udfs/citus_internal_database_command/latest.sql b/src/backend/distributed/sql/udfs/citus_internal_database_command/latest.sql index b20f6278e..9f6d873cc 100644 --- a/src/backend/distributed/sql/udfs/citus_internal_database_command/latest.sql +++ b/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 diff --git a/src/include/distributed/commands.h b/src/include/distributed/commands.h index 8686524c7..615585de4 100644 --- a/src/include/distributed/commands.h +++ b/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 diff --git a/src/test/regress/expected/create_drop_database_propagation.out b/src/test/regress/expected/create_drop_database_propagation.out index 7c8fa1b62..e0172f3e8 100644 --- a/src/test/regress/expected/create_drop_database_propagation.out +++ b/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( diff --git a/src/test/regress/multi_1_schedule b/src/test/regress/multi_1_schedule index 41672ae07..9528cc704 100644 --- a/src/test/regress/multi_1_schedule +++ b/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 diff --git a/src/test/regress/sql/create_drop_database_propagation.sql b/src/test/regress/sql/create_drop_database_propagation.sql index 569bae498..c83548d68 100644 --- a/src/test/regress/sql/create_drop_database_propagation.sql +++ b/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