From f8b3f322aae794128cd808fe8919e5ad490c9fd1 Mon Sep 17 00:00:00 2001 From: gindibay Date: Fri, 10 Nov 2023 08:33:51 +0300 Subject: [PATCH] Fixed review items --- src/backend/distributed/commands/database.c | 46 ++++--------------- .../distributed/commands/utility_hook.c | 1 - .../distributed/deparser/citus_deparseutils.c | 1 - .../deparser/deparse_database_stmts.c | 6 --- src/backend/distributed/metadata/distobject.c | 5 +- .../distributed/metadata/metadata_sync.c | 1 - 6 files changed, 11 insertions(+), 49 deletions(-) diff --git a/src/backend/distributed/commands/database.c b/src/backend/distributed/commands/database.c index 0f4705c21..2d0a2ce16 100644 --- a/src/backend/distributed/commands/database.c +++ b/src/backend/distributed/commands/database.c @@ -298,7 +298,7 @@ PreprocessCreateDatabaseStmt(Node *node, const char *queryString, /* * PostprocessCreatedbStmt is executed after 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 create the database. Since the CREATE DATABASE statement gives error + * all workers to create the database. Since the CREATE DATABASE statement gives error * in a transaction block, we need to use NontransactionalNodeDDLTaskList to send the * CREATE DATABASE statement to the workers. * @@ -388,11 +388,11 @@ GetDatabaseAddressFromDatabaseName(char *databaseName, bool missingOk) * object of the DropdbStmt. */ List * -DropDatabaseStmtObjectAddress(Node *node, bool missing_ok, bool isPostprocess) +DropDatabaseStmtObjectAddress(Node *node, bool missingOk, bool isPostprocess) { DropdbStmt *stmt = castNode(DropdbStmt, node); ObjectAddress *dbAddress = GetDatabaseAddressFromDatabaseName(stmt->dbname, - missing_ok); + missingOk); return list_make1(dbAddress); } @@ -402,11 +402,11 @@ DropDatabaseStmtObjectAddress(Node *node, bool missing_ok, bool isPostprocess) * object of the CreatedbStmt. */ List * -CreateDatabaseStmtObjectAddress(Node *node, bool missing_ok, bool isPostprocess) +CreateDatabaseStmtObjectAddress(Node *node, bool missingOk, bool isPostprocess) { CreatedbStmt *stmt = castNode(CreatedbStmt, node); ObjectAddress *dbAddress = GetDatabaseAddressFromDatabaseName(stmt->dbname, - missing_ok); + missingOk); return list_make1(dbAddress); } @@ -424,7 +424,7 @@ GetTablespaceName(Oid tablespaceOid) } Form_pg_tablespace tablespaceForm = (Form_pg_tablespace) GETSTRUCT(tuple); - char *tablespaceName = NameStr(tablespaceForm->spcname); + char *tablespaceName = pstrdup(NameStr(tablespaceForm->spcname)); ReleaseSysCache(tuple); @@ -437,17 +437,17 @@ GetTablespaceName(Oid tablespaceOid) * We need this method since collation related info in Form_pg_database is not accessible */ static DatabaseCollationInfo -GetDatabaseCollation(Oid db_oid) +GetDatabaseCollation(Oid dbOid) { DatabaseCollationInfo info; bool isNull; Snapshot snapshot = RegisterSnapshot(GetLatestSnapshot()); Relation rel = table_open(DatabaseRelationId, AccessShareLock); - HeapTuple tup = get_catalog_object_by_oid(rel, Anum_pg_database_oid, db_oid); + HeapTuple tup = get_catalog_object_by_oid(rel, Anum_pg_database_oid, dbOid); if (!HeapTupleIsValid(tup)) { - elog(ERROR, "cache lookup failed for database %u", db_oid); + elog(ERROR, "cache lookup failed for database %u", dbOid); } TupleDesc tupdesc = RelationGetDescr(rel); @@ -505,29 +505,6 @@ GetDatabaseCollation(Oid db_oid) } -/* - * FreeDatabaseCollationInfo frees the memory allocated for DatabaseCollationInfo - */ -static void -FreeDatabaseCollationInfo(DatabaseCollationInfo collInfo) -{ - if (collInfo.collation != NULL) - { - pfree(collInfo.collation); - } - if (collInfo.ctype != NULL) - { - pfree(collInfo.ctype); - } - #if PG_VERSION_NUM >= PG_VERSION_15 - if (collInfo.icu_locale != NULL) - { - pfree(collInfo.icu_locale); - } - #endif -} - - #if PG_VERSION_NUM >= PG_VERSION_15 /* @@ -640,8 +617,6 @@ GenerateCreateDatabaseStatementFromPgDatabase(Form_pg_database databaseForm) appendStringInfo(&str, " IS_TEMPLATE = %s", quote_literal_cstr(databaseForm->datistemplate ? "true" : "false")); - FreeDatabaseCollationInfo(collInfo); - return str.data; } @@ -669,7 +644,6 @@ GenerateCreateDatabaseCommandList(void) char *createStmt = GenerateCreateDatabaseStatementFromPgDatabase(databaseForm); - StringInfo outerDbStmt = makeStringInfo(); /* Generate the CREATE DATABASE statement */ @@ -678,8 +652,6 @@ GenerateCreateDatabaseCommandList(void) quote_literal_cstr( createStmt)); - elog(LOG, "outerDbStmt: %s", outerDbStmt->data); - /* Add the statement to the list of commands */ commands = lappend(commands, outerDbStmt->data); } diff --git a/src/backend/distributed/commands/utility_hook.c b/src/backend/distributed/commands/utility_hook.c index eb08b0539..29d7e08da 100644 --- a/src/backend/distributed/commands/utility_hook.c +++ b/src/backend/distributed/commands/utility_hook.c @@ -25,7 +25,6 @@ *------------------------------------------------------------------------- */ - #include "pg_version_constants.h" #include "postgres.h" diff --git a/src/backend/distributed/deparser/citus_deparseutils.c b/src/backend/distributed/deparser/citus_deparseutils.c index 1fae9407f..3f83f57fd 100644 --- a/src/backend/distributed/deparser/citus_deparseutils.c +++ b/src/backend/distributed/deparser/citus_deparseutils.c @@ -14,7 +14,6 @@ #include "postgres.h" - #include "commands/defrem.h" #include "distributed/deparser.h" #include "utils/builtins.h" diff --git a/src/backend/distributed/deparser/deparse_database_stmts.c b/src/backend/distributed/deparser/deparse_database_stmts.c index 494bfb30c..bba6bedb4 100644 --- a/src/backend/distributed/deparser/deparse_database_stmts.c +++ b/src/backend/distributed/deparser/deparse_database_stmts.c @@ -275,9 +275,6 @@ ValidateCreateDatabaseOptions(DefElem *option) } -/* - * Prepares a CREATE DATABASE statement with given empty StringInfo buffer and CreatedbStmt node. - */ static void AppendCreateDatabaseStmt(StringInfo buf, CreatedbStmt *stmt) { @@ -314,9 +311,6 @@ DeparseCreateDatabaseStmt(Node *node) } -/* - * Prepares a DROP DATABASE statement with given empty StringInfo buffer and DropdbStmt node. - */ static void AppendDropDatabaseStmt(StringInfo buf, DropdbStmt *stmt) { diff --git a/src/backend/distributed/metadata/distobject.c b/src/backend/distributed/metadata/distobject.c index 5350a08bc..94c12d47f 100644 --- a/src/backend/distributed/metadata/distobject.c +++ b/src/backend/distributed/metadata/distobject.c @@ -22,11 +22,13 @@ #include "catalog/dependency.h" #include "catalog/namespace.h" #include "catalog/objectaddress.h" +#include "catalog/pg_database.h" #include "catalog/pg_extension_d.h" #include "catalog/pg_namespace.h" #include "catalog/pg_proc.h" #include "catalog/pg_type.h" #include "citus_version.h" +#include "commands/dbcommands.h" #include "commands/extension.h" #include "distributed/listutils.h" #include "distributed/colocation_utils.h" @@ -48,9 +50,6 @@ #include "utils/lsyscache.h" #include "utils/regproc.h" #include "utils/rel.h" -#include "catalog/pg_database.h" -#include "commands/dbcommands.h" - static char * CreatePgDistObjectEntryCommand(const ObjectAddress *objectAddress); static int ExecuteCommandAsSuperuser(char *query, int paramCount, Oid *paramTypes, diff --git a/src/backend/distributed/metadata/metadata_sync.c b/src/backend/distributed/metadata/metadata_sync.c index dfcf243bf..e612a468a 100644 --- a/src/backend/distributed/metadata/metadata_sync.c +++ b/src/backend/distributed/metadata/metadata_sync.c @@ -3951,7 +3951,6 @@ citus_internal_database_command(PG_FUNCTION_ARGS) bool missingOk = false; Oid databaseOid = get_database_oid(stmt->dbname, missingOk); - if (OidIsValid(databaseOid)) { DropDatabase(pstate, (DropdbStmt *) parseTree);