diff --git a/.github/workflows/packaging-test-pipelines.yml b/.github/workflows/packaging-test-pipelines.yml index c66c5b4f7..356a590a4 100644 --- a/.github/workflows/packaging-test-pipelines.yml +++ b/.github/workflows/packaging-test-pipelines.yml @@ -125,7 +125,6 @@ jobs: - debian-bullseye-all - ubuntu-focal-all - ubuntu-jammy-all - - ubuntu-kinetic-all POSTGRES_VERSION: ${{ fromJson(needs.get_postgres_versions_from_file.outputs.pg_versions) }} diff --git a/src/backend/distributed/commands/database.c b/src/backend/distributed/commands/database.c index 71a726b91..6d0c7e95c 100644 --- a/src/backend/distributed/commands/database.c +++ b/src/backend/distributed/commands/database.c @@ -213,6 +213,7 @@ PreprocessAlterDatabaseRefreshCollStmt(Node *node, const char *queryString, #endif + /* * PreprocessAlterDatabaseSetStmt is executed before the statement is applied to the local * postgres instance. @@ -241,3 +242,4 @@ PreprocessAlterDatabaseSetStmt(Node *node, const char *queryString, return NodeDDLTaskList(NON_COORDINATOR_NODES, commands); } + diff --git a/src/backend/distributed/commands/distribute_object_ops.c b/src/backend/distributed/commands/distribute_object_ops.c index be34de2c2..a17d75e17 100644 --- a/src/backend/distributed/commands/distribute_object_ops.c +++ b/src/backend/distributed/commands/distribute_object_ops.c @@ -479,6 +479,7 @@ static DistributeObjectOps Database_Set = { .markDistributed = false, }; + static DistributeObjectOps Domain_Alter = { .deparse = DeparseAlterDomainStmt, .qualify = QualifyAlterDomainStmt, @@ -1335,6 +1336,7 @@ GetDistributeObjectOps(Node *node) return &Database_Set; } + case T_AlterDomainStmt: { return &Domain_Alter; diff --git a/src/backend/distributed/commands/role.c b/src/backend/distributed/commands/role.c index 99ce4fb9f..63ede986d 100644 --- a/src/backend/distributed/commands/role.c +++ b/src/backend/distributed/commands/role.c @@ -78,6 +78,7 @@ static const char * WrapQueryInAlterRoleIfExistsCall(const char *query, RoleSpec static VariableSetStmt * MakeVariableSetStmt(const char *config); static int ConfigGenericNameCompare(const void *lhs, const void *rhs); static List * RoleSpecToObjectAddress(RoleSpec *role, bool missing_ok); +static bool IsGrantRoleWithInheritOrSetOption(GrantRoleStmt *stmt); /* controlled via GUC */ bool EnableCreateRolePropagation = true; @@ -1141,6 +1142,19 @@ PreprocessGrantRoleStmt(Node *node, const char *queryString, return NIL; } + if (IsGrantRoleWithInheritOrSetOption(stmt)) + { + if (EnableUnsupportedFeatureMessages) + { + ereport(NOTICE, (errmsg("not propagating GRANT/REVOKE commands with specified" + " INHERIT/SET options to worker nodes"), + errhint( + "Connect to worker nodes directly to manually run the same" + " GRANT/REVOKE command after disabling DDL propagation."))); + } + return NIL; + } + /* * Postgres don't seem to use the grantor. Even dropping the grantor doesn't * seem to affect the membership. If this changes, we might need to add grantors @@ -1190,6 +1204,27 @@ PostprocessGrantRoleStmt(Node *node, const char *queryString) } +/* + * IsGrantRoleWithInheritOrSetOption returns true if the given + * GrantRoleStmt has inherit or set option specified in its options + */ +static bool +IsGrantRoleWithInheritOrSetOption(GrantRoleStmt *stmt) +{ +#if PG_VERSION_NUM >= PG_VERSION_16 + DefElem *opt = NULL; + foreach_ptr(opt, stmt->opt) + { + if (strcmp(opt->defname, "inherit") == 0 || strcmp(opt->defname, "set") == 0) + { + return true; + } + } +#endif + return false; +} + + /* * ConfigGenericNameCompare compares two config_generic structs based on their * name fields. If the name fields contain the same strings two structs are diff --git a/src/backend/distributed/deparser/deparse_database_stmts.c b/src/backend/distributed/deparser/deparse_database_stmts.c index 9bee13d61..aa1e2e51d 100644 --- a/src/backend/distributed/deparser/deparse_database_stmts.c +++ b/src/backend/distributed/deparser/deparse_database_stmts.c @@ -25,6 +25,7 @@ #include "distributed/log_utils.h" #include "parser/parse_type.h" + static void AppendAlterDatabaseOwnerStmt(StringInfo buf, AlterOwnerStmt *stmt); static void AppendAlterDatabaseStmt(StringInfo buf, AlterDatabaseStmt *stmt); @@ -85,16 +86,6 @@ AppendGrantOnDatabaseStmt(StringInfo buf, GrantStmt *stmt) AppendGrantSharedSuffix(buf, stmt); } - -static void -AppendDefElemIsTemplate(StringInfo buf, DefElem *def) -{ - appendStringInfo(buf, " %s %s", quote_identifier(def->defname), - quote_literal_cstr(strVal(def->arg))); -} - - -static void AppendDefElemConnLimit(StringInfo buf, DefElem *def) { appendStringInfo(buf, " CONNECTION LIMIT %ld", (long int) defGetNumeric(def)); @@ -109,13 +100,14 @@ AppendAlterDatabaseStmt(StringInfo buf, AlterDatabaseStmt *stmt) if (stmt->options) { ListCell *cell = NULL; - appendStringInfo(buf, "WITH"); + appendStringInfo(buf, "WITH "); foreach(cell, stmt->options) { DefElem *def = castNode(DefElem, lfirst(cell)); if (strcmp(def->defname, "is_template") == 0) { - AppendDefElemIsTemplate(buf, def); + appendStringInfo(buf, "IS_TEMPLATE %s", + quote_literal_cstr(strVal(def->arg))); } else if (strcmp(def->defname, "connection_limit") == 0) { @@ -187,7 +179,6 @@ DeparseAlterDatabaseRefreshCollStmt(Node *node) #endif - static void AppendAlterDatabaseSetStmt(StringInfo buf, AlterDatabaseSetStmt *stmt) { @@ -211,3 +202,4 @@ DeparseAlterDatabaseSetStmt(Node *node) return str.data; } + diff --git a/src/include/distributed/deparser.h b/src/include/distributed/deparser.h index 35d372ec5..6ffe33cc2 100644 --- a/src/include/distributed/deparser.h +++ b/src/include/distributed/deparser.h @@ -227,6 +227,7 @@ extern char * DeparseAlterDatabaseStmt(Node *node); extern char * DeparseAlterDatabaseRefreshCollStmt(Node *node); extern char * DeparseAlterDatabaseSetStmt(Node *node); + /* forward declaration for deparse_publication_stmts.c */ extern char * DeparseCreatePublicationStmt(Node *stmt); extern char * DeparseCreatePublicationStmtExtended(Node *node, diff --git a/src/test/regress/expected/alter_database_propagation.out b/src/test/regress/expected/alter_database_propagation.out index 2ab88dcc9..d1edb5b44 100644 --- a/src/test/regress/expected/alter_database_propagation.out +++ b/src/test/regress/expected/alter_database_propagation.out @@ -6,29 +6,30 @@ set citus.grep_remote_commands = '%ALTER DATABASE%'; alter database regression ALLOW_CONNECTIONS false; ERROR: ALLOW_CONNECTIONS is not supported alter database regression with CONNECTION LIMIT 100; -NOTICE: issuing ALTER DATABASE regression WITH CONNECTION LIMIT 100; + +NOTICE: issuing ALTER DATABASE regression WITH CONNECTION LIMIT 100; DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx -NOTICE: issuing ALTER DATABASE regression WITH CONNECTION LIMIT 100; +NOTICE: issuing ALTER DATABASE regression WITH CONNECTION LIMIT 100; DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx alter database regression with IS_TEMPLATE true CONNECTION LIMIT 50; -NOTICE: issuing ALTER DATABASE regression WITH is_template 'true' CONNECTION LIMIT 50; +NOTICE: issuing ALTER DATABASE regression WITH IS_TEMPLATE 'true' CONNECTION LIMIT 50; DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx -NOTICE: issuing ALTER DATABASE regression WITH is_template 'true' CONNECTION LIMIT 50; +NOTICE: issuing ALTER DATABASE regression WITH IS_TEMPLATE 'true' CONNECTION LIMIT 50; DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx alter database regression with CONNECTION LIMIT -1; -NOTICE: issuing ALTER DATABASE regression WITH CONNECTION LIMIT -1; +NOTICE: issuing ALTER DATABASE regression WITH CONNECTION LIMIT -1; DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx -NOTICE: issuing ALTER DATABASE regression WITH CONNECTION LIMIT -1; +NOTICE: issuing ALTER DATABASE regression WITH CONNECTION LIMIT -1; DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx alter database regression with IS_TEMPLATE true; -NOTICE: issuing ALTER DATABASE regression WITH is_template 'true'; +NOTICE: issuing ALTER DATABASE regression WITH IS_TEMPLATE 'true'; DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx -NOTICE: issuing ALTER DATABASE regression WITH is_template 'true'; +NOTICE: issuing ALTER DATABASE regression WITH IS_TEMPLATE 'true'; DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx alter database regression with IS_TEMPLATE false; -NOTICE: issuing ALTER DATABASE regression WITH is_template 'false'; +NOTICE: issuing ALTER DATABASE regression WITH IS_TEMPLATE 'false'; DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx -NOTICE: issuing ALTER DATABASE regression WITH is_template 'false'; +NOTICE: issuing ALTER DATABASE regression WITH IS_TEMPLATE 'false'; DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx -- this statement will get error since we don't have a multiple database support for now alter database regression rename to regression2; diff --git a/src/test/regress/expected/pg16.out b/src/test/regress/expected/pg16.out index 4d1b9bda8..8d47b6f1b 100644 --- a/src/test/regress/expected/pg16.out +++ b/src/test/regress/expected/pg16.out @@ -1009,6 +1009,95 @@ DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx REVOKE role1 FROM role2; RESET citus.log_remote_commands; RESET citus.grep_remote_commands; +-- +-- PG16 added new options to GRANT ROLE +-- inherit: https://github.com/postgres/postgres/commit/e3ce2de +-- set: https://github.com/postgres/postgres/commit/3d14e17 +-- We don't propagate for now in Citus +-- +GRANT role1 TO role2 WITH INHERIT FALSE; +NOTICE: not propagating GRANT/REVOKE commands with specified INHERIT/SET options to worker nodes +HINT: Connect to worker nodes directly to manually run the same GRANT/REVOKE command after disabling DDL propagation. +REVOKE role1 FROM role2; +GRANT role1 TO role2 WITH INHERIT TRUE; +NOTICE: not propagating GRANT/REVOKE commands with specified INHERIT/SET options to worker nodes +HINT: Connect to worker nodes directly to manually run the same GRANT/REVOKE command after disabling DDL propagation. +REVOKE role1 FROM role2; +GRANT role1 TO role2 WITH INHERIT OPTION; +NOTICE: not propagating GRANT/REVOKE commands with specified INHERIT/SET options to worker nodes +HINT: Connect to worker nodes directly to manually run the same GRANT/REVOKE command after disabling DDL propagation. +REVOKE role1 FROM role2; +GRANT role1 TO role2 WITH SET FALSE; +NOTICE: not propagating GRANT/REVOKE commands with specified INHERIT/SET options to worker nodes +HINT: Connect to worker nodes directly to manually run the same GRANT/REVOKE command after disabling DDL propagation. +REVOKE role1 FROM role2; +GRANT role1 TO role2 WITH SET TRUE; +NOTICE: not propagating GRANT/REVOKE commands with specified INHERIT/SET options to worker nodes +HINT: Connect to worker nodes directly to manually run the same GRANT/REVOKE command after disabling DDL propagation. +REVOKE role1 FROM role2; +GRANT role1 TO role2 WITH SET OPTION; +NOTICE: not propagating GRANT/REVOKE commands with specified INHERIT/SET options to worker nodes +HINT: Connect to worker nodes directly to manually run the same GRANT/REVOKE command after disabling DDL propagation. +REVOKE role1 FROM role2; +-- connect to worker node +GRANT role1 TO role2 WITH ADMIN OPTION, INHERIT FALSE, SET FALSE; +NOTICE: not propagating GRANT/REVOKE commands with specified INHERIT/SET options to worker nodes +HINT: Connect to worker nodes directly to manually run the same GRANT/REVOKE command after disabling DDL propagation. +SELECT roleid::regrole::text AS role, member::regrole::text, +admin_option, inherit_option, set_option FROM pg_auth_members +WHERE roleid::regrole::text = 'role1' ORDER BY 1, 2; + role | member | admin_option | inherit_option | set_option +--------------------------------------------------------------------- + role1 | role2 | t | f | f +(1 row) + +\c - - - :worker_1_port +SELECT roleid::regrole::text AS role, member::regrole::text, +admin_option, inherit_option, set_option FROM pg_auth_members +WHERE roleid::regrole::text = 'role1' ORDER BY 1, 2; + role | member | admin_option | inherit_option | set_option +--------------------------------------------------------------------- +(0 rows) + +SET citus.enable_ddl_propagation TO off; +GRANT role1 TO role2 WITH ADMIN OPTION, INHERIT FALSE, SET FALSE; +RESET citus.enable_ddl_propagation; +SELECT roleid::regrole::text AS role, member::regrole::text, +admin_option, inherit_option, set_option FROM pg_auth_members +WHERE roleid::regrole::text = 'role1' ORDER BY 1, 2; + role | member | admin_option | inherit_option | set_option +--------------------------------------------------------------------- + role1 | role2 | t | f | f +(1 row) + +\c - - - :master_port +REVOKE role1 FROM role2; +-- test REVOKES as well +GRANT role1 TO role2; +REVOKE SET OPTION FOR role1 FROM role2; +NOTICE: not propagating GRANT/REVOKE commands with specified INHERIT/SET options to worker nodes +HINT: Connect to worker nodes directly to manually run the same GRANT/REVOKE command after disabling DDL propagation. +REVOKE INHERIT OPTION FOR role1 FROM role2; +NOTICE: not propagating GRANT/REVOKE commands with specified INHERIT/SET options to worker nodes +HINT: Connect to worker nodes directly to manually run the same GRANT/REVOKE command after disabling DDL propagation. +DROP ROLE role1, role2; +-- test that everything works fine for roles that are not propagated +SET citus.enable_ddl_propagation TO off; +CREATE ROLE role3; +CREATE ROLE role4; +CREATE ROLE role5; +RESET citus.enable_ddl_propagation; +-- by default, admin option is false, inherit is true, set is true +GRANT role3 TO role4; +GRANT role3 TO role5 WITH ADMIN TRUE, INHERIT FALSE, SET FALSE; +SELECT roleid::regrole::text AS role, member::regrole::text, admin_option, inherit_option, set_option FROM pg_auth_members WHERE roleid::regrole::text = 'role3' ORDER BY 1, 2; + role | member | admin_option | inherit_option | set_option +--------------------------------------------------------------------- + role3 | role4 | f | t | t + role3 | role5 | t | f | f +(2 rows) + +DROP ROLE role3, role4, role5; \set VERBOSITY terse SET client_min_messages TO ERROR; DROP EXTENSION postgres_fdw CASCADE; diff --git a/src/test/regress/sql/alter_database_propagation.sql b/src/test/regress/sql/alter_database_propagation.sql index e5b363130..2b9d3ac33 100644 --- a/src/test/regress/sql/alter_database_propagation.sql +++ b/src/test/regress/sql/alter_database_propagation.sql @@ -1,7 +1,8 @@ set citus.log_remote_commands = true; set citus.grep_remote_commands = '%ALTER DATABASE%'; ---since ALLOW_CONNECTIONS alter option should be executed in a different database + +-- since ALLOW_CONNECTIONS alter option should be executed in a different database -- and since we don't have a multiple database support for now, -- this statement will get error alter database regression ALLOW_CONNECTIONS false; @@ -55,13 +56,4 @@ alter database regression set lock_timeout from current; alter database regression set lock_timeout to DEFAULT; alter database regression RESET lock_timeout; - - - - - - - - - set citus.log_remote_commands = false; diff --git a/src/test/regress/sql/pg16.sql b/src/test/regress/sql/pg16.sql index fb18a1b58..82e9edf1e 100644 --- a/src/test/regress/sql/pg16.sql +++ b/src/test/regress/sql/pg16.sql @@ -591,6 +591,69 @@ REVOKE role1 FROM role2; RESET citus.log_remote_commands; RESET citus.grep_remote_commands; +-- +-- PG16 added new options to GRANT ROLE +-- inherit: https://github.com/postgres/postgres/commit/e3ce2de +-- set: https://github.com/postgres/postgres/commit/3d14e17 +-- We don't propagate for now in Citus +-- +GRANT role1 TO role2 WITH INHERIT FALSE; +REVOKE role1 FROM role2; +GRANT role1 TO role2 WITH INHERIT TRUE; +REVOKE role1 FROM role2; +GRANT role1 TO role2 WITH INHERIT OPTION; +REVOKE role1 FROM role2; +GRANT role1 TO role2 WITH SET FALSE; +REVOKE role1 FROM role2; +GRANT role1 TO role2 WITH SET TRUE; +REVOKE role1 FROM role2; +GRANT role1 TO role2 WITH SET OPTION; +REVOKE role1 FROM role2; + +-- connect to worker node +GRANT role1 TO role2 WITH ADMIN OPTION, INHERIT FALSE, SET FALSE; + +SELECT roleid::regrole::text AS role, member::regrole::text, +admin_option, inherit_option, set_option FROM pg_auth_members +WHERE roleid::regrole::text = 'role1' ORDER BY 1, 2; + +\c - - - :worker_1_port + +SELECT roleid::regrole::text AS role, member::regrole::text, +admin_option, inherit_option, set_option FROM pg_auth_members +WHERE roleid::regrole::text = 'role1' ORDER BY 1, 2; + +SET citus.enable_ddl_propagation TO off; +GRANT role1 TO role2 WITH ADMIN OPTION, INHERIT FALSE, SET FALSE; +RESET citus.enable_ddl_propagation; + +SELECT roleid::regrole::text AS role, member::regrole::text, +admin_option, inherit_option, set_option FROM pg_auth_members +WHERE roleid::regrole::text = 'role1' ORDER BY 1, 2; + +\c - - - :master_port +REVOKE role1 FROM role2; + +-- test REVOKES as well +GRANT role1 TO role2; +REVOKE SET OPTION FOR role1 FROM role2; +REVOKE INHERIT OPTION FOR role1 FROM role2; + +DROP ROLE role1, role2; + +-- test that everything works fine for roles that are not propagated +SET citus.enable_ddl_propagation TO off; +CREATE ROLE role3; +CREATE ROLE role4; +CREATE ROLE role5; +RESET citus.enable_ddl_propagation; +-- by default, admin option is false, inherit is true, set is true +GRANT role3 TO role4; +GRANT role3 TO role5 WITH ADMIN TRUE, INHERIT FALSE, SET FALSE; +SELECT roleid::regrole::text AS role, member::regrole::text, admin_option, inherit_option, set_option FROM pg_auth_members WHERE roleid::regrole::text = 'role3' ORDER BY 1, 2; + +DROP ROLE role3, role4, role5; + \set VERBOSITY terse SET client_min_messages TO ERROR; DROP EXTENSION postgres_fdw CASCADE;