From 77bad0b90e40b5722fdf157495de08e9201b3a2e Mon Sep 17 00:00:00 2001 From: gurkanindibay Date: Mon, 25 Dec 2023 19:40:57 +0300 Subject: [PATCH] Fixes review comments --- src/backend/distributed/commands/database.c | 20 +++++++------------ src/backend/distributed/commands/role.c | 20 +++++++------------ .../deparser/deparse_database_stmts.c | 2 +- .../distributed/deparser/deparse_role_stmts.c | 2 +- src/include/distributed/commands.h | 4 ++-- .../regress/expected/comment_on_database.out | 16 +++++++-------- src/test/regress/sql/comment_on_database.sql | 16 +++++++-------- 7 files changed, 34 insertions(+), 46 deletions(-) diff --git a/src/backend/distributed/commands/database.c b/src/backend/distributed/commands/database.c index 6b66a6e99..d4e3b9474 100644 --- a/src/backend/distributed/commands/database.c +++ b/src/backend/distributed/commands/database.c @@ -712,20 +712,14 @@ List * DatabaseCommentObjectAddress(Node *node, bool missing_ok, bool isPostprocess) { CommentStmt *stmt = castNode(CommentStmt, node); + Relation relation; Assert(stmt->objtype == OBJECT_DATABASE); - char *databaseName = strVal(stmt->object); + ObjectAddress objectAddress = get_object_address(stmt->objtype, stmt->object, + &relation, AccessExclusiveLock, + missing_ok); - Oid objid = get_database_oid(databaseName, missing_ok); - - if (!OidIsValid(objid)) - { - ereport(ERROR, (errmsg("database \"%s\" does not exist", databaseName))); - } - else - { - ObjectAddress *address = palloc0(sizeof(ObjectAddress)); - ObjectAddressSet(*address, DatabaseRelationId, objid); - return list_make1(address); - } + ObjectAddress *objectAddressCopy = palloc0(sizeof(ObjectAddress)); + *objectAddressCopy = objectAddress; + return list_make1(objectAddressCopy); } diff --git a/src/backend/distributed/commands/role.c b/src/backend/distributed/commands/role.c index 45ec33b80..03705cfc2 100644 --- a/src/backend/distributed/commands/role.c +++ b/src/backend/distributed/commands/role.c @@ -1423,20 +1423,14 @@ List * RoleCommentObjectAddress(Node *node, bool missing_ok, bool isPostprocess) { CommentStmt *stmt = castNode(CommentStmt, node); + Relation relation; Assert(stmt->objtype == OBJECT_ROLE); - char *roleName = strVal(stmt->object); + ObjectAddress objectAddress = get_object_address(stmt->objtype, stmt->object, + &relation, AccessExclusiveLock, + missing_ok); - Oid objid = get_role_oid(roleName, missing_ok); - - if (!OidIsValid(objid)) - { - ereport(ERROR, (errmsg("role \"%s\" does not exist", roleName))); - } - else - { - ObjectAddress *address = palloc0(sizeof(ObjectAddress)); - ObjectAddressSet(*address, AuthIdRelationId, objid); - return list_make1(address); - } + ObjectAddress *objectAddressCopy = palloc0(sizeof(ObjectAddress)); + *objectAddressCopy = objectAddress; + return list_make1(objectAddressCopy); } diff --git a/src/backend/distributed/deparser/deparse_database_stmts.c b/src/backend/distributed/deparser/deparse_database_stmts.c index d3ce612fe..78917f731 100644 --- a/src/backend/distributed/deparser/deparse_database_stmts.c +++ b/src/backend/distributed/deparser/deparse_database_stmts.c @@ -321,7 +321,7 @@ DeparseDatabaseCommentStmt(Node *node) StringInfoData str = { 0 }; initStringInfo(&str); - char *databaseName = strVal(stmt->object); + char const *databaseName = quote_identifier(strVal(stmt->object)); char *comment = stmt->comment != NULL ? quote_literal_cstr(stmt->comment) : "NULL"; diff --git a/src/backend/distributed/deparser/deparse_role_stmts.c b/src/backend/distributed/deparser/deparse_role_stmts.c index b8f108ddc..e6cb91977 100644 --- a/src/backend/distributed/deparser/deparse_role_stmts.c +++ b/src/backend/distributed/deparser/deparse_role_stmts.c @@ -542,7 +542,7 @@ DeparseRoleCommentStmt(Node *node) StringInfoData str = { 0 }; initStringInfo(&str); - char *roleName = strVal(stmt->object); + char const *roleName = quote_identifier(strVal(stmt->object)); char *comment = stmt->comment != NULL ? quote_literal_cstr(stmt->comment) : "NULL"; appendStringInfo(&str, "COMMENT ON ROLE %s IS %s;", roleName, comment); diff --git a/src/include/distributed/commands.h b/src/include/distributed/commands.h index 952ce3f06..364d2c9df 100644 --- a/src/include/distributed/commands.h +++ b/src/include/distributed/commands.h @@ -246,8 +246,8 @@ extern List * CreateDatabaseStmtObjectAddress(Node *node, bool missingOk, bool isPostprocess); extern void EnsureSupportedCreateDatabaseCommand(CreatedbStmt *stmt); extern char * CreateDatabaseDDLCommand(Oid dbId); -extern List * DatabaseCommentObjectAddress(Node *node, bool missing_ok, bool - isPostprocess); +extern List * DatabaseCommentObjectAddress(Node *node, bool missing_ok, + bool isPostprocess); /* domain.c - forward declarations */ diff --git a/src/test/regress/expected/comment_on_database.out b/src/test/regress/expected/comment_on_database.out index 0bf0e43ad..89b1617fc 100644 --- a/src/test/regress/expected/comment_on_database.out +++ b/src/test/regress/expected/comment_on_database.out @@ -1,14 +1,14 @@ 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; -comment on DATABASE test1 is 'test-comment'; +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'; + WHERE d.datname = 'test1-\!escape'; $$ ); result @@ -18,13 +18,13 @@ SELECT result FROM run_command_on_all_nodes( test-comment (3 rows) -comment on DATABASE test1 is 'comment-needs\!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'; + WHERE d.datname = 'test1-\!escape'; $$ ); result @@ -34,13 +34,13 @@ SELECT result FROM run_command_on_all_nodes( comment-needs\!escape (3 rows) -comment on DATABASE test1 is null; +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'; + WHERE d.datname = 'test1-\!escape'; $$ ); result @@ -50,7 +50,7 @@ SELECT result FROM run_command_on_all_nodes( (3 rows) -drop DATABASE test1; +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_database.sql b/src/test/regress/sql/comment_on_database.sql index e51fb3448..45f901c1f 100644 --- a/src/test/regress/sql/comment_on_database.sql +++ b/src/test/regress/sql/comment_on_database.sql @@ -3,42 +3,42 @@ 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; +create database "test1-\!escape"; -comment on DATABASE test1 is 'test-comment'; +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'; + WHERE d.datname = 'test1-\!escape'; $$ ); -comment on DATABASE test1 is 'comment-needs\!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'; + WHERE d.datname = 'test1-\!escape'; $$ ); -comment on DATABASE test1 is null; +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'; + WHERE d.datname = 'test1-\!escape'; $$ ); -drop DATABASE test1; +drop DATABASE "test1-\!escape"; reset citus.enable_create_database_propagation; reset citus.grep_remote_commands; reset citus.log_remote_commands;