From 5ec056a172dd2db61c5d76ae57fb4b4b965cbe3f Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Thu, 18 Jan 2024 13:05:24 +0100 Subject: [PATCH 1/3] Add pytest test example about connecting to a worker (#7386) I noticed while reviewing #7203 that there as no example of executing sql on a worker for the pytest README. Since this is a pretty common thing that people want to do, this PR adds that. --- src/test/regress/citus_tests/test/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/regress/citus_tests/test/README.md b/src/test/regress/citus_tests/test/README.md index 6aac98e49..73435ecf6 100644 --- a/src/test/regress/citus_tests/test/README.md +++ b/src/test/regress/citus_tests/test/README.md @@ -82,6 +82,7 @@ the name of the fixture. For example: ```python def test_some_query(cluster): cluster.coordinator.sql("SELECT 1") + assert cluster.workers[0].sql_value('SELECT 2') == 2 ``` If you need a cluster of a specific size you can use the `cluster_factory` From 188614512f8f1bf7424bc50cc98a837e9c2124ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCrkan=20=C4=B0ndibay?= Date: Thu, 18 Jan 2024 20:58:44 +0300 Subject: [PATCH 2/3] Adds comment on database and role propagation (#7388) DESCRIPTION: Adds comment on database and role propagation. Example commands are as below comment on database is '' comment on database is NULL comment on role is '' comment on role is NULL --------- Co-authored-by: Jelte Fennema-Nio --- src/backend/distributed/commands/comment.c | 131 ++++++++++++++++++ src/backend/distributed/commands/database.c | 30 +++- .../distributed/commands/dependencies.c | 10 +- .../commands/distribute_object_ops.c | 46 +++++- src/backend/distributed/commands/role.c | 12 ++ .../distributed/commands/text_search.c | 39 ------ .../deparser/deparse_comment_stmts.c | 77 ++++++++++ .../deparser/deparse_text_search.c | 62 --------- src/include/distributed/commands.h | 6 +- src/include/distributed/comment.h | 26 ++++ src/include/distributed/deparser.h | 3 + .../regress/expected/comment_on_database.out | 101 ++++++++++++++ src/test/regress/expected/comment_on_role.out | 99 +++++++++++++ src/test/regress/multi_1_schedule | 2 + src/test/regress/sql/comment_on_database.sql | 73 ++++++++++ src/test/regress/sql/comment_on_role.sql | 72 ++++++++++ 16 files changed, 669 insertions(+), 120 deletions(-) create mode 100644 src/backend/distributed/commands/comment.c create mode 100644 src/backend/distributed/deparser/deparse_comment_stmts.c create mode 100644 src/include/distributed/comment.h create mode 100644 src/test/regress/expected/comment_on_database.out create mode 100644 src/test/regress/expected/comment_on_role.out create mode 100644 src/test/regress/sql/comment_on_database.sql create mode 100644 src/test/regress/sql/comment_on_role.sql diff --git a/src/backend/distributed/commands/comment.c b/src/backend/distributed/commands/comment.c new file mode 100644 index 000000000..e18a5c5cc --- /dev/null +++ b/src/backend/distributed/commands/comment.c @@ -0,0 +1,131 @@ +/*------------------------------------------------------------------------- + * + * comment.c + * Commands to interact with the comments for all database + * object types. + * + * Copyright (c) Citus Data, Inc. + * + *------------------------------------------------------------------------- + */ + +#include "postgres.h" + +#include "access/genam.h" +#include "access/htup_details.h" +#include "access/table.h" +#include "catalog/pg_shdescription.h" +#include "nodes/parsenodes.h" +#include "utils/builtins.h" +#include "utils/fmgroids.h" +#include "utils/rel.h" + +#include "distributed/comment.h" + +static char * GetCommentForObject(Oid classOid, Oid objectOid); + + +List * +GetCommentPropagationCommands(Oid classOid, Oid objOoid, char *objectName, ObjectType + objectType) +{ + List *commands = NIL; + + StringInfo commentStmt = makeStringInfo(); + + /* Get the comment for the database */ + char *comment = GetCommentForObject(classOid, objOoid); + char const *commentObjectType = ObjectTypeNames[objectType]; + + /* Create the SQL command to propagate the comment to other nodes */ + if (comment != NULL) + { + appendStringInfo(commentStmt, "COMMENT ON %s %s IS %s;", commentObjectType, + quote_identifier(objectName), + quote_literal_cstr(comment)); + } + + + /* Add the command to the list */ + if (commentStmt->len > 0) + { + commands = list_make1(commentStmt->data); + } + + return commands; +} + + +static char * +GetCommentForObject(Oid classOid, Oid objectOid) +{ + HeapTuple tuple; + char *comment = NULL; + + /* Open pg_shdescription catalog */ + Relation shdescRelation = table_open(SharedDescriptionRelationId, AccessShareLock); + + /* Scan the table */ + ScanKeyData scanKey[2]; + + ScanKeyInit(&scanKey[0], + Anum_pg_shdescription_objoid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(objectOid)); + ScanKeyInit(&scanKey[1], + Anum_pg_shdescription_classoid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(classOid)); + bool indexOk = true; + int scanKeyCount = 2; + SysScanDesc scan = systable_beginscan(shdescRelation, SharedDescriptionObjIndexId, + indexOk, NULL, scanKeyCount, + scanKey); + if ((tuple = systable_getnext(scan)) != NULL) + { + bool isNull = false; + + TupleDesc tupdesc = RelationGetDescr(shdescRelation); + + Datum descDatum = heap_getattr(tuple, Anum_pg_shdescription_description, tupdesc, + &isNull); + + + /* Add the command to the list */ + if (!isNull) + { + comment = TextDatumGetCString(descDatum); + } + else + { + comment = NULL; + } + } + + /* End the scan and close the catalog */ + systable_endscan(scan); + table_close(shdescRelation, AccessShareLock); + + return comment; +} + + +/* + * CommentObjectAddress resolves the ObjectAddress for the object + * on which the comment is placed. Optionally errors if the object does not + * exist based on the missing_ok flag passed in by the caller. + */ +List * +CommentObjectAddress(Node *node, bool missing_ok, bool isPostprocess) +{ + CommentStmt *stmt = castNode(CommentStmt, node); + Relation relation; + + ObjectAddress objectAddress = get_object_address(stmt->objtype, stmt->object, + &relation, AccessExclusiveLock, + missing_ok); + + ObjectAddress *objectAddressCopy = palloc0(sizeof(ObjectAddress)); + *objectAddressCopy = objectAddress; + return list_make1(objectAddressCopy); +} diff --git a/src/backend/distributed/commands/database.c b/src/backend/distributed/commands/database.c index 049af3a64..0eb87ec19 100644 --- a/src/backend/distributed/commands/database.c +++ b/src/backend/distributed/commands/database.c @@ -13,8 +13,10 @@ #include "miscadmin.h" +#include "access/genam.h" #include "access/heapam.h" #include "access/htup_details.h" +#include "access/table.h" #include "access/xact.h" #include "catalog/objectaddress.h" #include "catalog/pg_collation.h" @@ -25,6 +27,7 @@ #include "commands/defrem.h" #include "nodes/parsenodes.h" #include "utils/builtins.h" +#include "utils/fmgroids.h" #include "utils/lsyscache.h" #include "utils/rel.h" #include "utils/relcache.h" @@ -33,6 +36,7 @@ #include "distributed/adaptive_executor.h" #include "distributed/commands.h" #include "distributed/commands/utility_hook.h" +#include "distributed/comment.h" #include "distributed/deparse_shard_query.h" #include "distributed/deparser.h" #include "distributed/listutils.h" @@ -45,7 +49,6 @@ #include "distributed/worker_protocol.h" #include "distributed/worker_transaction.h" - /* * DatabaseCollationInfo is used to store collation related information of a database. */ @@ -672,6 +675,31 @@ GetTablespaceName(Oid tablespaceOid) } +/* + * GetDatabaseMetadataSyncCommands returns a list of sql statements + * for the given database id. The list contains the database ddl command, + * grant commands and comment propagation commands. + */ +List * +GetDatabaseMetadataSyncCommands(Oid dbOid) +{ + char *databaseName = get_database_name(dbOid); + char *databaseDDLCommand = CreateDatabaseDDLCommand(dbOid); + + List *ddlCommands = list_make1(databaseDDLCommand); + + List *grantDDLCommands = GrantOnDatabaseDDLCommands(dbOid); + List *commentDDLCommands = GetCommentPropagationCommands(DatabaseRelationId, dbOid, + databaseName, + OBJECT_DATABASE); + + ddlCommands = list_concat(ddlCommands, grantDDLCommands); + ddlCommands = list_concat(ddlCommands, commentDDLCommands); + + return ddlCommands; +} + + /* * GetDatabaseCollation gets oid of a database and returns all the collation related information * We need this method since collation related info in Form_pg_database is not accessible. diff --git a/src/backend/distributed/commands/dependencies.c b/src/backend/distributed/commands/dependencies.c index 3b3b3cfd6..c7de5d874 100644 --- a/src/backend/distributed/commands/dependencies.c +++ b/src/backend/distributed/commands/dependencies.c @@ -584,15 +584,7 @@ GetDependencyCreateDDLCommands(const ObjectAddress *dependency) */ 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 GetDatabaseMetadataSyncCommands(dependency->objectId); } return NIL; diff --git a/src/backend/distributed/commands/distribute_object_ops.c b/src/backend/distributed/commands/distribute_object_ops.c index fe1f422b6..5a62dd2c8 100644 --- a/src/backend/distributed/commands/distribute_object_ops.c +++ b/src/backend/distributed/commands/distribute_object_ops.c @@ -16,6 +16,7 @@ #include "distributed/commands.h" #include "distributed/commands/utility_hook.h" +#include "distributed/comment.h" #include "distributed/deparser.h" #include "distributed/version_compat.h" @@ -304,6 +305,17 @@ static DistributeObjectOps Any_DropRole = { .address = NULL, .markDistributed = false, }; + +static DistributeObjectOps Role_Comment = { + .deparse = DeparseCommentStmt, + .qualify = NULL, + .preprocess = PreprocessAlterDistributedObjectStmt, + .postprocess = NULL, + .objectType = OBJECT_DATABASE, + .operationType = DIST_OPS_ALTER, + .address = CommentObjectAddress, + .markDistributed = false, +}; static DistributeObjectOps Any_CreateForeignServer = { .deparse = DeparseCreateForeignServerStmt, .qualify = NULL, @@ -533,6 +545,17 @@ static DistributeObjectOps Database_Set = { .markDistributed = false, }; +static DistributeObjectOps Database_Comment = { + .deparse = DeparseCommentStmt, + .qualify = NULL, + .preprocess = PreprocessAlterDistributedObjectStmt, + .postprocess = NULL, + .objectType = OBJECT_DATABASE, + .operationType = DIST_OPS_ALTER, + .address = CommentObjectAddress, + .markDistributed = false, +}; + static DistributeObjectOps Database_Rename = { .deparse = DeparseAlterDatabaseRenameStmt, .qualify = NULL, @@ -972,13 +995,18 @@ static DistributeObjectOps TextSearchConfig_AlterOwner = { .markDistributed = false, }; static DistributeObjectOps TextSearchConfig_Comment = { - .deparse = DeparseTextSearchConfigurationCommentStmt, + .deparse = DeparseCommentStmt, + + /* TODO: When adding new comment types we should create an abstracted + * qualify function, just like we have an abstract deparse + * and adress function + */ .qualify = QualifyTextSearchConfigurationCommentStmt, .preprocess = PreprocessAlterDistributedObjectStmt, .postprocess = NULL, .objectType = OBJECT_TSCONFIGURATION, .operationType = DIST_OPS_ALTER, - .address = TextSearchConfigurationCommentObjectAddress, + .address = CommentObjectAddress, .markDistributed = false, }; static DistributeObjectOps TextSearchConfig_Define = { @@ -1041,13 +1069,13 @@ static DistributeObjectOps TextSearchDict_AlterOwner = { .markDistributed = false, }; static DistributeObjectOps TextSearchDict_Comment = { - .deparse = DeparseTextSearchDictionaryCommentStmt, + .deparse = DeparseCommentStmt, .qualify = QualifyTextSearchDictionaryCommentStmt, .preprocess = PreprocessAlterDistributedObjectStmt, .postprocess = NULL, .objectType = OBJECT_TSDICTIONARY, .operationType = DIST_OPS_ALTER, - .address = TextSearchDictCommentObjectAddress, + .address = CommentObjectAddress, .markDistributed = false, }; static DistributeObjectOps TextSearchDict_Define = { @@ -1780,6 +1808,16 @@ GetDistributeObjectOps(Node *node) return &TextSearchDict_Comment; } + case OBJECT_DATABASE: + { + return &Database_Comment; + } + + case OBJECT_ROLE: + { + return &Role_Comment; + } + default: { return &NoDistributeOps; diff --git a/src/backend/distributed/commands/role.c b/src/backend/distributed/commands/role.c index e6cd1d112..d0b33ccb9 100644 --- a/src/backend/distributed/commands/role.c +++ b/src/backend/distributed/commands/role.c @@ -45,6 +45,7 @@ #include "distributed/citus_safe_lib.h" #include "distributed/commands.h" #include "distributed/commands/utility_hook.h" +#include "distributed/comment.h" #include "distributed/coordinator_protocol.h" #include "distributed/deparser.h" #include "distributed/listutils.h" @@ -582,6 +583,17 @@ GenerateCreateOrAlterRoleCommand(Oid roleOid) { completeRoleList = lappend(completeRoleList, DeparseTreeNode(stmt)); } + + /* + * append COMMENT ON ROLE commands for this specific user + * When we propagate user creation, we also want to make sure that we propagate + * all the comments it has been given. For this, we check pg_shdescription + * for the ROLE entry corresponding to roleOid, and generate the relevant + * Comment stmts to be run in the new node. + */ + List *commentStmts = GetCommentPropagationCommands(AuthIdRelationId, roleOid, + rolename, OBJECT_ROLE); + completeRoleList = list_concat(completeRoleList, commentStmts); } return completeRoleList; diff --git a/src/backend/distributed/commands/text_search.c b/src/backend/distributed/commands/text_search.c index 4a386e321..cce246a73 100644 --- a/src/backend/distributed/commands/text_search.c +++ b/src/backend/distributed/commands/text_search.c @@ -790,45 +790,6 @@ AlterTextSearchDictionarySchemaStmtObjectAddress(Node *node, bool missing_ok, bo } -/* - * TextSearchConfigurationCommentObjectAddress resolves the ObjectAddress for the TEXT - * SEARCH CONFIGURATION on which the comment is placed. Optionally errors if the - * configuration does not exist based on the missing_ok flag passed in by the caller. - */ -List * -TextSearchConfigurationCommentObjectAddress(Node *node, bool missing_ok, bool - isPostprocess) -{ - CommentStmt *stmt = castNode(CommentStmt, node); - Assert(stmt->objtype == OBJECT_TSCONFIGURATION); - - Oid objid = get_ts_config_oid(castNode(List, stmt->object), missing_ok); - - ObjectAddress *address = palloc0(sizeof(ObjectAddress)); - ObjectAddressSet(*address, TSConfigRelationId, objid); - return list_make1(address); -} - - -/* - * TextSearchDictCommentObjectAddress resolves the ObjectAddress for the TEXT SEARCH - * DICTIONARY on which the comment is placed. Optionally errors if the dictionary does not - * exist based on the missing_ok flag passed in by the caller. - */ -List * -TextSearchDictCommentObjectAddress(Node *node, bool missing_ok, bool isPostprocess) -{ - CommentStmt *stmt = castNode(CommentStmt, node); - Assert(stmt->objtype == OBJECT_TSDICTIONARY); - - Oid objid = get_ts_dict_oid(castNode(List, stmt->object), missing_ok); - - ObjectAddress *address = palloc0(sizeof(ObjectAddress)); - ObjectAddressSet(*address, TSDictionaryRelationId, objid); - return list_make1(address); -} - - /* * AlterTextSearchConfigurationOwnerObjectAddress resolves the ObjectAddress for the TEXT * SEARCH CONFIGURATION for which the owner is changed. Optionally errors if the diff --git a/src/backend/distributed/deparser/deparse_comment_stmts.c b/src/backend/distributed/deparser/deparse_comment_stmts.c new file mode 100644 index 000000000..36a63c97b --- /dev/null +++ b/src/backend/distributed/deparser/deparse_comment_stmts.c @@ -0,0 +1,77 @@ +/*------------------------------------------------------------------------- + * + * deparse_coment_stmts.c + * + * All routines to deparse comment statements. + * + * Copyright (c), Citus Data, Inc. + * + *------------------------------------------------------------------------- + */ + +#include "postgres.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 "utils/elog.h" + +#include "pg_version_compat.h" + +#include "distributed/citus_ruleutils.h" +#include "distributed/commands.h" +#include "distributed/comment.h" +#include "distributed/deparser.h" +#include "distributed/listutils.h" +#include "distributed/log_utils.h" + + +const char *ObjectTypeNames[] = +{ + [OBJECT_DATABASE] = "DATABASE", + [OBJECT_ROLE] = "ROLE", + [OBJECT_TSCONFIGURATION] = "TEXT SEARCH CONFIGURATION", + [OBJECT_TSDICTIONARY] = "TEXT SEARCH DICTIONARY", + + /* When support for propagating comments to new objects is introduced, an entry for each + * statement type should be added to this list. The first element in each entry is the 'object_type' keyword + * that will be included in the 'COMMENT ON ..' statement (i.e. DATABASE,). The second element is the type of + * stmt->object, which represents the name of the propagated object. + */ +}; + +char * +DeparseCommentStmt(Node *node) +{ + CommentStmt *stmt = castNode(CommentStmt, node); + StringInfoData str = { 0 }; + initStringInfo(&str); + + const char *objectName = NULL; + if (IsA(stmt->object, String)) + { + objectName = quote_identifier(strVal(stmt->object)); + } + else if (IsA(stmt->object, List)) + { + objectName = NameListToQuotedString(castNode(List, stmt->object)); + } + else + { + ereport(ERROR, + (errcode(ERRCODE_INTERNAL_ERROR), + errmsg("unknown object type"))); + } + + const char *objectType = ObjectTypeNames[stmt->objtype]; + + char *comment = stmt->comment != NULL ? quote_literal_cstr(stmt->comment) : "NULL"; + + + appendStringInfo(&str, "COMMENT ON %s %s IS %s;", objectType, objectName, comment); + + return str.data; +} diff --git a/src/backend/distributed/deparser/deparse_text_search.c b/src/backend/distributed/deparser/deparse_text_search.c index e0c750d0d..ab5498ad8 100644 --- a/src/backend/distributed/deparser/deparse_text_search.c +++ b/src/backend/distributed/deparser/deparse_text_search.c @@ -395,68 +395,6 @@ DeparseAlterTextSearchDictionarySchemaStmt(Node *node) } -/* - * DeparseTextSearchConfigurationCommentStmt returns the sql statement representing - * COMMENT ON TEXT SEARCH CONFIGURATION ... IS ... - */ -char * -DeparseTextSearchConfigurationCommentStmt(Node *node) -{ - CommentStmt *stmt = castNode(CommentStmt, node); - Assert(stmt->objtype == OBJECT_TSCONFIGURATION); - - StringInfoData buf = { 0 }; - initStringInfo(&buf); - - appendStringInfo(&buf, "COMMENT ON TEXT SEARCH CONFIGURATION %s IS ", - NameListToQuotedString(castNode(List, stmt->object))); - - if (stmt->comment == NULL) - { - appendStringInfoString(&buf, "NULL"); - } - else - { - appendStringInfoString(&buf, quote_literal_cstr(stmt->comment)); - } - - appendStringInfoString(&buf, ";"); - - return buf.data; -} - - -/* - * DeparseTextSearchDictionaryCommentStmt returns the sql statement representing - * COMMENT ON TEXT SEARCH DICTIONARY ... IS ... - */ -char * -DeparseTextSearchDictionaryCommentStmt(Node *node) -{ - CommentStmt *stmt = castNode(CommentStmt, node); - Assert(stmt->objtype == OBJECT_TSDICTIONARY); - - StringInfoData buf = { 0 }; - initStringInfo(&buf); - - appendStringInfo(&buf, "COMMENT ON TEXT SEARCH DICTIONARY %s IS ", - NameListToQuotedString(castNode(List, stmt->object))); - - if (stmt->comment == NULL) - { - appendStringInfoString(&buf, "NULL"); - } - else - { - appendStringInfoString(&buf, quote_literal_cstr(stmt->comment)); - } - - appendStringInfoString(&buf, ";"); - - return buf.data; -} - - /* * AppendStringInfoTokentypeList specializes in adding a comma separated list of * token_tyoe's to TEXT SEARCH CONFIGURATION commands diff --git a/src/include/distributed/commands.h b/src/include/distributed/commands.h index 4eb6df8bf..de15553e7 100644 --- a/src/include/distributed/commands.h +++ b/src/include/distributed/commands.h @@ -230,6 +230,7 @@ extern List * PreprocessAlterDatabaseStmt(Node *node, const char *queryString, extern List * PreprocessAlterDatabaseRefreshCollStmt(Node *node, const char *queryString, ProcessUtilityContext processUtilityContext); +extern List * GetDatabaseMetadataSyncCommands(Oid dbOid); extern List * PreprocessAlterDatabaseSetStmt(Node *node, const char *queryString, @@ -693,11 +694,6 @@ extern List * AlterTextSearchConfigurationSchemaStmtObjectAddress(Node *node, extern List * AlterTextSearchDictionarySchemaStmtObjectAddress(Node *node, bool missing_ok, bool isPostprocess); -extern List * TextSearchConfigurationCommentObjectAddress(Node *node, - bool missing_ok, bool - isPostprocess); -extern List * TextSearchDictCommentObjectAddress(Node *node, - bool missing_ok, bool isPostprocess); extern List * AlterTextSearchConfigurationOwnerObjectAddress(Node *node, bool missing_ok, bool isPostprocess); diff --git a/src/include/distributed/comment.h b/src/include/distributed/comment.h new file mode 100644 index 000000000..bef216ae4 --- /dev/null +++ b/src/include/distributed/comment.h @@ -0,0 +1,26 @@ +/*------------------------------------------------------------------------- + * + * comment.h + * Declarations for comment related operations. + * + * Copyright (c) Citus Data, Inc. + * + *------------------------------------------------------------------------- + */ + +#ifndef COMMENT_H +#define COMMENT_H + +#include "postgres.h" + +#include "nodes/parsenodes.h" + + +extern const char *ObjectTypeNames[]; + + +extern List * GetCommentPropagationCommands(Oid classOid, Oid oid, char *objectName, + ObjectType objectType); +extern List * CommentObjectAddress(Node *node, bool missing_ok, bool isPostprocess); + +# endif /* COMMENT_H */ diff --git a/src/include/distributed/deparser.h b/src/include/distributed/deparser.h index 22636b401..437a9fd8e 100644 --- a/src/include/distributed/deparser.h +++ b/src/include/distributed/deparser.h @@ -143,6 +143,9 @@ extern void DefElemOptionToStatement(StringInfo buf, DefElem *option, const DefElemOptionFormat *opt_formats, int opt_formats_len); +/* forward declarations for deparse_comment_stmts.c */ +extern char * DeparseCommentStmt(Node *node); + /* forward declarations for deparse_statistics_stmts.c */ extern char * DeparseCreateStatisticsStmt(Node *node); diff --git a/src/test/regress/expected/comment_on_database.out b/src/test/regress/expected/comment_on_database.out new file mode 100644 index 000000000..a56fe8e03 --- /dev/null +++ b/src/test/regress/expected/comment_on_database.out @@ -0,0 +1,101 @@ +set citus.log_remote_commands to on; +set citus.enable_create_database_propagation to on; +set citus.grep_remote_commands to 'COMMENT ON DATABASE'; +create database "test1-\!escape"; +comment on DATABASE "test1-\!escape" is 'test-comment'; +SELECT result FROM run_command_on_all_nodes( + $$ + SELECT ds.description AS database_comment + FROM pg_database d + LEFT JOIN pg_shdescription ds ON d.oid = ds.objoid + WHERE d.datname = 'test1-\!escape'; + $$ +); + result +--------------------------------------------------------------------- + test-comment + test-comment + test-comment +(3 rows) + +comment on DATABASE "test1-\!escape" is 'comment-needs\!escape'; +SELECT result FROM run_command_on_all_nodes( + $$ + SELECT ds.description AS database_comment + FROM pg_database d + LEFT JOIN pg_shdescription ds ON d.oid = ds.objoid + WHERE d.datname = 'test1-\!escape'; + $$ +); + result +--------------------------------------------------------------------- + comment-needs\!escape + comment-needs\!escape + comment-needs\!escape +(3 rows) + +comment on DATABASE "test1-\!escape" is null; +SELECT result FROM run_command_on_all_nodes( + $$ + SELECT ds.description AS database_comment + FROM pg_database d + LEFT JOIN pg_shdescription ds ON d.oid = ds.objoid + WHERE d.datname = 'test1-\!escape'; + $$ +); + result +--------------------------------------------------------------------- + + + +(3 rows) + +drop DATABASE "test1-\!escape"; +--test metadata sync +select 1 from citus_remove_node('localhost', :worker_2_port); + ?column? +--------------------------------------------------------------------- + 1 +(1 row) + +create database "test1-\!escape"; +comment on DATABASE "test1-\!escape" is 'test-comment'; +SELECT result FROM run_command_on_all_nodes( + $$ + SELECT ds.description AS database_comment + FROM pg_database d + LEFT JOIN pg_shdescription ds ON d.oid = ds.objoid + WHERE d.datname = 'test1-\!escape'; + $$ +); + result +--------------------------------------------------------------------- + test-comment + test-comment +(2 rows) + +select 1 from citus_add_node('localhost', :worker_2_port); + ?column? +--------------------------------------------------------------------- + 1 +(1 row) + +SELECT result FROM run_command_on_all_nodes( + $$ + SELECT ds.description AS database_comment + FROM pg_database d + LEFT JOIN pg_shdescription ds ON d.oid = ds.objoid + WHERE d.datname = 'test1-\!escape'; + $$ +); + result +--------------------------------------------------------------------- + test-comment + test-comment + test-comment +(3 rows) + +drop DATABASE "test1-\!escape"; +reset citus.enable_create_database_propagation; +reset citus.grep_remote_commands; +reset citus.log_remote_commands; diff --git a/src/test/regress/expected/comment_on_role.out b/src/test/regress/expected/comment_on_role.out new file mode 100644 index 000000000..2981195f1 --- /dev/null +++ b/src/test/regress/expected/comment_on_role.out @@ -0,0 +1,99 @@ +set citus.log_remote_commands to on; +set citus.grep_remote_commands to 'COMMENT ON ROLE'; +create role "role1-\!escape"; +comment on ROLE "role1-\!escape" is 'test-comment'; +SELECT result FROM run_command_on_all_nodes( + $$ + SELECT ds.description AS role_comment + FROM pg_roles r + LEFT JOIN pg_shdescription ds ON r.oid = ds.objoid + WHERE r.rolname = 'role1-\!escape'; + $$ +); + result +--------------------------------------------------------------------- + test-comment + test-comment + test-comment +(3 rows) + +comment on role "role1-\!escape" is 'comment-needs\!escape'; +SELECT result FROM run_command_on_all_nodes( + $$ + SELECT ds.description AS role_comment + FROM pg_roles r + LEFT JOIN pg_shdescription ds ON r.oid = ds.objoid + WHERE r.rolname = 'role1-\!escape'; + $$ +); + result +--------------------------------------------------------------------- + comment-needs\!escape + comment-needs\!escape + comment-needs\!escape +(3 rows) + +comment on role "role1-\!escape" is NULL; +SELECT result FROM run_command_on_all_nodes( + $$ + SELECT ds.description AS role_comment + FROM pg_roles r + LEFT JOIN pg_shdescription ds ON r.oid = ds.objoid + WHERE r.rolname = 'role1-\!escape'; + $$ +); + result +--------------------------------------------------------------------- + + + +(3 rows) + +drop role "role1-\!escape"; +--test metadata sync +select 1 from citus_remove_node('localhost', :worker_2_port); + ?column? +--------------------------------------------------------------------- + 1 +(1 row) + +create role "role1-\!escape"; +comment on ROLE "role1-\!escape" is 'test-comment'; +SELECT result FROM run_command_on_all_nodes( + $$ + SELECT ds.description AS role_comment + FROM pg_roles r + LEFT JOIN pg_shdescription ds ON r.oid = ds.objoid + WHERE r.rolname = 'role1-\!escape'; + $$ +); + result +--------------------------------------------------------------------- + test-comment + test-comment +(2 rows) + +select 1 from citus_add_node('localhost', :worker_2_port); + ?column? +--------------------------------------------------------------------- + 1 +(1 row) + +SELECT result FROM run_command_on_all_nodes( + $$ + SELECT ds.description AS role_comment + FROM pg_roles r + LEFT JOIN pg_shdescription ds ON r.oid = ds.objoid + WHERE r.rolname = 'role1-\!escape'; + $$ +); + result +--------------------------------------------------------------------- + test-comment + test-comment + test-comment +(3 rows) + +drop role "role1-\!escape"; +reset citus.grep_remote_commands; +reset citus.log_remote_commands; diff --git a/src/test/regress/multi_1_schedule b/src/test/regress/multi_1_schedule index 2b9fdeb2d..cfff00942 100644 --- a/src/test/regress/multi_1_schedule +++ b/src/test/regress/multi_1_schedule @@ -38,6 +38,8 @@ test: create_single_shard_table test: create_drop_database_propagation test: create_drop_database_propagation_pg15 test: create_drop_database_propagation_pg16 +test: comment_on_database +test: comment_on_role # 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/comment_on_database.sql b/src/test/regress/sql/comment_on_database.sql new file mode 100644 index 000000000..2c5ced81f --- /dev/null +++ b/src/test/regress/sql/comment_on_database.sql @@ -0,0 +1,73 @@ +set citus.log_remote_commands to on; + +set citus.enable_create_database_propagation to on; +set citus.grep_remote_commands to 'COMMENT ON DATABASE'; + +create database "test1-\!escape"; + +comment on DATABASE "test1-\!escape" is 'test-comment'; + +SELECT result FROM run_command_on_all_nodes( + $$ + SELECT ds.description AS database_comment + FROM pg_database d + LEFT JOIN pg_shdescription ds ON d.oid = ds.objoid + WHERE d.datname = 'test1-\!escape'; + $$ +); + +comment on DATABASE "test1-\!escape" is 'comment-needs\!escape'; + +SELECT result FROM run_command_on_all_nodes( + $$ + SELECT ds.description AS database_comment + FROM pg_database d + LEFT JOIN pg_shdescription ds ON d.oid = ds.objoid + WHERE d.datname = 'test1-\!escape'; + $$ +); + +comment on DATABASE "test1-\!escape" is null; + +SELECT result FROM run_command_on_all_nodes( + $$ + SELECT ds.description AS database_comment + FROM pg_database d + LEFT JOIN pg_shdescription ds ON d.oid = ds.objoid + WHERE d.datname = 'test1-\!escape'; + $$ +); + +drop DATABASE "test1-\!escape"; + +--test metadata sync +select 1 from citus_remove_node('localhost', :worker_2_port); +create database "test1-\!escape"; +comment on DATABASE "test1-\!escape" is 'test-comment'; + +SELECT result FROM run_command_on_all_nodes( + $$ + SELECT ds.description AS database_comment + FROM pg_database d + LEFT JOIN pg_shdescription ds ON d.oid = ds.objoid + WHERE d.datname = 'test1-\!escape'; + $$ +); + +select 1 from citus_add_node('localhost', :worker_2_port); + +SELECT result FROM run_command_on_all_nodes( + $$ + SELECT ds.description AS database_comment + FROM pg_database d + LEFT JOIN pg_shdescription ds ON d.oid = ds.objoid + WHERE d.datname = 'test1-\!escape'; + $$ +); + +drop DATABASE "test1-\!escape"; + + +reset citus.enable_create_database_propagation; +reset citus.grep_remote_commands; +reset citus.log_remote_commands; diff --git a/src/test/regress/sql/comment_on_role.sql b/src/test/regress/sql/comment_on_role.sql new file mode 100644 index 000000000..d65d57cca --- /dev/null +++ b/src/test/regress/sql/comment_on_role.sql @@ -0,0 +1,72 @@ +set citus.log_remote_commands to on; + +set citus.grep_remote_commands to 'COMMENT ON ROLE'; + +create role "role1-\!escape"; + +comment on ROLE "role1-\!escape" is 'test-comment'; + +SELECT result FROM run_command_on_all_nodes( + $$ + SELECT ds.description AS role_comment + FROM pg_roles r + LEFT JOIN pg_shdescription ds ON r.oid = ds.objoid + WHERE r.rolname = 'role1-\!escape'; + $$ +); + +comment on role "role1-\!escape" is 'comment-needs\!escape'; + +SELECT result FROM run_command_on_all_nodes( + $$ + SELECT ds.description AS role_comment + FROM pg_roles r + LEFT JOIN pg_shdescription ds ON r.oid = ds.objoid + WHERE r.rolname = 'role1-\!escape'; + $$ +); + +comment on role "role1-\!escape" is NULL; + +SELECT result FROM run_command_on_all_nodes( + $$ + SELECT ds.description AS role_comment + FROM pg_roles r + LEFT JOIN pg_shdescription ds ON r.oid = ds.objoid + WHERE r.rolname = 'role1-\!escape'; + $$ +); + +drop role "role1-\!escape"; + + +--test metadata sync + +select 1 from citus_remove_node('localhost', :worker_2_port); +create role "role1-\!escape"; +comment on ROLE "role1-\!escape" is 'test-comment'; + +SELECT result FROM run_command_on_all_nodes( + $$ + SELECT ds.description AS role_comment + FROM pg_roles r + LEFT JOIN pg_shdescription ds ON r.oid = ds.objoid + WHERE r.rolname = 'role1-\!escape'; + $$ +); + +select 1 from citus_add_node('localhost', :worker_2_port); + +SELECT result FROM run_command_on_all_nodes( + $$ + SELECT ds.description AS role_comment + FROM pg_roles r + LEFT JOIN pg_shdescription ds ON r.oid = ds.objoid + WHERE r.rolname = 'role1-\!escape'; + $$ +); + +drop role "role1-\!escape"; + +reset citus.grep_remote_commands; +reset citus.log_remote_commands; From 14ecebe47cdd21d40edff0fcde09eab37d22cbf3 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Fri, 19 Jan 2024 17:11:29 +0100 Subject: [PATCH 3/3] Fix problems with make check (#7433) This fixes two problems: 1. Allow `make check -j20` to work, by disabling parallelism. This was reported by a user in #7432 2. Actually run all the tests by forwarding to `make check` instead of `check-full`, because confusingly `check-full` does not run all the tests. --- Makefile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index e42d0ffd3..77d8cad3e 100644 --- a/Makefile +++ b/Makefile @@ -61,6 +61,7 @@ check-style: # depend on install-all so that downgrade scripts are installed as well check: all install-all - $(MAKE) -C src/test/regress check-full + # explicetely does not use $(MAKE) to avoid parallelism + make -C src/test/regress check .PHONY: all check clean install install-downgrades install-all