From d940cfa9927b3f88fa863ee59adadb159450cf4a Mon Sep 17 00:00:00 2001 From: Onur Tirtir Date: Wed, 3 Jan 2024 17:03:06 +0300 Subject: [PATCH] Do nothing if the database is not distributed (#7392) Fixes the remaining cases reported in https://github.com/citusdata/citus/issues/7370. --- src/backend/distributed/commands/database.c | 53 ++++++++++++++++--- src/test/regress/citus_tests/run_test.py | 1 + .../create_drop_database_propagation.out | 47 +++++++++------- .../grant_on_database_propagation.out | 16 +++--- src/test/regress/expected/pg15.out | 10 ++++ .../sql/create_drop_database_propagation.sql | 13 +++++ src/test/regress/sql/pg15.sql | 13 +++++ 7 files changed, 118 insertions(+), 35 deletions(-) diff --git a/src/backend/distributed/commands/database.c b/src/backend/distributed/commands/database.c index 5bd84fb9c..fd83ae0fa 100644 --- a/src/backend/distributed/commands/database.c +++ b/src/backend/distributed/commands/database.c @@ -74,6 +74,7 @@ static char * GetTablespaceName(Oid tablespaceOid); static ObjectAddress * GetDatabaseAddressFromDatabaseName(char *databaseName, bool missingOk); +static List * FilterDistributedDatabases(List *databases); static Oid get_database_owner(Oid dbId); @@ -173,17 +174,23 @@ PreprocessGrantOnDatabaseStmt(Node *node, const char *queryString, GrantStmt *stmt = castNode(GrantStmt, node); Assert(stmt->objtype == OBJECT_DATABASE); - List *databaseList = stmt->objects; + List *distributedDatabases = FilterDistributedDatabases(stmt->objects); - if (list_length(databaseList) == 0) + if (list_length(distributedDatabases) == 0) { return NIL; } EnsureCoordinator(); + List *originalObjects = stmt->objects; + + stmt->objects = distributedDatabases; + char *sql = DeparseTreeNode((Node *) stmt); + stmt->objects = originalObjects; + List *commands = list_make3(DISABLE_DDL_PROPAGATION, (void *) sql, ENABLE_DDL_PROPAGATION); @@ -192,6 +199,30 @@ PreprocessGrantOnDatabaseStmt(Node *node, const char *queryString, } +/* + * FilterDistributedDatabases filters the database list and returns the distributed ones, + * as a list. + */ +static List * +FilterDistributedDatabases(List *databases) +{ + List *distributedDatabases = NIL; + String *databaseName = NULL; + foreach_ptr(databaseName, databases) + { + bool missingOk = true; + ObjectAddress *dbAddress = + GetDatabaseAddressFromDatabaseName(strVal(databaseName), missingOk); + if (IsAnyObjectDistributed(list_make1(dbAddress))) + { + distributedDatabases = lappend(distributedDatabases, databaseName); + } + } + + return distributedDatabases; +} + + /* * IsSetTablespaceStatement returns true if the statement is a SET TABLESPACE statement, * false otherwise. @@ -270,13 +301,16 @@ List * PreprocessAlterDatabaseRefreshCollStmt(Node *node, const char *queryString, ProcessUtilityContext processUtilityContext) { - if (!ShouldPropagate()) + bool missingOk = true; + AlterDatabaseRefreshCollStmt *stmt = castNode(AlterDatabaseRefreshCollStmt, node); + ObjectAddress *dbAddress = GetDatabaseAddressFromDatabaseName(stmt->dbname, + missingOk); + + if (!ShouldPropagate() || !IsAnyObjectDistributed(list_make1(dbAddress))) { return NIL; } - AlterDatabaseRefreshCollStmt *stmt = castNode(AlterDatabaseRefreshCollStmt, node); - EnsureCoordinator(); char *sql = DeparseTreeNode((Node *) stmt); @@ -332,13 +366,16 @@ List * PreprocessAlterDatabaseSetStmt(Node *node, const char *queryString, ProcessUtilityContext processUtilityContext) { - if (!ShouldPropagate()) + AlterDatabaseSetStmt *stmt = castNode(AlterDatabaseSetStmt, node); + + bool missingOk = true; + ObjectAddress *dbAddress = GetDatabaseAddressFromDatabaseName(stmt->dbname, + missingOk); + if (!ShouldPropagate() || !IsAnyObjectDistributed(list_make1(dbAddress))) { return NIL; } - AlterDatabaseSetStmt *stmt = castNode(AlterDatabaseSetStmt, node); - EnsureCoordinator(); char *sql = DeparseTreeNode((Node *) stmt); diff --git a/src/test/regress/citus_tests/run_test.py b/src/test/regress/citus_tests/run_test.py index 158a44ef6..16f562af9 100755 --- a/src/test/regress/citus_tests/run_test.py +++ b/src/test/regress/citus_tests/run_test.py @@ -199,6 +199,7 @@ DEPS = { repeatable=False, ), "multi_prepare_plsql": TestDeps("base_schedule"), + "pg15": TestDeps("base_schedule"), } diff --git a/src/test/regress/expected/create_drop_database_propagation.out b/src/test/regress/expected/create_drop_database_propagation.out index eb637f8c2..8104f6666 100644 --- a/src/test/regress/expected/create_drop_database_propagation.out +++ b/src/test/regress/expected/create_drop_database_propagation.out @@ -544,10 +544,10 @@ SELECT result from run_command_on_all_nodes( revoke connect,temp,temporary,create on database db_role_grants_test_non_distributed from public $$ ) ORDER BY result; - result + result --------------------------------------------------------------------- - ERROR: operation is not allowed on this node - ERROR: operation is not allowed on this node + REVOKE + REVOKE REVOKE (3 rows) @@ -577,11 +577,7 @@ grant CONNECT,TEMPORARY,CREATE on DATABASE db_role_grants_test to db_role_grants 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; -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, -- role db_role_grants_test_role_exists_on_node_2 SELECT result from run_command_on_all_nodes( @@ -661,7 +657,7 @@ SELECT result from run_command_on_all_nodes( ) ORDER BY result; result --------------------------------------------------------------------- - t + f t (2 rows) @@ -672,7 +668,7 @@ SELECT result from run_command_on_all_nodes( ) ORDER BY result; result --------------------------------------------------------------------- - t + f t (2 rows) @@ -683,7 +679,7 @@ SELECT result from run_command_on_all_nodes( ) ORDER BY result; result --------------------------------------------------------------------- - t + f t (2 rows) @@ -696,7 +692,7 @@ SELECT result from run_command_on_all_nodes( ) ORDER BY result; result --------------------------------------------------------------------- - t + f t (2 rows) @@ -707,7 +703,7 @@ SELECT result from run_command_on_all_nodes( ) ORDER BY result; result --------------------------------------------------------------------- - t + f t (2 rows) @@ -718,7 +714,7 @@ SELECT result from run_command_on_all_nodes( ) ORDER BY result; result --------------------------------------------------------------------- - t + f t (2 rows) @@ -816,7 +812,7 @@ SELECT result from run_command_on_all_nodes( result --------------------------------------------------------------------- f - t + f t (3 rows) @@ -828,7 +824,7 @@ SELECT result from run_command_on_all_nodes( result --------------------------------------------------------------------- f - t + f t (3 rows) @@ -840,7 +836,7 @@ SELECT result from run_command_on_all_nodes( result --------------------------------------------------------------------- f - t + f t (3 rows) @@ -854,7 +850,7 @@ SELECT result from run_command_on_all_nodes( result --------------------------------------------------------------------- f - t + f t (3 rows) @@ -866,7 +862,7 @@ SELECT result from run_command_on_all_nodes( result --------------------------------------------------------------------- f - t + f t (3 rows) @@ -878,7 +874,7 @@ SELECT result from run_command_on_all_nodes( result --------------------------------------------------------------------- f - t + f t (3 rows) @@ -932,6 +928,19 @@ 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; +-- show that we don't try to propagate commands on non-distributed databases +SET citus.enable_create_database_propagation TO OFF; +CREATE DATABASE local_database_1; +NOTICE: Citus partially supports CREATE DATABASE for distributed databases +DETAIL: Citus does not propagate CREATE DATABASE command to workers +HINT: You can manually create a database and its extensions on workers. +SET citus.enable_create_database_propagation TO ON; +CREATE ROLE local_role_1; +GRANT CONNECT, TEMPORARY, CREATE ON DATABASE local_database_1 TO local_role_1; +ALTER DATABASE local_database_1 SET default_transaction_read_only = 'true'; +REVOKE CONNECT, TEMPORARY, CREATE ON DATABASE local_database_1 FROM local_role_1; +DROP ROLE local_role_1; +DROP DATABASE local_database_1; --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/expected/grant_on_database_propagation.out b/src/test/regress/expected/grant_on_database_propagation.out index 2fd135314..b3b8b4b90 100644 --- a/src/test/regress/expected/grant_on_database_propagation.out +++ b/src/test/regress/expected/grant_on_database_propagation.out @@ -677,7 +677,7 @@ select has_database_privilege('myuser','regression', 'TEMPORARY'); select has_database_privilege('myuser','test_db', 'CREATE'); has_database_privilege --------------------------------------------------------------------- - t + f (1 row) select has_database_privilege('myuser','test_db', 'CONNECT'); @@ -725,7 +725,7 @@ select has_database_privilege('myuser_1','regression', 'TEMPORARY'); select has_database_privilege('myuser_1','test_db', 'CREATE'); has_database_privilege --------------------------------------------------------------------- - t + f (1 row) select has_database_privilege('myuser_1','test_db', 'CONNECT'); @@ -884,19 +884,19 @@ select has_database_privilege('myuser','test_db', 'CREATE'); select has_database_privilege('myuser','test_db', 'CONNECT'); has_database_privilege --------------------------------------------------------------------- - f + t (1 row) select has_database_privilege('myuser','test_db', 'TEMP'); has_database_privilege --------------------------------------------------------------------- - f + t (1 row) select has_database_privilege('myuser','test_db', 'TEMPORARY'); has_database_privilege --------------------------------------------------------------------- - f + t (1 row) select has_database_privilege('myuser_1','regression', 'CREATE'); @@ -932,19 +932,19 @@ select has_database_privilege('myuser_1','test_db', 'CREATE'); select has_database_privilege('myuser_1','test_db', 'CONNECT'); has_database_privilege --------------------------------------------------------------------- - f + t (1 row) select has_database_privilege('myuser_1','test_db', 'TEMP'); has_database_privilege --------------------------------------------------------------------- - f + t (1 row) select has_database_privilege('myuser_1','test_db', 'TEMPORARY'); has_database_privilege --------------------------------------------------------------------- - f + t (1 row) \c - - - :master_port diff --git a/src/test/regress/expected/pg15.out b/src/test/regress/expected/pg15.out index fcbb0cd12..eff8b0ce6 100644 --- a/src/test/regress/expected/pg15.out +++ b/src/test/regress/expected/pg15.out @@ -1529,6 +1529,16 @@ alter database regression REFRESH COLLATION VERSION; NOTICE: version has not changed NOTICE: issuing ALTER DATABASE regression REFRESH COLLATION VERSION; NOTICE: issuing ALTER DATABASE regression REFRESH COLLATION VERSION; +SET citus.enable_create_database_propagation TO OFF; +CREATE DATABASE local_database_1; +NOTICE: Citus partially supports CREATE DATABASE for distributed databases +RESET citus.enable_create_database_propagation; +CREATE ROLE local_role_1; +ALTER DATABASE local_database_1 REFRESH COLLATION VERSION; +NOTICE: version has not changed +REVOKE CONNECT, TEMPORARY, CREATE ON DATABASE local_database_1 FROM local_role_1; +DROP ROLE local_role_1; +DROP DATABASE local_database_1; set citus.log_remote_commands = false; -- Clean up \set VERBOSITY terse diff --git a/src/test/regress/sql/create_drop_database_propagation.sql b/src/test/regress/sql/create_drop_database_propagation.sql index c71841eee..aeb469c1e 100644 --- a/src/test/regress/sql/create_drop_database_propagation.sql +++ b/src/test/regress/sql/create_drop_database_propagation.sql @@ -536,6 +536,19 @@ REVOKE CONNECT ON DATABASE test_db FROM propagated_role; DROP DATABASE test_db; DROP ROLE propagated_role, non_propagated_role; +-- show that we don't try to propagate commands on non-distributed databases +SET citus.enable_create_database_propagation TO OFF; +CREATE DATABASE local_database_1; +SET citus.enable_create_database_propagation TO ON; + +CREATE ROLE local_role_1; + +GRANT CONNECT, TEMPORARY, CREATE ON DATABASE local_database_1 TO local_role_1; +ALTER DATABASE local_database_1 SET default_transaction_read_only = 'true'; + +REVOKE CONNECT, TEMPORARY, CREATE ON DATABASE local_database_1 FROM local_role_1; +DROP ROLE local_role_1; +DROP DATABASE local_database_1; --clean up resources created by this test diff --git a/src/test/regress/sql/pg15.sql b/src/test/regress/sql/pg15.sql index fe60222dd..cd9dab58c 100644 --- a/src/test/regress/sql/pg15.sql +++ b/src/test/regress/sql/pg15.sql @@ -968,6 +968,19 @@ ORDER BY is_coordinator DESC, result; set citus.log_remote_commands = true; set citus.grep_remote_commands = '%ALTER DATABASE%'; alter database regression REFRESH COLLATION VERSION; + +SET citus.enable_create_database_propagation TO OFF; +CREATE DATABASE local_database_1; +RESET citus.enable_create_database_propagation; + +CREATE ROLE local_role_1; + +ALTER DATABASE local_database_1 REFRESH COLLATION VERSION; + +REVOKE CONNECT, TEMPORARY, CREATE ON DATABASE local_database_1 FROM local_role_1; +DROP ROLE local_role_1; +DROP DATABASE local_database_1; + set citus.log_remote_commands = false; -- Clean up