From 111b4c19bc0a429abc5be56dbb6ded9f080fa110 Mon Sep 17 00:00:00 2001 From: Onur Tirtir Date: Mon, 25 Sep 2023 11:14:35 +0300 Subject: [PATCH 1/4] Make sure to disallow creating a replicated distributed table concurrently (#7219) See explanation in https://github.com/citusdata/citus/issues/7216. Fixes https://github.com/citusdata/citus/issues/7216. DESCRIPTION: Makes sure to disallow creating a replicated distributed table concurrently --- .../commands/create_distributed_table.c | 13 ++++++ .../create_distributed_table_concurrently.out | 40 ++++++++----------- .../create_distributed_table_concurrently.sql | 9 +++++ 3 files changed, 39 insertions(+), 23 deletions(-) diff --git a/src/backend/distributed/commands/create_distributed_table.c b/src/backend/distributed/commands/create_distributed_table.c index dc06692b3..1e89c6b93 100644 --- a/src/backend/distributed/commands/create_distributed_table.c +++ b/src/backend/distributed/commands/create_distributed_table.c @@ -445,6 +445,19 @@ CreateDistributedTableConcurrently(Oid relationId, char *distributionColumnName, if (!IsColocateWithDefault(colocateWithTableName) && !IsColocateWithNone( colocateWithTableName)) { + if (replicationModel != REPLICATION_MODEL_STREAMING) + { + ereport(ERROR, (errmsg("cannot create distributed table " + "concurrently because Citus allows " + "concurrent table distribution only when " + "citus.shard_replication_factor = 1"), + errhint("table %s is requested to be colocated " + "with %s which has " + "citus.shard_replication_factor > 1", + get_rel_name(relationId), + colocateWithTableName))); + } + EnsureColocateWithTableIsValid(relationId, distributionMethod, distributionColumnName, colocateWithTableName); diff --git a/src/test/regress/expected/create_distributed_table_concurrently.out b/src/test/regress/expected/create_distributed_table_concurrently.out index 025629efa..1bf366fb3 100644 --- a/src/test/regress/expected/create_distributed_table_concurrently.out +++ b/src/test/regress/expected/create_distributed_table_concurrently.out @@ -36,6 +36,19 @@ set citus.shard_replication_factor to 2; select create_distributed_table_concurrently('test','key', 'hash'); ERROR: cannot distribute a table concurrently when citus.shard_replication_factor > 1 set citus.shard_replication_factor to 1; +set citus.shard_replication_factor to 2; +create table dist_1(a int); +select create_distributed_table('dist_1', 'a'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +set citus.shard_replication_factor to 1; +create table dist_2(a int); +select create_distributed_table_concurrently('dist_2', 'a', colocate_with=>'dist_1'); +ERROR: cannot create distributed table concurrently because Citus allows concurrent table distribution only when citus.shard_replication_factor = 1 +HINT: table dist_2 is requested to be colocated with dist_1 which has citus.shard_replication_factor > 1 begin; select create_distributed_table_concurrently('test','key'); ERROR: create_distributed_table_concurrently cannot run inside a transaction block @@ -138,27 +151,8 @@ select count(*) from test; rollback; -- verify that we can undistribute the table begin; +set local client_min_messages to warning; select undistribute_table('test', cascade_via_foreign_keys := true); -NOTICE: converting the partitions of create_distributed_table_concurrently.test -NOTICE: creating a new table for create_distributed_table_concurrently.test -NOTICE: dropping the old create_distributed_table_concurrently.test -NOTICE: renaming the new table to create_distributed_table_concurrently.test -NOTICE: creating a new table for create_distributed_table_concurrently.ref -NOTICE: moving the data of create_distributed_table_concurrently.ref -NOTICE: dropping the old create_distributed_table_concurrently.ref -NOTICE: drop cascades to constraint test_id_fkey_1190041 on table create_distributed_table_concurrently.test_1190041 -CONTEXT: SQL statement "SELECT citus_drop_all_shards(v_obj.objid, v_obj.schema_name, v_obj.object_name, drop_shards_metadata_only := false)" -PL/pgSQL function citus_drop_trigger() line XX at PERFORM -SQL statement "DROP TABLE create_distributed_table_concurrently.ref CASCADE" -NOTICE: renaming the new table to create_distributed_table_concurrently.ref -NOTICE: creating a new table for create_distributed_table_concurrently.test_1 -NOTICE: moving the data of create_distributed_table_concurrently.test_1 -NOTICE: dropping the old create_distributed_table_concurrently.test_1 -NOTICE: renaming the new table to create_distributed_table_concurrently.test_1 -NOTICE: creating a new table for create_distributed_table_concurrently.test_2 -NOTICE: moving the data of create_distributed_table_concurrently.test_2 -NOTICE: dropping the old create_distributed_table_concurrently.test_2 -NOTICE: renaming the new table to create_distributed_table_concurrently.test_2 undistribute_table --------------------------------------------------------------------- @@ -245,7 +239,7 @@ insert into dist_table4 select s from generate_series(1,100) s; select count(*) as total from dist_table4; total --------------------------------------------------------------------- - 100 + 100 (1 row) -- verify we do not allow foreign keys from distributed table to citus local table concurrently @@ -295,13 +289,13 @@ select count(*) from test_columnar; select id from test_columnar where id = 1; id --------------------------------------------------------------------- - 1 + 1 (1 row) select id from test_columnar where id = 51; id --------------------------------------------------------------------- - 51 + 51 (1 row) select count(*) from test_columnar_1; diff --git a/src/test/regress/sql/create_distributed_table_concurrently.sql b/src/test/regress/sql/create_distributed_table_concurrently.sql index 9632eba6e..6820d782c 100644 --- a/src/test/regress/sql/create_distributed_table_concurrently.sql +++ b/src/test/regress/sql/create_distributed_table_concurrently.sql @@ -28,6 +28,14 @@ set citus.shard_replication_factor to 2; select create_distributed_table_concurrently('test','key', 'hash'); set citus.shard_replication_factor to 1; +set citus.shard_replication_factor to 2; +create table dist_1(a int); +select create_distributed_table('dist_1', 'a'); +set citus.shard_replication_factor to 1; + +create table dist_2(a int); +select create_distributed_table_concurrently('dist_2', 'a', colocate_with=>'dist_1'); + begin; select create_distributed_table_concurrently('test','key'); rollback; @@ -63,6 +71,7 @@ rollback; -- verify that we can undistribute the table begin; +set local client_min_messages to warning; select undistribute_table('test', cascade_via_foreign_keys := true); rollback; From a9d28ca96ff4dca2c63617873e2a49428c49944a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCrkan=20=C4=B0ndibay?= Date: Mon, 25 Sep 2023 12:42:23 +0300 Subject: [PATCH 2/4] Adds make clean to installation steps (#7052) If you make a fresh install make clean is not required. However, if you install before, without a make install, one can get errors --------- Co-authored-by: aykut-bozkurt <51649454+aykut-bozkurt@users.noreply.github.com> --- CONTRIBUTING.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 66d026b8b..ac1f600ab 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -41,6 +41,8 @@ that are missing in earlier minor versions. cd citus ./configure + # If you have already installed the project, you need to clean it first + make clean make make install # Optionally, you might instead want to use `make install-all` @@ -79,6 +81,8 @@ that are missing in earlier minor versions. git clone https://github.com/citusdata/citus.git cd citus ./configure + # If you have already installed the project previously, you need to clean it first + make clean make sudo make install # Optionally, you might instead want to use `sudo make install-all` @@ -129,6 +133,8 @@ that are missing in earlier minor versions. git clone https://github.com/citusdata/citus.git cd citus PG_CONFIG=/usr/pgsql-14/bin/pg_config ./configure + # If you have already installed the project previously, you need to clean it first + make clean make sudo make install # Optionally, you might instead want to use `sudo make install-all` From 7fa109c9778c2d1d3f534f30a6534383e7f1abf1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCrkan=20=C4=B0ndibay?= Date: Tue, 26 Sep 2023 12:28:07 +0300 Subject: [PATCH 3/4] Adds alter user missing features (#7204) DESCRIPTION: Adds alter user rename propagation and enriches alter user tests --------- Co-authored-by: Jelte Fennema --- .../commands/distribute_object_ops.c | 16 +++ src/backend/distributed/commands/role.c | 51 ++++++++++ .../distributed/commands/utility_hook.c | 13 --- .../distributed/deparser/deparse_role_stmts.c | 99 ++++++++++++++----- src/include/distributed/commands.h | 8 ++ src/include/distributed/deparser.h | 1 + src/test/regress/citus_tests/run_test.py | 5 + .../expected/alter_role_propagation.out | 53 +++++++++- .../expected/multi_modifying_xacts.out | 27 +++-- .../regress/sql/alter_role_propagation.sql | 33 ++++++- .../regress/sql/multi_modifying_xacts.sql | 23 ++++- 11 files changed, 275 insertions(+), 54 deletions(-) diff --git a/src/backend/distributed/commands/distribute_object_ops.c b/src/backend/distributed/commands/distribute_object_ops.c index a17d75e17..a0a306e8d 100644 --- a/src/backend/distributed/commands/distribute_object_ops.c +++ b/src/backend/distributed/commands/distribute_object_ops.c @@ -150,6 +150,17 @@ static DistributeObjectOps Any_AlterRole = { .address = AlterRoleStmtObjectAddress, .markDistributed = false, }; + +static DistributeObjectOps Any_AlterRoleRename = { + .deparse = DeparseRenameRoleStmt, + .qualify = NULL, + .preprocess = PreprocessAlterRoleRenameStmt, + .postprocess = NULL, + .operationType = DIST_OPS_ALTER, + .address = RenameRoleStmtObjectAddress, + .markDistributed = false, +}; + static DistributeObjectOps Any_AlterRoleSet = { .deparse = DeparseAlterRoleSetStmt, .qualify = QualifyAlterRoleSetStmt, @@ -2059,6 +2070,11 @@ GetDistributeObjectOps(Node *node) return &Publication_Rename; } + case OBJECT_ROLE: + { + return &Any_AlterRoleRename; + } + case OBJECT_ROUTINE: { return &Routine_Rename; diff --git a/src/backend/distributed/commands/role.c b/src/backend/distributed/commands/role.c index 63ede986d..754be1a2b 100644 --- a/src/backend/distributed/commands/role.c +++ b/src/backend/distributed/commands/role.c @@ -1306,3 +1306,54 @@ EnsureSequentialModeForRoleDDL(void) "use only one connection for all future commands"))); SetLocalMultiShardModifyModeToSequential(); } + + +/* + * PreprocessAlterDatabaseSetStmt is executed before the statement is applied to the local + * postgres instance. + * + * In this stage we can prepare the commands that need to be run on all workers to grant + * on databases. + */ +List * +PreprocessAlterRoleRenameStmt(Node *node, const char *queryString, + ProcessUtilityContext processUtilityContext) +{ + if (!ShouldPropagate()) + { + return NIL; + } + + if (!EnableAlterRolePropagation) + { + return NIL; + } + + RenameStmt *stmt = castNode(RenameStmt, node); + Assert(stmt->renameType == OBJECT_ROLE); + + + EnsureCoordinator(); + + char *sql = DeparseTreeNode((Node *) stmt); + + List *commands = list_make3(DISABLE_DDL_PROPAGATION, + (void *) sql, + ENABLE_DDL_PROPAGATION); + + return NodeDDLTaskList(NON_COORDINATOR_NODES, commands); +} + + +List * +RenameRoleStmtObjectAddress(Node *node, bool missing_ok, bool isPostprocess) +{ + RenameStmt *stmt = castNode(RenameStmt, node); + Assert(stmt->renameType == OBJECT_ROLE); + + Oid roleOid = get_role_oid(stmt->subname, missing_ok); + ObjectAddress *address = palloc0(sizeof(ObjectAddress)); + ObjectAddressSet(*address, AuthIdRelationId, roleOid); + + return list_make1(address); +} diff --git a/src/backend/distributed/commands/utility_hook.c b/src/backend/distributed/commands/utility_hook.c index 10e424623..cf8e0644e 100644 --- a/src/backend/distributed/commands/utility_hook.c +++ b/src/backend/distributed/commands/utility_hook.c @@ -817,19 +817,6 @@ ProcessUtilityInternal(PlannedStmt *pstmt, ddlJobs = processJobs; } } - - if (IsA(parsetree, RenameStmt) && ((RenameStmt *) parsetree)->renameType == - OBJECT_ROLE && EnableAlterRolePropagation) - { - if (EnableUnsupportedFeatureMessages) - { - ereport(NOTICE, (errmsg( - "not propagating ALTER ROLE ... RENAME TO commands " - "to worker nodes"), - errhint("Connect to worker nodes directly to manually " - "rename the role"))); - } - } } if (IsA(parsetree, CreateStmt)) diff --git a/src/backend/distributed/deparser/deparse_role_stmts.c b/src/backend/distributed/deparser/deparse_role_stmts.c index 4d41f8ec4..ee216809e 100644 --- a/src/backend/distributed/deparser/deparse_role_stmts.c +++ b/src/backend/distributed/deparser/deparse_role_stmts.c @@ -201,17 +201,51 @@ DeparseCreateRoleStmt(Node *node) } +static void +AppendSysIdStatement(StringInfo buf, ListCell *optionCell) +{ + DefElem *option = (DefElem *) lfirst(optionCell); + if (strcmp(option->defname, "sysid") == 0) + { + appendStringInfo(buf, " SYSID %d", intVal(option->arg)); + } +} + + /* - * AppendCreateRoleStmt generates the string representation of the - * CreateRoleStmt and appends it to the buffer. + * AppendInlinePriviliges generates the string representation for the inline + * privileges of the role in create statement and appends it to the buffer. */ static void -AppendCreateRoleStmt(StringInfo buf, CreateRoleStmt *stmt) +AppendInlinePriviliges(StringInfo buf, ListCell *optionCell) { - ListCell *optionCell = NULL; + DefElem *option = (DefElem *) lfirst(optionCell); - appendStringInfo(buf, "CREATE "); + if (strcmp(option->defname, "adminmembers") == 0) + { + appendStringInfo(buf, " ADMIN "); + AppendRoleList(buf, (List *) option->arg); + } + else if (strcmp(option->defname, "rolemembers") == 0) + { + appendStringInfo(buf, " ROLE "); + AppendRoleList(buf, (List *) option->arg); + } + else if (strcmp(option->defname, "addroleto") == 0) + { + appendStringInfo(buf, " IN ROLE "); + AppendRoleList(buf, (List *) option->arg); + } +} + +/* + * AppendStatementType generates the string representation for the statement + * type (role, user or group) in alter/create statement and appends it to the buffer. + */ +static void +AppendStatementType(StringInfo buf, CreateRoleStmt *stmt) +{ switch (stmt->stmt_type) { case ROLESTMT_ROLE: @@ -232,34 +266,29 @@ AppendCreateRoleStmt(StringInfo buf, CreateRoleStmt *stmt) break; } } +} + + +/* + * AppendCreateRoleStmt generates the string representation of the + * CreateRoleStmt and appends it to the buffer. + */ +static void +AppendCreateRoleStmt(StringInfo buf, CreateRoleStmt *stmt) +{ + ListCell *optionCell = NULL; + + appendStringInfo(buf, "CREATE "); + + AppendStatementType(buf, stmt); appendStringInfo(buf, "%s", quote_identifier(stmt->role)); foreach(optionCell, stmt->options) { AppendRoleOption(buf, optionCell); - - DefElem *option = (DefElem *) lfirst(optionCell); - - if (strcmp(option->defname, "sysid") == 0) - { - appendStringInfo(buf, " SYSID %d", intVal(option->arg)); - } - else if (strcmp(option->defname, "adminmembers") == 0) - { - appendStringInfo(buf, " ADMIN "); - AppendRoleList(buf, (List *) option->arg); - } - else if (strcmp(option->defname, "rolemembers") == 0) - { - appendStringInfo(buf, " ROLE "); - AppendRoleList(buf, (List *) option->arg); - } - else if (strcmp(option->defname, "addroleto") == 0) - { - appendStringInfo(buf, " IN ROLE "); - AppendRoleList(buf, (List *) option->arg); - } + AppendInlinePriviliges(buf, optionCell); + AppendSysIdStatement(buf, optionCell); } } @@ -326,6 +355,22 @@ AppendRoleList(StringInfo buf, List *roleList) } +char * +DeparseRenameRoleStmt(Node *node) +{ + RenameStmt *stmt = castNode(RenameStmt, node); + StringInfoData str = { 0 }; + initStringInfo(&str); + + Assert(stmt->renameType == OBJECT_ROLE); + + appendStringInfo(&str, "ALTER ROLE %s RENAME TO %s;", + quote_identifier(stmt->subname), quote_identifier(stmt->newname)); + + return str.data; +} + + /* * DeparseGrantRoleStmt builds and returns a string representing of the * GrantRoleStmt for application on a remote server. diff --git a/src/include/distributed/commands.h b/src/include/distributed/commands.h index 429016f9f..43429278f 100644 --- a/src/include/distributed/commands.h +++ b/src/include/distributed/commands.h @@ -479,6 +479,10 @@ extern List * PreprocessRenameAttributeStmt(Node *stmt, const char *queryString, extern List * PostprocessAlterRoleStmt(Node *stmt, const char *queryString); extern List * PreprocessAlterRoleSetStmt(Node *stmt, const char *queryString, ProcessUtilityContext processUtilityContext); + +extern List * PreprocessAlterRoleRenameStmt(Node *stmt, const char *queryString, + ProcessUtilityContext processUtilityContext); + extern List * GenerateAlterRoleSetCommandForRole(Oid roleid); extern List * AlterRoleStmtObjectAddress(Node *node, bool missing_ok, bool isPostprocess); @@ -494,6 +498,10 @@ extern List * PostprocessGrantRoleStmt(Node *stmt, const char *queryString); extern List * GenerateCreateOrAlterRoleCommand(Oid roleOid); extern List * CreateRoleStmtObjectAddress(Node *stmt, bool missing_ok, bool isPostprocess); + +extern List * RenameRoleStmtObjectAddress(Node *stmt, bool missing_ok, bool + isPostprocess); + extern void UnmarkRolesDistributed(List *roles); extern List * FilterDistributedRoles(List *roles); diff --git a/src/include/distributed/deparser.h b/src/include/distributed/deparser.h index 6ffe33cc2..95d948bc9 100644 --- a/src/include/distributed/deparser.h +++ b/src/include/distributed/deparser.h @@ -201,6 +201,7 @@ extern void QualifyAlterFunctionDependsStmt(Node *stmt); /* forward declarations for deparse_role_stmts.c */ extern char * DeparseAlterRoleStmt(Node *stmt); extern char * DeparseAlterRoleSetStmt(Node *stmt); +extern char * DeparseRenameRoleStmt(Node *stmt); extern List * MakeSetStatementArguments(char *configurationName, char *configurationValue); diff --git a/src/test/regress/citus_tests/run_test.py b/src/test/regress/citus_tests/run_test.py index 2b71f5e1b..f1e1ec827 100755 --- a/src/test/regress/citus_tests/run_test.py +++ b/src/test/regress/citus_tests/run_test.py @@ -108,6 +108,9 @@ DEPS = { "minimal_cluster_management": TestDeps( None, ["multi_test_helpers_superuser"], repeatable=False ), + "multi_behavioral_analytics_create_table": TestDeps( + "minimal_schedule", ["multi_test_helpers_superuser"], repeatable=False + ), "create_role_propagation": TestDeps(None, ["multi_cluster_management"]), "single_node_enterprise": TestDeps(None), "single_node": TestDeps(None, ["multi_test_helpers"]), @@ -131,6 +134,7 @@ DEPS = { "alter_distributed_table": TestDeps( "minimal_schedule", ["multi_behavioral_analytics_create_table"] ), + "alter_role_propagation": TestDeps("minimal_schedule"), "background_rebalance": TestDeps( None, [ @@ -149,6 +153,7 @@ DEPS = { ), "function_propagation": TestDeps("minimal_schedule"), "grant_on_foreign_server_propagation": TestDeps("minimal_schedule"), + "multi_modifying_xacts": TestDeps("minimal_schedule"), "multi_mx_modifying_xacts": TestDeps(None, ["multi_mx_create_table"]), "multi_mx_router_planner": TestDeps(None, ["multi_mx_create_table"]), "multi_mx_copy_data": TestDeps(None, ["multi_mx_create_table"]), diff --git a/src/test/regress/expected/alter_role_propagation.out b/src/test/regress/expected/alter_role_propagation.out index 4e04f0e92..82310f477 100644 --- a/src/test/regress/expected/alter_role_propagation.out +++ b/src/test/regress/expected/alter_role_propagation.out @@ -1,7 +1,7 @@ CREATE SCHEMA alter_role; CREATE SCHEMA ",CitUs,.TeeN!?"; -- test if the passowrd of the extension owner can be upgraded -ALTER ROLE CURRENT_USER PASSWORD 'password123' VALID UNTIL 'infinity'; +ALTER ROLE CURRENT_USER CONNECTION LIMIT -1 PASSWORD 'password123' VALID UNTIL 'infinity'; SELECT run_command_on_workers($$SELECT row(rolname, rolsuper, rolinherit, rolcreaterole, rolcreatedb, rolcanlogin, rolreplication, rolbypassrls, rolconnlimit, EXTRACT (year FROM rolvaliduntil)) FROM pg_authid WHERE rolname = current_user$$); run_command_on_workers --------------------------------------------------------------------- @@ -356,5 +356,56 @@ SELECT workers.result AS worker_password, pg_authid.rolpassword AS coord_passwor RESET password_encryption; DROP ROLE new_role; +drop user if exists test1 ; +NOTICE: role "test1" does not exist, skipping +create user test1; +SELECT run_command_on_workers($$SELECT row() FROM pg_roles WHERE rolname = 'test1'$$); + run_command_on_workers +--------------------------------------------------------------------- + (localhost,57637,t,"()") + (localhost,57638,t,"()") +(2 rows) + +alter user test1 with encrypted password 'test1' nosuperuser noinherit nocreaterole nocreatedb nologin noreplication nobypassrls connection limit -1 valid until 'infinity'; +SELECT run_command_on_workers($$SELECT row(rolname, rolsuper, rolinherit, rolcreaterole, rolcreatedb, rolcanlogin, rolreplication, rolbypassrls, rolconnlimit, EXTRACT (year FROM rolvaliduntil)) FROM pg_authid WHERE rolname = 'test1'$$); + run_command_on_workers +--------------------------------------------------------------------- + (localhost,57637,t,"(test1,f,f,f,f,f,f,f,-1,Infinity)") + (localhost,57638,t,"(test1,f,f,f,f,f,f,f,-1,Infinity)") +(2 rows) + +alter user test1 with password NULL superuser inherit createrole createdb login replication bypassrls connection limit 10 valid until '2019-01-01'; +SELECT run_command_on_workers($$SELECT row(rolname, rolsuper, rolinherit, rolcreaterole, rolcreatedb, rolcanlogin, rolreplication, rolbypassrls, rolconnlimit, EXTRACT (year FROM rolvaliduntil)) FROM pg_authid WHERE rolname = 'test1'$$); + run_command_on_workers +--------------------------------------------------------------------- + (localhost,57637,t,"(test1,t,t,t,t,t,t,t,10,2019)") + (localhost,57638,t,"(test1,t,t,t,t,t,t,t,10,2019)") +(2 rows) + +SET citus.enable_alter_role_set_propagation TO on; +SET citus.log_remote_commands = true; +set citus.grep_remote_commands = '%ALTER ROLE%'; +ALTER USER test1 SET timezone TO 'America/New_York'; +NOTICE: issuing ALTER ROLE test1 SET timezone = 'America/New_York' +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing ALTER ROLE test1 SET timezone = 'America/New_York' +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +ALTER USER test1 SET work_mem TO '64MB'; +NOTICE: issuing ALTER ROLE test1 SET work_mem = '64MB' +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing ALTER ROLE test1 SET work_mem = '64MB' +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +ALTER USER test1 SET random_page_cost TO 1.5; +NOTICE: issuing ALTER ROLE test1 SET random_page_cost = 1.5 +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing ALTER ROLE test1 SET random_page_cost = 1.5 +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +alter user test1 rename to test2; +NOTICE: issuing ALTER ROLE test1 RENAME TO test2; +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing ALTER ROLE test1 RENAME TO test2; +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +drop user test2; +SET citus.log_remote_commands = false; DROP TABLE test_search_path; DROP SCHEMA alter_role, ",CitUs,.TeeN!?", test_sp CASCADE; diff --git a/src/test/regress/expected/multi_modifying_xacts.out b/src/test/regress/expected/multi_modifying_xacts.out index 5eba6e21d..7c3344ee5 100644 --- a/src/test/regress/expected/multi_modifying_xacts.out +++ b/src/test/regress/expected/multi_modifying_xacts.out @@ -1200,8 +1200,9 @@ SET citus.override_table_visibility TO false; -- and rename the existing user \c - :default_user - :worker_1_port SET search_path TO multi_modifying_xacts; +set citus.enable_alter_role_propagation=false; ALTER USER test_user RENAME TO test_user_new; -NOTICE: not propagating ALTER ROLE ... RENAME TO commands to worker nodes +set citus.enable_alter_role_propagation=true; -- connect back to master and query the reference table \c - test_user - :master_port SET search_path TO multi_modifying_xacts; @@ -1320,8 +1321,9 @@ WARNING: connection to the remote node localhost:xxxxx failed with the followin -- break the other node as well \c - :default_user - :worker_2_port SET search_path TO multi_modifying_xacts; +set citus.enable_alter_role_propagation=false; ALTER USER test_user RENAME TO test_user_new; -NOTICE: not propagating ALTER ROLE ... RENAME TO commands to worker nodes +set citus.enable_alter_role_propagation=true; \c - test_user - :master_port SET search_path TO multi_modifying_xacts; -- fails on all shard placements @@ -1330,16 +1332,21 @@ ERROR: connection to the remote node localhost:xxxxx failed with the following -- connect back to the master with the proper user to continue the tests \c - :default_user - :master_port SET search_path TO multi_modifying_xacts; +-- unbreak both nodes by renaming the user back to the original name +\c - :default_user - :worker_2_port +SET search_path TO multi_modifying_xacts; +set citus.enable_alter_role_propagation=false; +ALTER USER test_user_new RENAME TO test_user; +set citus.enable_alter_role_propagation=true; +\c - :default_user - :worker_1_port +SET search_path TO multi_modifying_xacts; +set citus.enable_alter_role_propagation=false; +ALTER USER test_user_new RENAME TO test_user; +set citus.enable_alter_role_propagation=true; +\c - :default_user - :master_port +SET search_path TO multi_modifying_xacts; SET citus.next_shard_id TO 1200020; SET citus.next_placement_id TO 1200033; --- unbreak both nodes by renaming the user back to the original name -SELECT * FROM run_command_on_workers('ALTER USER test_user_new RENAME TO test_user'); - nodename | nodeport | success | result ---------------------------------------------------------------------- - localhost | 57637 | t | ALTER ROLE - localhost | 57638 | t | ALTER ROLE -(2 rows) - DROP TABLE reference_modifying_xacts, hash_modifying_xacts, hash_modifying_xacts_second, reference_failure_test, numbers_hash_failure_test; REVOKE ALL ON SCHEMA multi_modifying_xacts FROM test_user; diff --git a/src/test/regress/sql/alter_role_propagation.sql b/src/test/regress/sql/alter_role_propagation.sql index 40c7395c4..658b42e3d 100644 --- a/src/test/regress/sql/alter_role_propagation.sql +++ b/src/test/regress/sql/alter_role_propagation.sql @@ -2,7 +2,7 @@ CREATE SCHEMA alter_role; CREATE SCHEMA ",CitUs,.TeeN!?"; -- test if the passowrd of the extension owner can be upgraded -ALTER ROLE CURRENT_USER PASSWORD 'password123' VALID UNTIL 'infinity'; +ALTER ROLE CURRENT_USER CONNECTION LIMIT -1 PASSWORD 'password123' VALID UNTIL 'infinity'; SELECT run_command_on_workers($$SELECT row(rolname, rolsuper, rolinherit, rolcreaterole, rolcreatedb, rolcanlogin, rolreplication, rolbypassrls, rolconnlimit, EXTRACT (year FROM rolvaliduntil)) FROM pg_authid WHERE rolname = current_user$$); SELECT workers.result = pg_authid.rolpassword AS password_is_same FROM run_command_on_workers($$SELECT rolpassword FROM pg_authid WHERE rolname = current_user$$) workers, pg_authid WHERE pg_authid.rolname = current_user; @@ -79,6 +79,7 @@ SELECT run_command_on_workers('SHOW enable_hashagg'); -- check that ALTER ROLE SET is not propagated when scoped to a different database -- also test case sensitivity CREATE DATABASE "REGRESSION"; + ALTER ROLE CURRENT_USER IN DATABASE "REGRESSION" SET public.myguc TO "Hello from coordinator only"; SELECT d.datname, r.setconfig FROM pg_db_role_setting r LEFT JOIN pg_database d ON r.setdatabase=d.oid WHERE r.setconfig::text LIKE '%Hello from coordinator only%'; SELECT run_command_on_workers($$SELECT json_agg((d.datname, r.setconfig)) FROM pg_db_role_setting r LEFT JOIN pg_database d ON r.setdatabase=d.oid WHERE r.setconfig::text LIKE '%Hello from coordinator only%'$$); @@ -119,5 +120,35 @@ SELECT workers.result AS worker_password, pg_authid.rolpassword AS coord_passwor RESET password_encryption; DROP ROLE new_role; + + +drop user if exists test1 ; + +create user test1; + +SELECT run_command_on_workers($$SELECT row() FROM pg_roles WHERE rolname = 'test1'$$); + + +alter user test1 with encrypted password 'test1' nosuperuser noinherit nocreaterole nocreatedb nologin noreplication nobypassrls connection limit -1 valid until 'infinity'; +SELECT run_command_on_workers($$SELECT row(rolname, rolsuper, rolinherit, rolcreaterole, rolcreatedb, rolcanlogin, rolreplication, rolbypassrls, rolconnlimit, EXTRACT (year FROM rolvaliduntil)) FROM pg_authid WHERE rolname = 'test1'$$); +alter user test1 with password NULL superuser inherit createrole createdb login replication bypassrls connection limit 10 valid until '2019-01-01'; +SELECT run_command_on_workers($$SELECT row(rolname, rolsuper, rolinherit, rolcreaterole, rolcreatedb, rolcanlogin, rolreplication, rolbypassrls, rolconnlimit, EXTRACT (year FROM rolvaliduntil)) FROM pg_authid WHERE rolname = 'test1'$$); + +SET citus.enable_alter_role_set_propagation TO on; +SET citus.log_remote_commands = true; +set citus.grep_remote_commands = '%ALTER ROLE%'; + +ALTER USER test1 SET timezone TO 'America/New_York'; +ALTER USER test1 SET work_mem TO '64MB'; +ALTER USER test1 SET random_page_cost TO 1.5; + + + +alter user test1 rename to test2; + +drop user test2; + +SET citus.log_remote_commands = false; + DROP TABLE test_search_path; DROP SCHEMA alter_role, ",CitUs,.TeeN!?", test_sp CASCADE; diff --git a/src/test/regress/sql/multi_modifying_xacts.sql b/src/test/regress/sql/multi_modifying_xacts.sql index d38f0cc99..8b90d9267 100644 --- a/src/test/regress/sql/multi_modifying_xacts.sql +++ b/src/test/regress/sql/multi_modifying_xacts.sql @@ -981,7 +981,9 @@ SET citus.override_table_visibility TO false; -- and rename the existing user \c - :default_user - :worker_1_port SET search_path TO multi_modifying_xacts; +set citus.enable_alter_role_propagation=false; ALTER USER test_user RENAME TO test_user_new; +set citus.enable_alter_role_propagation=true; -- connect back to master and query the reference table \c - test_user - :master_port @@ -1050,7 +1052,9 @@ SELECT count(*) FROM numbers_hash_failure_test; -- break the other node as well \c - :default_user - :worker_2_port SET search_path TO multi_modifying_xacts; +set citus.enable_alter_role_propagation=false; ALTER USER test_user RENAME TO test_user_new; +set citus.enable_alter_role_propagation=true; \c - test_user - :master_port SET search_path TO multi_modifying_xacts; @@ -1061,10 +1065,25 @@ INSERT INTO numbers_hash_failure_test VALUES (2,2); -- connect back to the master with the proper user to continue the tests \c - :default_user - :master_port SET search_path TO multi_modifying_xacts; + +-- unbreak both nodes by renaming the user back to the original name +\c - :default_user - :worker_2_port +SET search_path TO multi_modifying_xacts; +set citus.enable_alter_role_propagation=false; +ALTER USER test_user_new RENAME TO test_user; +set citus.enable_alter_role_propagation=true; + +\c - :default_user - :worker_1_port +SET search_path TO multi_modifying_xacts; +set citus.enable_alter_role_propagation=false; +ALTER USER test_user_new RENAME TO test_user; +set citus.enable_alter_role_propagation=true; + +\c - :default_user - :master_port +SET search_path TO multi_modifying_xacts; + SET citus.next_shard_id TO 1200020; SET citus.next_placement_id TO 1200033; --- unbreak both nodes by renaming the user back to the original name -SELECT * FROM run_command_on_workers('ALTER USER test_user_new RENAME TO test_user'); DROP TABLE reference_modifying_xacts, hash_modifying_xacts, hash_modifying_xacts_second, reference_failure_test, numbers_hash_failure_test; From b87fbcbf792c46c789b4e77ef15646e91e4390fd Mon Sep 17 00:00:00 2001 From: Nils Dijk Date: Tue, 26 Sep 2023 13:47:50 +0200 Subject: [PATCH 4/4] Shard moves/isolate report LSN's in lsn format (#7227) DESCRIPTION: Shard moves/isolate report LSN's in lsn format While investigating an issue with our catchup mechanism on certain postgres versions we noticed we print LSN's in the format of the native long type. This is an uncommon representation for LSN's in postgres logs. This patch changes the output of our log message to go from the long type representation to the native LSN type representation. Making it easier for postgres users to recognize and compare LSN's with other related reports. example of new output: ``` 2023-09-25 17:28:47.544 CEST [11345] LOG: The LSN of the target subscriptions on node localhost:9701 have increased from 0/0 to 0/E1ED20F8 at 2023-09-25 17:28:47.544165+02 where the source LSN is 1/415DCAD0 ``` --- .../replication/multi_logical_replication.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/backend/distributed/replication/multi_logical_replication.c b/src/backend/distributed/replication/multi_logical_replication.c index c8d0325b6..f66e309ab 100644 --- a/src/backend/distributed/replication/multi_logical_replication.c +++ b/src/backend/distributed/replication/multi_logical_replication.c @@ -1882,14 +1882,15 @@ WaitForGroupedLogicalRepTargetsToCatchUp(XLogRecPtr sourcePosition, GetCurrentTimestamp(), logicalReplicationProgressReportTimeout)) { - ereport(LOG, (errmsg( - "The LSN of the target subscriptions on node %s:%d have " - "increased from %ld to %ld at %s where the source LSN is %ld ", - superuserConnection->hostname, - superuserConnection->port, previousTargetBeforeThisLoop, - targetPosition, - timestamptz_to_str(previousLSNIncrementTime), - sourcePosition))); + ereport(LOG, (errmsg("The LSN of the target subscriptions on node %s:%d " + "has increased from %X/%X to %X/%X at %s where the " + "source LSN is %X/%X ", + superuserConnection->hostname, + superuserConnection->port, + LSN_FORMAT_ARGS(previousTargetBeforeThisLoop), + LSN_FORMAT_ARGS(targetPosition), + timestamptz_to_str(previousLSNIncrementTime), + LSN_FORMAT_ARGS(sourcePosition)))); previousReportTime = GetCurrentTimestamp(); }