From 3bdecb10a0aa29b0db6fb202c15c2fc416936800 Mon Sep 17 00:00:00 2001 From: Onur Tirtir Date: Wed, 28 Feb 2024 18:44:49 +0300 Subject: [PATCH] phase3: generalize the fact that some commands cannot be exec in a xact --- .../non_main_db_distribute_object_ops.c | 130 +++++++++++------- src/include/distributed/commands.h | 1 - 2 files changed, 80 insertions(+), 51 deletions(-) diff --git a/src/backend/distributed/commands/non_main_db_distribute_object_ops.c b/src/backend/distributed/commands/non_main_db_distribute_object_ops.c index c2fc3891b..cb0d7ddfc 100644 --- a/src/backend/distributed/commands/non_main_db_distribute_object_ops.c +++ b/src/backend/distributed/commands/non_main_db_distribute_object_ops.c @@ -62,7 +62,7 @@ typedef enum DistObjectOperation typedef struct NonMainDbDistributedStatementInfo { int statementType; - DistObjectOperation DistObjectOperation; + DistObjectOperation distObjectOperation; /* * checkSupportedObjectTypes is a callback function that checks whether @@ -71,6 +71,9 @@ typedef struct NonMainDbDistributedStatementInfo * Can be NULL if not applicable for the statement type. */ bool (*checkSupportedObjectTypes)(Node *node); + + /* indicates whether the statement cannot be executed in a transaction */ + bool cannotBeExecutedInTransaction; } NonMainDbDistributedStatementInfo; /* @@ -90,6 +93,8 @@ typedef struct DistObjectOperationParams * checkSupportedObjectTypes callbacks for * NonMainDbDistributedStatementInfo objects. */ +static bool NonMainDbCheckSupportedObjectTypeForCreateDatabase(Node *node); +static bool NonMainDbCheckSupportedObjectTypeForDropDatabase(Node *node); static bool NonMainDbCheckSupportedObjectTypeForGrant(Node *node); static bool NonMainDbCheckSupportedObjectTypeForSecLabel(Node *node); @@ -99,49 +104,30 @@ static bool NonMainDbCheckSupportedObjectTypeForSecLabel(Node *node); */ ObjectType supportedObjectTypesForGrantStmt[] = { OBJECT_DATABASE }; static const NonMainDbDistributedStatementInfo NonMainDbSupportedStatements[] = { - { T_GrantRoleStmt, NO_DIST_OBJECT_OPERATION, NULL }, - { T_CreateRoleStmt, MARK_DISTRIBUTED_GLOBALLY, NULL }, - { T_DropRoleStmt, UNMARK_DISTRIBUTED_LOCALLY, NULL }, - { T_AlterRoleStmt, NO_DIST_OBJECT_OPERATION, NULL }, - { T_GrantStmt, NO_DIST_OBJECT_OPERATION, NonMainDbCheckSupportedObjectTypeForGrant }, - { T_CreatedbStmt, NO_DIST_OBJECT_OPERATION, NULL }, - { T_DropdbStmt, NO_DIST_OBJECT_OPERATION, NULL }, + { T_GrantRoleStmt, NO_DIST_OBJECT_OPERATION, NULL, false }, + { T_CreateRoleStmt, MARK_DISTRIBUTED_GLOBALLY, NULL, false }, + { T_DropRoleStmt, UNMARK_DISTRIBUTED_LOCALLY, NULL, false }, + { T_AlterRoleStmt, NO_DIST_OBJECT_OPERATION, NULL, false }, + { T_GrantStmt, NO_DIST_OBJECT_OPERATION, + NonMainDbCheckSupportedObjectTypeForGrant, false }, + { T_CreatedbStmt, NO_DIST_OBJECT_OPERATION, + NonMainDbCheckSupportedObjectTypeForCreateDatabase, true }, + { T_DropdbStmt, NO_DIST_OBJECT_OPERATION, + NonMainDbCheckSupportedObjectTypeForDropDatabase, true }, { T_SecLabelStmt, NO_DIST_OBJECT_OPERATION, - NonMainDbCheckSupportedObjectTypeForSecLabel }, + NonMainDbCheckSupportedObjectTypeForSecLabel, false }, }; static bool IsStatementSupportedFromNonMainDb(Node *parsetree); static bool StatementRequiresMarkDistributedGloballyFromNonMainDb(Node *parsetree); static bool StatementRequiresUnmarkDistributedLocallyFromNonMainDb(Node *parsetree); +static bool StatementCannotBeExecutedInTransaction(Node *parsetree); static void MarkObjectDistributedGloballyFromNonMainDb(Node *parsetree); static void UnMarkObjectDistributedLocallyFromNonMainDb(List *unmarkDistributedList); static List * GetDistObjectOperationParams(Node *parsetree); -/* - * IsCommandToCreateOrDropMainDB checks if this query creates or drops the - * main database, so we can make an exception and not send this query to - * the main database. - */ -bool -IsCommandToCreateOrDropMainDB(Node *parsetree) -{ - if (IsA(parsetree, CreatedbStmt)) - { - CreatedbStmt *createdbStmt = castNode(CreatedbStmt, parsetree); - return strcmp(createdbStmt->dbname, MainDb) == 0; - } - else if (IsA(parsetree, DropdbStmt)) - { - DropdbStmt *dropdbStmt = castNode(DropdbStmt, parsetree); - return strcmp(dropdbStmt->dbname, MainDb) == 0; - } - - return false; -} - - /* * RunPreprocessNonMainDBCommand runs the necessary commands for a query, in main * database before query is run on the local node with PrevProcessUtility. @@ -157,25 +143,15 @@ RunPreprocessNonMainDBCommand(Node *parsetree) return false; } - if (IsCommandToCreateOrDropMainDB(parsetree)) - { - /* - * We don't try to send the query to the main database if the CREATE/DROP DATABASE - * command is for the main database itself, this is a very rare case but it's - * exercised by our test suite. - */ - return false; - } - char *queryString = DeparseTreeNode(parsetree); - if (IsA(parsetree, CreatedbStmt) || IsA(parsetree, DropdbStmt)) + /* + * For the commands that cannot be executed in a transaction, there are no + * transactional visibility issues. We directly route them to main database + * so that we only have to consider one code-path when creating databases. + */ + if (StatementCannotBeExecutedInTransaction(parsetree)) { - /* - * We always execute CREATE/DROP DATABASE from the main database. There are no - * transactional visibility issues, since these commands are non-transactional. - * And this way we only have to consider one codepath when creating databases. - */ IsMainDBCommandInXact = false; RunCitusMainDBQuery((char *) queryString); return true; @@ -264,7 +240,7 @@ StatementRequiresMarkDistributedGloballyFromNonMainDb(Node *parsetree) { if (type == NonMainDbSupportedStatements[i].statementType) { - return NonMainDbSupportedStatements[i].DistObjectOperation == + return NonMainDbSupportedStatements[i].distObjectOperation == MARK_DISTRIBUTED_GLOBALLY; } } @@ -287,7 +263,7 @@ StatementRequiresUnmarkDistributedLocallyFromNonMainDb(Node *parsetree) { if (type == NonMainDbSupportedStatements[i].statementType) { - return NonMainDbSupportedStatements[i].DistObjectOperation == + return NonMainDbSupportedStatements[i].distObjectOperation == UNMARK_DISTRIBUTED_LOCALLY; } } @@ -296,6 +272,28 @@ StatementRequiresUnmarkDistributedLocallyFromNonMainDb(Node *parsetree) } +/* + * StatementCannotBeExecutedInTransaction returns true if the statement cannot be executed in a + * transaction. + */ +static bool +StatementCannotBeExecutedInTransaction(Node *parsetree) +{ + NodeTag type = nodeTag(parsetree); + + for (int i = 0; i < sizeof(NonMainDbSupportedStatements) / + sizeof(NonMainDbSupportedStatements[0]); i++) + { + if (type == NonMainDbSupportedStatements[i].statementType) + { + return NonMainDbSupportedStatements[i].cannotBeExecutedInTransaction; + } + } + + return false; +} + + /* * MarkObjectDistributedGloballyFromNonMainDb marks the given object as distributed on the * non-main database. @@ -396,6 +394,38 @@ GetDistObjectOperationParams(Node *parsetree) } +/* + * NonMainDbCheckSupportedObjectTypeForCreateDatabase implements checkSupportedObjectTypes + * callback for CreatedbStmt. + * + * We don't try to send the query to the main database if the CREATE DATABASE + * command is for the main database itself, this is a very rare case but it's + * exercised by our test suite. + */ +static bool +NonMainDbCheckSupportedObjectTypeForCreateDatabase(Node *node) +{ + CreatedbStmt *stmt = castNode(CreatedbStmt, node); + return strcmp(stmt->dbname, MainDb) != 0; +} + + +/* + * NonMainDbCheckSupportedObjectTypeForDropDatabase implements checkSupportedObjectTypes + * callback for DropdbStmt. + * + * We don't try to send the query to the main database if the DROP DATABASE + * command is for the main database itself, this is a very rare case but it's + * exercised by our test suite. + */ +static bool +NonMainDbCheckSupportedObjectTypeForDropDatabase(Node *node) +{ + DropdbStmt *stmt = castNode(DropdbStmt, node); + return strcmp(stmt->dbname, MainDb) != 0; +} + + /* * NonMainDbCheckSupportedObjectTypeForGrant implements checkSupportedObjectTypes * callback for GrantStmt. diff --git a/src/include/distributed/commands.h b/src/include/distributed/commands.h index e9468521f..084308a8f 100644 --- a/src/include/distributed/commands.h +++ b/src/include/distributed/commands.h @@ -105,7 +105,6 @@ typedef struct DistributeObjectOps const DistributeObjectOps * GetDistributeObjectOps(Node *node); /* functions to support node-wide object management commands from non-main dbs */ -extern bool IsCommandToCreateOrDropMainDB(Node *parsetree); extern bool RunPreprocessNonMainDBCommand(Node *parsetree); extern void RunPostprocessNonMainDBCommand(Node *parsetree);