diff --git a/src/backend/columnar/columnar_tableam.c b/src/backend/columnar/columnar_tableam.c index 40486d08f..ca3a5f4c4 100644 --- a/src/backend/columnar/columnar_tableam.c +++ b/src/backend/columnar/columnar_tableam.c @@ -2946,7 +2946,7 @@ MajorVersionsCompatibleColumnar(char *leftVersion, char *rightVersion) } else { - rightComparisionLimit = strlen(leftVersion); + rightComparisionLimit = strlen(rightVersion); } /* we can error out early if hypens are not in the same position */ 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 new file mode 100644 index 000000000..b777936d3 --- /dev/null +++ b/src/backend/distributed/commands/non_main_db_distribute_object_ops.c @@ -0,0 +1,351 @@ +/*------------------------------------------------------------------------- + * + * non_main_db_distribute_object_ops.c + * + * Routines to support node-wide object management commands from non-main + * databases. + * + * RunPreprocessNonMainDBCommand and RunPostprocessNonMainDBCommand are + * the entrypoints for this module. These functions are called from + * utility_hook.c to support some of the node-wide object management + * commands from non-main databases. + * + * To add support for a new command type, one needs to define a new + * NonMainDbDistributeObjectOps object within OperationArray. Also, if + * the command requires marking or unmarking some objects as distributed, + * the necessary operations can be implemented in + * RunPreprocessNonMainDBCommand and RunPostprocessNonMainDBCommand. + * + *------------------------------------------------------------------------- + */ + +#include "postgres.h" + +#include "access/xact.h" +#include "catalog/pg_authid_d.h" +#include "nodes/nodes.h" +#include "nodes/parsenodes.h" +#include "utils/builtins.h" + +#include "distributed/commands.h" +#include "distributed/deparser.h" +#include "distributed/listutils.h" +#include "distributed/metadata_cache.h" +#include "distributed/remote_transaction.h" + + +#define EXECUTE_COMMAND_ON_REMOTE_NODES_AS_USER \ + "SELECT citus_internal.execute_command_on_remote_nodes_as_user(%s, %s)" +#define START_MANAGEMENT_TRANSACTION \ + "SELECT citus_internal.start_management_transaction('%lu')" +#define MARK_OBJECT_DISTRIBUTED \ + "SELECT citus_internal.mark_object_distributed(%d, %s, %d, %s)" +#define UNMARK_OBJECT_DISTRIBUTED \ + "SELECT pg_catalog.citus_unmark_object_distributed(%d, %d, %d, %s)" + + +/* + * NonMainDbDistributeObjectOps contains the necessary callbacks / flags to + * support node-wide object management commands from non-main databases. + * + * cannotBeExecutedInTransaction: + * Indicates whether the statement cannot be executed in a transaction. If + * this is set to true, the statement will be executed directly on the main + * database because there are no transactional visibility issues for such + * commands. + * + * checkSupportedObjectType: + * Callback function that checks whether type of the object referred to by + * given statement is supported. Can be NULL if not applicable for the + * statement type. + */ +typedef struct NonMainDbDistributeObjectOps +{ + bool cannotBeExecutedInTransaction; + bool (*checkSupportedObjectType)(Node *parsetree); +} NonMainDbDistributeObjectOps; + + +/* + * checkSupportedObjectType callbacks for OperationArray. + */ +static bool CreateDbStmtCheckSupportedObjectType(Node *node); +static bool DropDbStmtCheckSupportedObjectType(Node *node); +static bool GrantStmtCheckSupportedObjectType(Node *node); +static bool SecLabelStmtCheckSupportedObjectType(Node *node); + +/* + * OperationArray that holds NonMainDbDistributeObjectOps for different command types. + */ +static const NonMainDbDistributeObjectOps *const OperationArray[] = { + [T_CreateRoleStmt] = &(NonMainDbDistributeObjectOps) { + .cannotBeExecutedInTransaction = false, + .checkSupportedObjectType = NULL + }, + [T_DropRoleStmt] = &(NonMainDbDistributeObjectOps) { + .cannotBeExecutedInTransaction = false, + .checkSupportedObjectType = NULL + }, + [T_AlterRoleStmt] = &(NonMainDbDistributeObjectOps) { + .cannotBeExecutedInTransaction = false, + .checkSupportedObjectType = NULL + }, + [T_GrantRoleStmt] = &(NonMainDbDistributeObjectOps) { + .cannotBeExecutedInTransaction = false, + .checkSupportedObjectType = NULL + }, + [T_CreatedbStmt] = &(NonMainDbDistributeObjectOps) { + .cannotBeExecutedInTransaction = true, + .checkSupportedObjectType = CreateDbStmtCheckSupportedObjectType + }, + [T_DropdbStmt] = &(NonMainDbDistributeObjectOps) { + .cannotBeExecutedInTransaction = true, + .checkSupportedObjectType = DropDbStmtCheckSupportedObjectType + }, + [T_GrantStmt] = &(NonMainDbDistributeObjectOps) { + .cannotBeExecutedInTransaction = false, + .checkSupportedObjectType = GrantStmtCheckSupportedObjectType + }, + [T_SecLabelStmt] = &(NonMainDbDistributeObjectOps) { + .cannotBeExecutedInTransaction = false, + .checkSupportedObjectType = SecLabelStmtCheckSupportedObjectType + }, +}; + + +/* other static function declarations */ +const NonMainDbDistributeObjectOps * GetNonMainDbDistributeObjectOps(Node *parsetree); +static void CreateRoleStmtMarkDistGloballyOnMainDbs(CreateRoleStmt *createRoleStmt); +static void DropRoleStmtUnmarkDistOnLocalMainDb(DropRoleStmt *dropRoleStmt); +static void MarkObjectDistributedGloballyOnMainDbs(Oid catalogRelId, Oid objectId, + char *objectName); +static void UnmarkObjectDistributedOnLocalMainDb(uint16 catalogRelId, Oid objectId); + + +/* + * RunPreprocessNonMainDBCommand runs the necessary commands for a query, in main + * database before query is run on the local node with PrevProcessUtility. + * + * Returns true if previous utility hook needs to be skipped after completing + * preprocess phase. + */ +bool +RunPreprocessNonMainDBCommand(Node *parsetree) +{ + if (IsMainDB) + { + return false; + } + + const NonMainDbDistributeObjectOps *ops = GetNonMainDbDistributeObjectOps(parsetree); + if (!ops) + { + return false; + } + + char *queryString = DeparseTreeNode(parsetree); + + /* + * 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 for such commands. + */ + if (ops->cannotBeExecutedInTransaction) + { + IsMainDBCommandInXact = false; + RunCitusMainDBQuery((char *) queryString); + return true; + } + + IsMainDBCommandInXact = true; + + StringInfo mainDBQuery = makeStringInfo(); + appendStringInfo(mainDBQuery, + START_MANAGEMENT_TRANSACTION, + GetCurrentFullTransactionId().value); + RunCitusMainDBQuery(mainDBQuery->data); + + mainDBQuery = makeStringInfo(); + appendStringInfo(mainDBQuery, + EXECUTE_COMMAND_ON_REMOTE_NODES_AS_USER, + quote_literal_cstr(queryString), + quote_literal_cstr(CurrentUserName())); + RunCitusMainDBQuery(mainDBQuery->data); + + if (IsA(parsetree, DropRoleStmt)) + { + DropRoleStmtUnmarkDistOnLocalMainDb((DropRoleStmt *) parsetree); + } + + return false; +} + + +/* + * RunPostprocessNonMainDBCommand runs the necessary commands for a query, in main + * database after query is run on the local node with PrevProcessUtility. + */ +void +RunPostprocessNonMainDBCommand(Node *parsetree) +{ + if (IsMainDB || !GetNonMainDbDistributeObjectOps(parsetree)) + { + return; + } + + if (IsA(parsetree, CreateRoleStmt)) + { + CreateRoleStmtMarkDistGloballyOnMainDbs((CreateRoleStmt *) parsetree); + } +} + + +/* + * GetNonMainDbDistributeObjectOps returns the NonMainDbDistributeObjectOps for given + * command if it's node-wide object management command that's supported from non-main + * databases. + */ +const NonMainDbDistributeObjectOps * +GetNonMainDbDistributeObjectOps(Node *parsetree) +{ + NodeTag tag = nodeTag(parsetree); + if (tag >= lengthof(OperationArray)) + { + return NULL; + } + + const NonMainDbDistributeObjectOps *ops = OperationArray[tag]; + + if (ops == NULL) + { + return NULL; + } + + if (!ops->checkSupportedObjectType || + ops->checkSupportedObjectType(parsetree)) + { + return ops; + } + + return NULL; +} + + +/* + * CreateRoleStmtMarkDistGloballyOnMainDbs marks the role as + * distributed on all main databases globally. + */ +static void +CreateRoleStmtMarkDistGloballyOnMainDbs(CreateRoleStmt *createRoleStmt) +{ + /* object must exist as we've just created it */ + bool missingOk = false; + Oid roleId = get_role_oid(createRoleStmt->role, missingOk); + + MarkObjectDistributedGloballyOnMainDbs(AuthIdRelationId, roleId, + createRoleStmt->role); +} + + +/* + * DropRoleStmtUnmarkDistOnLocalMainDb unmarks the roles as + * distributed on the local main database. + */ +static void +DropRoleStmtUnmarkDistOnLocalMainDb(DropRoleStmt *dropRoleStmt) +{ + RoleSpec *roleSpec = NULL; + foreach_ptr(roleSpec, dropRoleStmt->roles) + { + Oid roleOid = get_role_oid(roleSpec->rolename, + dropRoleStmt->missing_ok); + if (roleOid == InvalidOid) + { + continue; + } + + UnmarkObjectDistributedOnLocalMainDb(AuthIdRelationId, roleOid); + } +} + + +/* + * MarkObjectDistributedGloballyOnMainDbs marks an object as + * distributed on all main databases globally. + */ +static void +MarkObjectDistributedGloballyOnMainDbs(Oid catalogRelId, Oid objectId, char *objectName) +{ + StringInfo mainDBQuery = makeStringInfo(); + appendStringInfo(mainDBQuery, + MARK_OBJECT_DISTRIBUTED, + catalogRelId, + quote_literal_cstr(objectName), + objectId, + quote_literal_cstr(CurrentUserName())); + RunCitusMainDBQuery(mainDBQuery->data); +} + + +/* + * UnmarkObjectDistributedOnLocalMainDb unmarks an object as + * distributed on the local main database. + */ +static void +UnmarkObjectDistributedOnLocalMainDb(uint16 catalogRelId, Oid objectId) +{ + const int subObjectId = 0; + const char *checkObjectExistence = "false"; + + StringInfo query = makeStringInfo(); + appendStringInfo(query, + UNMARK_OBJECT_DISTRIBUTED, + catalogRelId, objectId, + subObjectId, checkObjectExistence); + RunCitusMainDBQuery(query->data); +} + + +/* + * checkSupportedObjectTypes callbacks for OperationArray lie below. + */ +static bool +CreateDbStmtCheckSupportedObjectType(Node *node) +{ + /* + * 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. + */ + CreatedbStmt *stmt = castNode(CreatedbStmt, node); + return strcmp(stmt->dbname, MainDb) != 0; +} + + +static bool +DropDbStmtCheckSupportedObjectType(Node *node) +{ + /* + * 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. + */ + DropdbStmt *stmt = castNode(DropdbStmt, node); + return strcmp(stmt->dbname, MainDb) != 0; +} + + +static bool +GrantStmtCheckSupportedObjectType(Node *node) +{ + GrantStmt *stmt = castNode(GrantStmt, node); + return stmt->objtype == OBJECT_DATABASE; +} + + +static bool +SecLabelStmtCheckSupportedObjectType(Node *node) +{ + SecLabelStmt *stmt = castNode(SecLabelStmt, node); + return stmt->objtype == OBJECT_ROLE; +} diff --git a/src/backend/distributed/commands/role.c b/src/backend/distributed/commands/role.c index f2b567e6e..7f5f697f2 100644 --- a/src/backend/distributed/commands/role.c +++ b/src/backend/distributed/commands/role.c @@ -491,18 +491,17 @@ GenerateRoleOptionsList(HeapTuple tuple) options = lappend(options, makeDefElem("password", NULL, -1)); } - /* load valid unitl data from the heap tuple, use default of infinity if not set */ + /* load valid until data from the heap tuple */ Datum rolValidUntilDatum = SysCacheGetAttr(AUTHNAME, tuple, Anum_pg_authid_rolvaliduntil, &isNull); - char *rolValidUntil = "infinity"; if (!isNull) { - rolValidUntil = pstrdup((char *) timestamptz_to_str(rolValidUntilDatum)); - } + char *rolValidUntil = pstrdup((char *) timestamptz_to_str(rolValidUntilDatum)); - Node *validUntilStringNode = (Node *) makeString(rolValidUntil); - DefElem *validUntilOption = makeDefElem("validUntil", validUntilStringNode, -1); - options = lappend(options, validUntilOption); + Node *validUntilStringNode = (Node *) makeString(rolValidUntil); + DefElem *validUntilOption = makeDefElem("validUntil", validUntilStringNode, -1); + options = lappend(options, validUntilOption); + } return options; } diff --git a/src/backend/distributed/commands/table.c b/src/backend/distributed/commands/table.c index 074a789ed..30b028b79 100644 --- a/src/backend/distributed/commands/table.c +++ b/src/backend/distributed/commands/table.c @@ -3053,11 +3053,15 @@ ErrorUnsupportedAlterTableAddColumn(Oid relationId, AlterTableCmd *command, else if (constraint->contype == CONSTR_FOREIGN) { RangeVar *referencedTable = constraint->pktable; - char *referencedColumn = strVal(lfirst(list_head(constraint->pk_attrs))); Oid referencedRelationId = RangeVarGetRelid(referencedTable, NoLock, false); - appendStringInfo(errHint, "FOREIGN KEY (%s) REFERENCES %s(%s)", colName, - get_rel_name(referencedRelationId), referencedColumn); + appendStringInfo(errHint, "FOREIGN KEY (%s) REFERENCES %s", colName, + get_rel_name(referencedRelationId)); + + if (list_length(constraint->pk_attrs) > 0) + { + AppendColumnNameList(errHint, constraint->pk_attrs); + } if (constraint->fk_del_action == FKCONSTR_ACTION_SETNULL) { diff --git a/src/backend/distributed/commands/utility_hook.c b/src/backend/distributed/commands/utility_hook.c index 7dff9cbf6..9426e13c0 100644 --- a/src/backend/distributed/commands/utility_hook.c +++ b/src/backend/distributed/commands/utility_hook.c @@ -87,48 +87,6 @@ #include "distributed/worker_shard_visibility.h" #include "distributed/worker_transaction.h" -#define EXECUTE_COMMAND_ON_REMOTE_NODES_AS_USER \ - "SELECT citus_internal.execute_command_on_remote_nodes_as_user(%s, %s)" -#define START_MANAGEMENT_TRANSACTION \ - "SELECT citus_internal.start_management_transaction('%lu')" -#define MARK_OBJECT_DISTRIBUTED \ - "SELECT citus_internal.mark_object_distributed(%d, %s, %d, %s)" - -/* - * NonMainDbDistributedStatementInfo is used to determine whether a statement is - * supported from non-main databases and whether it should be marked as - * distributed explicitly (*). - * - * (*) We always have to mark such objects as "distributed" but while for some - * object types we can delegate this to main database, for some others we have - * to explicitly send a command to all nodes in this code-path to achieve this. - */ -typedef struct NonMainDbDistributedStatementInfo -{ - int statementType; - bool explicitlyMarkAsDistributed; - - /* - * checkSupportedObjectTypes is a callback function that checks whether - * type of the object referred to by given statement is supported. - * - * Can be NULL if not applicable for the statement type. - */ - bool (*checkSupportedObjectTypes)(Node *node); -} NonMainDbDistributedStatementInfo; - -/* - * MarkObjectDistributedParams is used to pass parameters to the - * MarkObjectDistributedFromNonMainDb function. - */ -typedef struct MarkObjectDistributedParams -{ - char *name; - Oid id; - uint16 catalogRelId; -} MarkObjectDistributedParams; - - bool EnableDDLPropagation = true; /* ddl propagation is enabled */ int CreateObjectPropagationMode = CREATE_OBJECT_PROPAGATION_IMMEDIATE; PropSetCmdBehavior PropagateSetCommands = PROPSETCMD_NONE; /* SET prop off */ @@ -158,41 +116,6 @@ static bool IsDropSchemaOrDB(Node *parsetree); static bool ShouldCheckUndistributeCitusLocalTables(void); -/* - * Functions to support commands used to manage node-wide objects from non-main - * databases. - */ -static bool IsCommandToCreateOrDropMainDB(Node *parsetree); -static void RunPreprocessMainDBCommand(Node *parsetree); -static void RunPostprocessMainDBCommand(Node *parsetree); -static bool IsStatementSupportedFromNonMainDb(Node *parsetree); -static bool StatementRequiresMarkDistributedFromNonMainDb(Node *parsetree); -static void MarkObjectDistributedFromNonMainDb(Node *parsetree); -static MarkObjectDistributedParams GetMarkObjectDistributedParams(Node *parsetree); - -/* - * checkSupportedObjectTypes callbacks for - * NonMainDbDistributedStatementInfo objects. - */ -static bool NonMainDbCheckSupportedObjectTypeForGrant(Node *node); -static bool NonMainDbCheckSupportedObjectTypeForSecLabel(Node *node); - - -/* - * NonMainDbSupportedStatements is an array of statements that are supported - * from non-main databases. - */ -ObjectType supportedObjectTypesForGrantStmt[] = { OBJECT_DATABASE }; -static const NonMainDbDistributedStatementInfo NonMainDbSupportedStatements[] = { - { T_GrantRoleStmt, false, NULL }, - { T_CreateRoleStmt, true, NULL }, - { T_GrantStmt, false, NonMainDbCheckSupportedObjectTypeForGrant }, - { T_CreatedbStmt, false, NULL }, - { T_DropdbStmt, false, NULL }, - { T_SecLabelStmt, false, NonMainDbCheckSupportedObjectTypeForSecLabel }, -}; - - /* * ProcessUtilityParseTree is a convenience method to create a PlannedStmt out of * pieces of a utility statement before invoking ProcessUtility. @@ -324,36 +247,25 @@ citus_ProcessUtility(PlannedStmt *pstmt, if (!CitusHasBeenLoaded()) { /* - * 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. - * 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. + * Process the command via RunPreprocessNonMainDBCommand and + * RunPostprocessNonMainDBCommand hooks if we're in a non-main database + * and if the command is a node-wide object management command that we + * support from non-main databases. */ - if (!IsMainDB && - !IsCommandToCreateOrDropMainDB(parsetree)) - { - RunPreprocessMainDBCommand(parsetree); - if (IsA(parsetree, CreatedbStmt) || - IsA(parsetree, DropdbStmt)) - { - return; - } + bool shouldSkipPrevUtilityHook = RunPreprocessNonMainDBCommand(parsetree); + + if (!shouldSkipPrevUtilityHook) + { + /* + * Ensure that utility commands do not behave any differently until CREATE + * EXTENSION is invoked. + */ + PrevProcessUtility(pstmt, queryString, false, context, + params, queryEnv, dest, completionTag); } - /* - * Ensure that utility commands do not behave any differently until CREATE - * EXTENSION is invoked. - */ - PrevProcessUtility(pstmt, queryString, false, context, - params, queryEnv, dest, completionTag); - - if (!IsMainDB) - { - RunPostprocessMainDBCommand(parsetree); - } + RunPostprocessNonMainDBCommand(parsetree); return; } @@ -1689,195 +1601,3 @@ DropSchemaOrDBInProgress(void) { return activeDropSchemaOrDBs > 0; } - - -/* - * 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. - */ -static 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; -} - - -/* - * RunPreprocessMainDBCommand runs the necessary commands for a query, in main - * database before query is run on the local node with PrevProcessUtility - */ -static void -RunPreprocessMainDBCommand(Node *parsetree) -{ - if (!IsStatementSupportedFromNonMainDb(parsetree)) - { - return; - } - - char *queryString = DeparseTreeNode(parsetree); - - if (IsA(parsetree, CreatedbStmt) || - IsA(parsetree, DropdbStmt)) - { - IsMainDBCommandInXact = false; - RunCitusMainDBQuery((char *) queryString); - return; - } - - IsMainDBCommandInXact = true; - - StringInfo mainDBQuery = makeStringInfo(); - appendStringInfo(mainDBQuery, - START_MANAGEMENT_TRANSACTION, - GetCurrentFullTransactionId().value); - RunCitusMainDBQuery(mainDBQuery->data); - mainDBQuery = makeStringInfo(); - appendStringInfo(mainDBQuery, - EXECUTE_COMMAND_ON_REMOTE_NODES_AS_USER, - quote_literal_cstr(queryString), - quote_literal_cstr(CurrentUserName())); - RunCitusMainDBQuery(mainDBQuery->data); -} - - -/* - * RunPostprocessMainDBCommand runs the necessary commands for a query, in main - * database after query is run on the local node with PrevProcessUtility - */ -static void -RunPostprocessMainDBCommand(Node *parsetree) -{ - if (IsStatementSupportedFromNonMainDb(parsetree) && - StatementRequiresMarkDistributedFromNonMainDb(parsetree)) - { - MarkObjectDistributedFromNonMainDb(parsetree); - } -} - - -/* - * IsStatementSupportedFromNonMainDb returns true if the statement is supported from a - * non-main database. - */ -static bool -IsStatementSupportedFromNonMainDb(Node *parsetree) -{ - NodeTag type = nodeTag(parsetree); - - for (int i = 0; i < sizeof(NonMainDbSupportedStatements) / - sizeof(NonMainDbSupportedStatements[0]); i++) - { - if (type != NonMainDbSupportedStatements[i].statementType) - { - continue; - } - - return !NonMainDbSupportedStatements[i].checkSupportedObjectTypes || - NonMainDbSupportedStatements[i].checkSupportedObjectTypes(parsetree); - } - - return false; -} - - -/* - * StatementRequiresMarkDistributedFromNonMainDb returns true if the statement should be marked - * as distributed when executed from a non-main database. - */ -static bool -StatementRequiresMarkDistributedFromNonMainDb(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].explicitlyMarkAsDistributed; - } - } - - return false; -} - - -/* - * MarkObjectDistributedFromNonMainDb marks the given object as distributed on the - * non-main database. - */ -static void -MarkObjectDistributedFromNonMainDb(Node *parsetree) -{ - MarkObjectDistributedParams markObjectDistributedParams = - GetMarkObjectDistributedParams(parsetree); - StringInfo mainDBQuery = makeStringInfo(); - appendStringInfo(mainDBQuery, - MARK_OBJECT_DISTRIBUTED, - markObjectDistributedParams.catalogRelId, - quote_literal_cstr(markObjectDistributedParams.name), - markObjectDistributedParams.id, - quote_literal_cstr(CurrentUserName())); - RunCitusMainDBQuery(mainDBQuery->data); -} - - -/* - * GetMarkObjectDistributedParams returns MarkObjectDistributedParams for the target - * object of given parsetree. - */ -static MarkObjectDistributedParams -GetMarkObjectDistributedParams(Node *parsetree) -{ - if (IsA(parsetree, CreateRoleStmt)) - { - CreateRoleStmt *stmt = castNode(CreateRoleStmt, parsetree); - MarkObjectDistributedParams info = { - .name = stmt->role, - .catalogRelId = AuthIdRelationId, - .id = get_role_oid(stmt->role, false) - }; - - return info; - } - - /* Add else if branches for other statement types */ - - elog(ERROR, "unsupported statement type"); -} - - -/* - * NonMainDbCheckSupportedObjectTypeForGrant implements checkSupportedObjectTypes - * callback for GrantStmt. - */ -static bool -NonMainDbCheckSupportedObjectTypeForGrant(Node *node) -{ - GrantStmt *stmt = castNode(GrantStmt, node); - return stmt->objtype == OBJECT_DATABASE; -} - - -/* - * NonMainDbCheckSupportedObjectTypeForSecLabel implements checkSupportedObjectTypes - * callback for SecLabel. - */ -static bool -NonMainDbCheckSupportedObjectTypeForSecLabel(Node *node) -{ - SecLabelStmt *stmt = castNode(SecLabelStmt, node); - return stmt->objtype == OBJECT_ROLE; -} diff --git a/src/backend/distributed/deparser/deparse_table_stmts.c b/src/backend/distributed/deparser/deparse_table_stmts.c index e976b0e2f..5d184fa66 100644 --- a/src/backend/distributed/deparser/deparse_table_stmts.c +++ b/src/backend/distributed/deparser/deparse_table_stmts.c @@ -121,7 +121,7 @@ AppendAlterTableStmt(StringInfo buf, AlterTableStmt *stmt) * AppendColumnNameList converts a list of columns into comma separated string format * (colname_1, colname_2, .., colname_n). */ -static void +void AppendColumnNameList(StringInfo buf, List *columns) { appendStringInfoString(buf, " ("); diff --git a/src/backend/distributed/metadata/distobject.c b/src/backend/distributed/metadata/distobject.c index 007d07bdc..ff5b2c7a9 100644 --- a/src/backend/distributed/metadata/distobject.c +++ b/src/backend/distributed/metadata/distobject.c @@ -98,10 +98,10 @@ mark_object_distributed(PG_FUNCTION_ARGS) /* - * citus_unmark_object_distributed(classid oid, objid oid, objsubid int) + * citus_unmark_object_distributed(classid oid, objid oid, objsubid int,checkobjectexistence bool) * - * removes the entry for an object address from pg_dist_object. Only removes the entry if - * the object does not exist anymore. + * Removes the entry for an object address from pg_dist_object. If checkobjectexistence is true, + * throws an error if the object still exists. */ Datum citus_unmark_object_distributed(PG_FUNCTION_ARGS) @@ -109,6 +109,12 @@ citus_unmark_object_distributed(PG_FUNCTION_ARGS) Oid classid = PG_GETARG_OID(0); Oid objid = PG_GETARG_OID(1); int32 objsubid = PG_GETARG_INT32(2); + bool checkObjectExistence = true; + if (!PG_ARGISNULL(3)) + { + checkObjectExistence = PG_GETARG_BOOL(3); + } + ObjectAddress address = { 0 }; ObjectAddressSubSet(address, classid, objid, objsubid); @@ -119,7 +125,7 @@ citus_unmark_object_distributed(PG_FUNCTION_ARGS) PG_RETURN_VOID(); } - if (ObjectExists(&address)) + if (checkObjectExistence && ObjectExists(&address)) { ereport(ERROR, (errmsg("object still exists"), errdetail("the %s \"%s\" still exists", diff --git a/src/backend/distributed/metadata/metadata_sync.c b/src/backend/distributed/metadata/metadata_sync.c index 14f5b4624..31d586e90 100644 --- a/src/backend/distributed/metadata/metadata_sync.c +++ b/src/backend/distributed/metadata/metadata_sync.c @@ -492,19 +492,7 @@ stop_metadata_sync_to_node(PG_FUNCTION_ARGS) bool ClusterHasKnownMetadataWorkers() { - bool workerWithMetadata = false; - - if (!IsCoordinator()) - { - workerWithMetadata = true; - } - - if (workerWithMetadata || HasMetadataWorkers()) - { - return true; - } - - return false; + return !IsCoordinator() || HasMetadataWorkers(); } diff --git a/src/backend/distributed/operations/shard_rebalancer.c b/src/backend/distributed/operations/shard_rebalancer.c index d1868d3c4..03dc4c1b8 100644 --- a/src/backend/distributed/operations/shard_rebalancer.c +++ b/src/backend/distributed/operations/shard_rebalancer.c @@ -384,6 +384,7 @@ CheckRebalanceStateInvariants(const RebalanceState *state) Assert(shardCost->cost <= prevShardCost->cost); } totalCost += shardCost->cost; + prevShardCost = shardCost; } /* Check that utilization field is up to date. */ diff --git a/src/backend/distributed/planner/function_call_delegation.c b/src/backend/distributed/planner/function_call_delegation.c index f17b02347..4a79dc25a 100644 --- a/src/backend/distributed/planner/function_call_delegation.c +++ b/src/backend/distributed/planner/function_call_delegation.c @@ -91,6 +91,10 @@ bool InDelegatedFunctionCall = false; static bool contain_param_walker(Node *node, void *context) { + if (node == NULL) + { + return false; + } if (IsA(node, Param)) { Param *paramNode = (Param *) node; diff --git a/src/backend/distributed/sql/citus--12.1-1--12.2-1.sql b/src/backend/distributed/sql/citus--12.1-1--12.2-1.sql index 68823b3be..2d5f88676 100644 --- a/src/backend/distributed/sql/citus--12.1-1--12.2-1.sql +++ b/src/backend/distributed/sql/citus--12.1-1--12.2-1.sql @@ -7,6 +7,8 @@ #include "udfs/start_management_transaction/12.2-1.sql" #include "udfs/execute_command_on_remote_nodes_as_user/12.2-1.sql" #include "udfs/mark_object_distributed/12.2-1.sql" +DROP FUNCTION pg_catalog.citus_unmark_object_distributed(oid, oid, int); +#include "udfs/citus_unmark_object_distributed/12.2-1.sql" #include "udfs/commit_management_command_2pc/12.2-1.sql" ALTER TABLE pg_catalog.pg_dist_transaction ADD COLUMN outer_xid xid8; diff --git a/src/backend/distributed/sql/downgrades/citus--12.2-1--12.1-1.sql b/src/backend/distributed/sql/downgrades/citus--12.2-1--12.1-1.sql index 5b2828cfe..581c65ea8 100644 --- a/src/backend/distributed/sql/downgrades/citus--12.2-1--12.1-1.sql +++ b/src/backend/distributed/sql/downgrades/citus--12.2-1--12.1-1.sql @@ -18,6 +18,9 @@ DROP FUNCTION citus_internal.mark_object_distributed( classId Oid, objectName text, objectId Oid, connectionUser text ); +DROP FUNCTION pg_catalog.citus_unmark_object_distributed(oid,oid,int,boolean); +#include "../udfs/citus_unmark_object_distributed/10.0-1.sql" + DROP FUNCTION citus_internal.commit_management_command_2pc(); ALTER TABLE pg_catalog.pg_dist_transaction DROP COLUMN outer_xid; diff --git a/src/backend/distributed/sql/udfs/citus_unmark_object_distributed/12.2-1.sql b/src/backend/distributed/sql/udfs/citus_unmark_object_distributed/12.2-1.sql new file mode 100644 index 000000000..3c1b1bdec --- /dev/null +++ b/src/backend/distributed/sql/udfs/citus_unmark_object_distributed/12.2-1.sql @@ -0,0 +1,7 @@ +CREATE FUNCTION pg_catalog.citus_unmark_object_distributed(classid oid, objid oid, objsubid int, checkobjectexistence boolean DEFAULT true) + RETURNS void + LANGUAGE C STRICT + AS 'MODULE_PATHNAME', $$citus_unmark_object_distributed$$; +COMMENT ON FUNCTION pg_catalog.citus_unmark_object_distributed(classid oid, objid oid, objsubid int, checkobjectexistence boolean) + IS 'Removes an object from citus.pg_dist_object after deletion. If checkobjectexistence is true, object existence check performed.' + 'Otherwise, object existence check is skipped.'; diff --git a/src/backend/distributed/sql/udfs/citus_unmark_object_distributed/latest.sql b/src/backend/distributed/sql/udfs/citus_unmark_object_distributed/latest.sql index 3f60c60c3..3c1b1bdec 100644 --- a/src/backend/distributed/sql/udfs/citus_unmark_object_distributed/latest.sql +++ b/src/backend/distributed/sql/udfs/citus_unmark_object_distributed/latest.sql @@ -1,6 +1,7 @@ -CREATE FUNCTION pg_catalog.citus_unmark_object_distributed(classid oid, objid oid, objsubid int) +CREATE FUNCTION pg_catalog.citus_unmark_object_distributed(classid oid, objid oid, objsubid int, checkobjectexistence boolean DEFAULT true) RETURNS void LANGUAGE C STRICT AS 'MODULE_PATHNAME', $$citus_unmark_object_distributed$$; -COMMENT ON FUNCTION pg_catalog.citus_unmark_object_distributed(classid oid, objid oid, objsubid int) - IS 'remove an object address from citus.pg_dist_object once the object has been deleted'; +COMMENT ON FUNCTION pg_catalog.citus_unmark_object_distributed(classid oid, objid oid, objsubid int, checkobjectexistence boolean) + IS 'Removes an object from citus.pg_dist_object after deletion. If checkobjectexistence is true, object existence check performed.' + 'Otherwise, object existence check is skipped.'; diff --git a/src/backend/distributed/test/metadata_sync.c b/src/backend/distributed/test/metadata_sync.c index dec20c772..ce025cff9 100644 --- a/src/backend/distributed/test/metadata_sync.c +++ b/src/backend/distributed/test/metadata_sync.c @@ -50,6 +50,13 @@ activate_node_snapshot(PG_FUNCTION_ARGS) * so we are using first primary worker node just for test purposes. */ WorkerNode *dummyWorkerNode = GetFirstPrimaryWorkerNode(); + if (dummyWorkerNode == NULL) + { + ereport(ERROR, (errmsg("no worker nodes found"), + errdetail("Function activate_node_snapshot is meant to be " + "used when running tests on a multi-node cluster " + "with workers."))); + } /* * Create MetadataSyncContext which is used throughout nodes' activation. diff --git a/src/backend/distributed/transaction/transaction_recovery.c b/src/backend/distributed/transaction/transaction_recovery.c index 653b962db..c31dc85a2 100644 --- a/src/backend/distributed/transaction/transaction_recovery.c +++ b/src/backend/distributed/transaction/transaction_recovery.c @@ -34,6 +34,7 @@ #include "utils/fmgroids.h" #include "utils/memutils.h" #include "utils/rel.h" +#include "utils/syscache.h" #include "utils/xid8.h" #include "pg_version_constants.h" @@ -261,11 +262,28 @@ RecoverWorkerTransactions(WorkerNode *workerNode) continue; } - /* Check if the transaction is created by an outer transaction from a non-main database */ bool outerXidIsNull = false; - Datum outerXidDatum = heap_getattr(heapTuple, - Anum_pg_dist_transaction_outerxid, - tupleDescriptor, &outerXidIsNull); + Datum outerXidDatum = 0; + if (EnableVersionChecks || + SearchSysCacheExistsAttName(DistTransactionRelationId(), "outer_xid")) + { + /* Check if the transaction is created by an outer transaction from a non-main database */ + outerXidDatum = heap_getattr(heapTuple, + Anum_pg_dist_transaction_outerxid, + tupleDescriptor, &outerXidIsNull); + } + else + { + /* + * Normally we don't try to recover prepared transactions when the + * binary version doesn't match the sql version. However, we skip + * those checks in regression tests by disabling + * citus.enable_version_checks. And when this is the case, while + * the C code looks for "outer_xid" attribute, pg_dist_transaction + * doesn't yet have it. + */ + Assert(!EnableVersionChecks); + } TransactionId outerXid = 0; if (!outerXidIsNull) diff --git a/src/backend/distributed/utils/resource_lock.c b/src/backend/distributed/utils/resource_lock.c index 13e88a16e..8ac269e43 100644 --- a/src/backend/distributed/utils/resource_lock.c +++ b/src/backend/distributed/utils/resource_lock.c @@ -707,13 +707,27 @@ SerializeNonCommutativeWrites(List *shardIntervalList, LOCKMODE lockMode) } List *replicatedShardList = NIL; - if (AnyTableReplicated(shardIntervalList, &replicatedShardList)) - { - if (ClusterHasKnownMetadataWorkers() && !IsFirstWorkerNode()) - { - LockShardListResourcesOnFirstWorker(lockMode, replicatedShardList); - } + bool anyTableReplicated = AnyTableReplicated(shardIntervalList, &replicatedShardList); + /* + * Acquire locks on the modified table. + * If the table is replicated, the locks are first acquired on the first worker node then locally. + * But if we're already on the first worker, acquiring on the first worker node and locally are the same operation. + * So we only acquire locally in that case. + */ + if (anyTableReplicated && ClusterHasKnownMetadataWorkers() && !IsFirstWorkerNode()) + { + LockShardListResourcesOnFirstWorker(lockMode, replicatedShardList); + } + LockShardListResources(shardIntervalList, lockMode); + + /* + * Next, acquire locks on the reference tables that are referenced by a foreign key if there are any. + * Note that LockReferencedReferenceShardResources() first acquires locks on the first worker, + * then locally. + */ + if (anyTableReplicated) + { ShardInterval *firstShardInterval = (ShardInterval *) linitial(replicatedShardList); if (ReferenceTableShardId(firstShardInterval->shardId)) @@ -728,8 +742,6 @@ SerializeNonCommutativeWrites(List *shardIntervalList, LOCKMODE lockMode) LockReferencedReferenceShardResources(firstShardInterval->shardId, lockMode); } } - - LockShardListResources(shardIntervalList, lockMode); } diff --git a/src/include/distributed/commands.h b/src/include/distributed/commands.h index de15553e7..084308a8f 100644 --- a/src/include/distributed/commands.h +++ b/src/include/distributed/commands.h @@ -104,6 +104,10 @@ typedef struct DistributeObjectOps const DistributeObjectOps * GetDistributeObjectOps(Node *node); +/* functions to support node-wide object management commands from non-main dbs */ +extern bool RunPreprocessNonMainDBCommand(Node *parsetree); +extern void RunPostprocessNonMainDBCommand(Node *parsetree); + /* * Flags that can be passed to GetForeignKeyOids to indicate * which foreign key constraint OIDs are to be extracted diff --git a/src/include/distributed/deparser.h b/src/include/distributed/deparser.h index 437a9fd8e..4d4005c19 100644 --- a/src/include/distributed/deparser.h +++ b/src/include/distributed/deparser.h @@ -121,6 +121,8 @@ extern void AppendGrantedByInGrant(StringInfo buf, GrantStmt *stmt); extern void AppendGrantSharedPrefix(StringInfo buf, GrantStmt *stmt); extern void AppendGrantSharedSuffix(StringInfo buf, GrantStmt *stmt); +extern void AppendColumnNameList(StringInfo buf, List *columns); + /* Common deparser utils */ typedef struct DefElemOptionFormat diff --git a/src/test/regress/expected/alter_table_add_column.out b/src/test/regress/expected/alter_table_add_column.out index 61e7319d9..0408aeeab 100644 --- a/src/test/regress/expected/alter_table_add_column.out +++ b/src/test/regress/expected/alter_table_add_column.out @@ -44,6 +44,15 @@ ERROR: cannot execute ADD COLUMN command with PRIMARY KEY, UNIQUE, FOREIGN and DETAIL: Adding a column with a constraint in one command is not supported because all constraints in Citus must have explicit names HINT: You can issue each command separately such as ALTER TABLE referencing ADD COLUMN test_8 data_type; ALTER TABLE referencing ADD CONSTRAINT constraint_name CHECK (check_expression); ALTER TABLE referencing ADD COLUMN test_8 integer CONSTRAINT check_test_8 CHECK (test_8 > 0); +-- error out properly even if the REFERENCES does not include the column list of the referenced table +ALTER TABLE referencing ADD COLUMN test_9 bool, ADD COLUMN test_10 int REFERENCES referenced; +ERROR: cannot execute ADD COLUMN command with PRIMARY KEY, UNIQUE, FOREIGN and CHECK constraints +DETAIL: Adding a column with a constraint in one command is not supported because all constraints in Citus must have explicit names +HINT: You can issue each command separately such as ALTER TABLE referencing ADD COLUMN test_10 data_type; ALTER TABLE referencing ADD CONSTRAINT constraint_name FOREIGN KEY (test_10) REFERENCES referenced; +ALTER TABLE referencing ADD COLUMN test_9 bool, ADD COLUMN test_10 int REFERENCES referenced(int_col); +ERROR: cannot execute ADD COLUMN command with PRIMARY KEY, UNIQUE, FOREIGN and CHECK constraints +DETAIL: Adding a column with a constraint in one command is not supported because all constraints in Citus must have explicit names +HINT: You can issue each command separately such as ALTER TABLE referencing ADD COLUMN test_10 data_type; ALTER TABLE referencing ADD CONSTRAINT constraint_name FOREIGN KEY (test_10) REFERENCES referenced (int_col ); -- try to add test_6 again, but with IF NOT EXISTS ALTER TABLE referencing ADD COLUMN IF NOT EXISTS test_6 text; NOTICE: column "test_6" of relation "referencing" already exists, skipping diff --git a/src/test/regress/expected/create_role_propagation.out b/src/test/regress/expected/create_role_propagation.out index 90f2690ce..4d594ddab 100644 --- a/src/test/regress/expected/create_role_propagation.out +++ b/src/test/regress/expected/create_role_propagation.out @@ -121,17 +121,17 @@ SELECT 1 FROM master_add_node('localhost', :worker_2_port); SELECT rolname, rolsuper, rolinherit, rolcreaterole, rolcreatedb, rolcanlogin, rolreplication, rolbypassrls, rolconnlimit, (rolpassword != '') as pass_not_empty, rolvaliduntil FROM pg_authid WHERE rolname LIKE 'create\_%' ORDER BY rolname; rolname | rolsuper | rolinherit | rolcreaterole | rolcreatedb | rolcanlogin | rolreplication | rolbypassrls | rolconnlimit | pass_not_empty | rolvaliduntil --------------------------------------------------------------------- - create_group | f | t | f | f | f | f | f | -1 | | infinity - create_group_2 | f | t | f | f | f | f | f | -1 | | infinity - create_role | f | t | f | f | f | f | f | -1 | | infinity - create_role"edge | f | t | f | f | f | f | f | -1 | | infinity - create_role'edge | f | t | f | f | f | f | f | -1 | | infinity - create_role_2 | f | t | f | f | f | f | f | -1 | | infinity - create_role_sysid | f | t | f | f | f | f | f | -1 | | infinity + create_group | f | t | f | f | f | f | f | -1 | | + create_group_2 | f | t | f | f | f | f | f | -1 | | + create_role | f | t | f | f | f | f | f | -1 | | + create_role"edge | f | t | f | f | f | f | f | -1 | | + create_role'edge | f | t | f | f | f | f | f | -1 | | + create_role_2 | f | t | f | f | f | f | f | -1 | | + create_role_sysid | f | t | f | f | f | f | f | -1 | | create_role_with_everything | t | t | t | t | t | t | t | 105 | t | Thu May 04 17:00:00 2045 PDT create_role_with_nothing | f | f | f | f | f | f | f | 3 | t | Mon May 04 17:00:00 2015 PDT - create_user | f | t | f | f | t | f | f | -1 | | infinity - create_user_2 | f | t | f | f | t | f | f | -1 | | infinity + create_user | f | t | f | f | t | f | f | -1 | | + create_user_2 | f | t | f | f | t | f | f | -1 | | (11 rows) SELECT roleid::regrole::text AS role, member::regrole::text, grantor::regrole::text, admin_option FROM pg_auth_members WHERE roleid::regrole::text LIKE 'create\_%' ORDER BY 1, 2; diff --git a/src/test/regress/expected/function_with_case_when.out b/src/test/regress/expected/function_with_case_when.out new file mode 100644 index 000000000..18df5be0a --- /dev/null +++ b/src/test/regress/expected/function_with_case_when.out @@ -0,0 +1,32 @@ +CREATE SCHEMA function_with_case; +SET search_path TO function_with_case; +-- create function +CREATE OR REPLACE FUNCTION test_err(v1 text) + RETURNS text + LANGUAGE plpgsql + SECURITY DEFINER +AS $function$ + +begin + return v1 || ' - ok'; +END; +$function$; +do $$ declare + lNewValues text; + val text; +begin + val = 'test'; + lNewValues = test_err(v1 => case when val::text = 'test'::text then 'yes' else 'no' end); + raise notice 'lNewValues= %', lNewValues; +end;$$ ; +NOTICE: lNewValues= yes - ok +CONTEXT: PL/pgSQL function inline_code_block line XX at RAISE +-- call function +SELECT test_err('test'); + test_err +--------------------------------------------------------------------- + test - ok +(1 row) + +DROP SCHEMA function_with_case CASCADE; +NOTICE: drop cascades to function test_err(text) diff --git a/src/test/regress/expected/issue_7477.out b/src/test/regress/expected/issue_7477.out new file mode 100644 index 000000000..224d85c6e --- /dev/null +++ b/src/test/regress/expected/issue_7477.out @@ -0,0 +1,62 @@ +--- Test for updating a table that has a foreign key reference to another reference table. +--- Issue #7477: Distributed deadlock after issuing a simple UPDATE statement +--- https://github.com/citusdata/citus/issues/7477 +CREATE TABLE table1 (id INT PRIMARY KEY); +SELECT create_reference_table('table1'); + create_reference_table +--------------------------------------------------------------------- + +(1 row) + +INSERT INTO table1 VALUES (1); +CREATE TABLE table2 ( + id INT, + info TEXT, + CONSTRAINT table1_id_fk FOREIGN KEY (id) REFERENCES table1 (id) + ); +SELECT create_reference_table('table2'); + create_reference_table +--------------------------------------------------------------------- + +(1 row) + +INSERT INTO table2 VALUES (1, 'test'); +--- Runs the update command in parallel on workers. +--- Due to bug #7477, before the fix, the result is non-deterministic +--- and have several rows of the form: +--- localhost | 57638 | f | ERROR: deadlock detected +--- localhost | 57637 | f | ERROR: deadlock detected +--- localhost | 57637 | f | ERROR: canceling the transaction since it was involved in a distributed deadlock +SELECT * FROM master_run_on_worker( + ARRAY['localhost', 'localhost','localhost', 'localhost','localhost', + 'localhost','localhost', 'localhost','localhost', 'localhost']::text[], + ARRAY[57638, 57637, 57637, 57638, 57637, 57638, 57637, 57638, 57638, 57637]::int[], + ARRAY['UPDATE table2 SET info = ''test_update'' WHERE id = 1', + 'UPDATE table2 SET info = ''test_update'' WHERE id = 1', + 'UPDATE table2 SET info = ''test_update'' WHERE id = 1', + 'UPDATE table2 SET info = ''test_update'' WHERE id = 1', + 'UPDATE table2 SET info = ''test_update'' WHERE id = 1', + 'UPDATE table2 SET info = ''test_update'' WHERE id = 1', + 'UPDATE table2 SET info = ''test_update'' WHERE id = 1', + 'UPDATE table2 SET info = ''test_update'' WHERE id = 1', + 'UPDATE table2 SET info = ''test_update'' WHERE id = 1', + 'UPDATE table2 SET info = ''test_update'' WHERE id = 1' + ]::text[], + true); + node_name | node_port | success | result +--------------------------------------------------------------------- + localhost | 57638 | t | UPDATE 1 + localhost | 57637 | t | UPDATE 1 + localhost | 57637 | t | UPDATE 1 + localhost | 57638 | t | UPDATE 1 + localhost | 57637 | t | UPDATE 1 + localhost | 57638 | t | UPDATE 1 + localhost | 57637 | t | UPDATE 1 + localhost | 57638 | t | UPDATE 1 + localhost | 57638 | t | UPDATE 1 + localhost | 57637 | t | UPDATE 1 +(10 rows) + +--- cleanup +DROP TABLE table2; +DROP TABLE table1; diff --git a/src/test/regress/expected/metadata_sync_from_non_maindb.out b/src/test/regress/expected/metadata_sync_from_non_maindb.out index 91ca1c82d..2aac507bd 100644 --- a/src/test/regress/expected/metadata_sync_from_non_maindb.out +++ b/src/test/regress/expected/metadata_sync_from_non_maindb.out @@ -164,6 +164,146 @@ revoke CONNECT on database metadata_sync_2pc_db from "grant_role2pc'_user2"; revoke CREATE on database metadata_sync_2pc_db from "grant_role2pc'_user1"; \c regression drop user "grant_role2pc'_user1","grant_role2pc'_user2","grant_role2pc'_user3",grant_role2pc_user4,grant_role2pc_user5; +--test for user operations +--test for create user +\c regression - - :master_port +select 1 from citus_remove_node('localhost', :worker_2_port); + ?column? +--------------------------------------------------------------------- + 1 +(1 row) + +\c metadata_sync_2pc_db - - :master_port +CREATE ROLE test_role1 WITH LOGIN PASSWORD 'password1'; +\c metadata_sync_2pc_db - - :worker_1_port +CREATE USER "test_role2-needs\!escape" +WITH + SUPERUSER CREATEDB CREATEROLE INHERIT LOGIN REPLICATION BYPASSRLS CONNECTION +LIMIT 10 VALID UNTIL '2023-01-01' IN ROLE test_role1; +create role test_role3; +\c regression - - :master_port +select 1 from citus_add_node('localhost', :worker_2_port); + ?column? +--------------------------------------------------------------------- + 1 +(1 row) + +select result FROM run_command_on_all_nodes($$ + SELECT array_to_json(array_agg(row_to_json(t))) + FROM ( + SELECT rolname, rolsuper, rolinherit, rolcreaterole, rolcreatedb, + rolcanlogin, rolreplication, rolbypassrls, rolconnlimit, + (rolpassword != '') as pass_not_empty, DATE(rolvaliduntil) + FROM pg_authid + WHERE rolname in ('test_role1', 'test_role2-needs\!escape','test_role3') + ORDER BY rolname + ) t +$$); + result +--------------------------------------------------------------------- + [{"rolname":"test_role1","rolsuper":false,"rolinherit":true,"rolcreaterole":false,"rolcreatedb":false,"rolcanlogin":true,"rolreplication":false,"rolbypassrls":false,"rolconnlimit":-1,"pass_not_empty":true,"date":null},{"rolname":"test_role2-needs\\!escape","rolsuper":true,"rolinherit":true,"rolcreaterole":true,"rolcreatedb":true,"rolcanlogin":true,"rolreplication":true,"rolbypassrls":true,"rolconnlimit":10,"pass_not_empty":null,"date":"2023-01-01"},{"rolname":"test_role3","rolsuper":false,"rolinherit":true,"rolcreaterole":false,"rolcreatedb":false,"rolcanlogin":false,"rolreplication":false,"rolbypassrls":false,"rolconnlimit":-1,"pass_not_empty":null,"date":null}] + [{"rolname":"test_role1","rolsuper":false,"rolinherit":true,"rolcreaterole":false,"rolcreatedb":false,"rolcanlogin":true,"rolreplication":false,"rolbypassrls":false,"rolconnlimit":-1,"pass_not_empty":true,"date":null},{"rolname":"test_role2-needs\\!escape","rolsuper":true,"rolinherit":true,"rolcreaterole":true,"rolcreatedb":true,"rolcanlogin":true,"rolreplication":true,"rolbypassrls":true,"rolconnlimit":10,"pass_not_empty":null,"date":"2023-01-01"},{"rolname":"test_role3","rolsuper":false,"rolinherit":true,"rolcreaterole":false,"rolcreatedb":false,"rolcanlogin":false,"rolreplication":false,"rolbypassrls":false,"rolconnlimit":-1,"pass_not_empty":null,"date":null}] + [{"rolname":"test_role1","rolsuper":false,"rolinherit":true,"rolcreaterole":false,"rolcreatedb":false,"rolcanlogin":true,"rolreplication":false,"rolbypassrls":false,"rolconnlimit":-1,"pass_not_empty":true,"date":null},{"rolname":"test_role2-needs\\!escape","rolsuper":true,"rolinherit":true,"rolcreaterole":true,"rolcreatedb":true,"rolcanlogin":true,"rolreplication":true,"rolbypassrls":true,"rolconnlimit":10,"pass_not_empty":null,"date":"2023-01-01"},{"rolname":"test_role3","rolsuper":false,"rolinherit":true,"rolcreaterole":false,"rolcreatedb":false,"rolcanlogin":false,"rolreplication":false,"rolbypassrls":false,"rolconnlimit":-1,"pass_not_empty":null,"date":null}] +(3 rows) + +--test for alter user +select 1 from citus_remove_node('localhost', :worker_2_port); + ?column? +--------------------------------------------------------------------- + 1 +(1 row) + +\c metadata_sync_2pc_db - - :master_port +-- Test ALTER ROLE with various options +ALTER ROLE test_role1 WITH PASSWORD 'new_password1'; +\c metadata_sync_2pc_db - - :worker_1_port +ALTER USER "test_role2-needs\!escape" +WITH + NOSUPERUSER NOCREATEDB NOCREATEROLE NOINHERIT NOLOGIN NOREPLICATION NOBYPASSRLS CONNECTION +LIMIT 5 VALID UNTIL '2024-01-01'; +\c regression - - :master_port +select 1 from citus_add_node('localhost', :worker_2_port); + ?column? +--------------------------------------------------------------------- + 1 +(1 row) + +select result FROM run_command_on_all_nodes($$ + SELECT array_to_json(array_agg(row_to_json(t))) + FROM ( + SELECT rolname, rolsuper, rolinherit, rolcreaterole, rolcreatedb, + rolcanlogin, rolreplication, rolbypassrls, rolconnlimit, + (rolpassword != '') as pass_not_empty, DATE(rolvaliduntil) + FROM pg_authid + WHERE rolname in ('test_role1', 'test_role2-needs\!escape','test_role3') + ORDER BY rolname + ) t +$$); + result +--------------------------------------------------------------------- + [{"rolname":"test_role1","rolsuper":false,"rolinherit":true,"rolcreaterole":false,"rolcreatedb":false,"rolcanlogin":true,"rolreplication":false,"rolbypassrls":false,"rolconnlimit":-1,"pass_not_empty":true,"date":null},{"rolname":"test_role2-needs\\!escape","rolsuper":false,"rolinherit":false,"rolcreaterole":false,"rolcreatedb":false,"rolcanlogin":false,"rolreplication":false,"rolbypassrls":false,"rolconnlimit":5,"pass_not_empty":null,"date":"2024-01-01"},{"rolname":"test_role3","rolsuper":false,"rolinherit":true,"rolcreaterole":false,"rolcreatedb":false,"rolcanlogin":false,"rolreplication":false,"rolbypassrls":false,"rolconnlimit":-1,"pass_not_empty":null,"date":null}] + [{"rolname":"test_role1","rolsuper":false,"rolinherit":true,"rolcreaterole":false,"rolcreatedb":false,"rolcanlogin":true,"rolreplication":false,"rolbypassrls":false,"rolconnlimit":-1,"pass_not_empty":true,"date":null},{"rolname":"test_role2-needs\\!escape","rolsuper":false,"rolinherit":false,"rolcreaterole":false,"rolcreatedb":false,"rolcanlogin":false,"rolreplication":false,"rolbypassrls":false,"rolconnlimit":5,"pass_not_empty":null,"date":"2024-01-01"},{"rolname":"test_role3","rolsuper":false,"rolinherit":true,"rolcreaterole":false,"rolcreatedb":false,"rolcanlogin":false,"rolreplication":false,"rolbypassrls":false,"rolconnlimit":-1,"pass_not_empty":null,"date":null}] + [{"rolname":"test_role1","rolsuper":false,"rolinherit":true,"rolcreaterole":false,"rolcreatedb":false,"rolcanlogin":true,"rolreplication":false,"rolbypassrls":false,"rolconnlimit":-1,"pass_not_empty":true,"date":null},{"rolname":"test_role2-needs\\!escape","rolsuper":false,"rolinherit":false,"rolcreaterole":false,"rolcreatedb":false,"rolcanlogin":false,"rolreplication":false,"rolbypassrls":false,"rolconnlimit":5,"pass_not_empty":null,"date":"2024-01-01"},{"rolname":"test_role3","rolsuper":false,"rolinherit":true,"rolcreaterole":false,"rolcreatedb":false,"rolcanlogin":false,"rolreplication":false,"rolbypassrls":false,"rolconnlimit":-1,"pass_not_empty":null,"date":null}] +(3 rows) + +--test for drop user +select 1 from citus_remove_node('localhost', :worker_2_port); + ?column? +--------------------------------------------------------------------- + 1 +(1 row) + +\c metadata_sync_2pc_db - - :worker_1_port +DROP ROLE test_role1, "test_role2-needs\!escape"; +\c metadata_sync_2pc_db - - :master_port +DROP ROLE test_role3; +\c regression - - :master_port +select 1 from citus_add_node('localhost', :worker_2_port); + ?column? +--------------------------------------------------------------------- + 1 +(1 row) + +select result FROM run_command_on_all_nodes($$ + SELECT array_to_json(array_agg(row_to_json(t))) + FROM ( + SELECT rolname, rolsuper, rolinherit, rolcreaterole, rolcreatedb, + rolcanlogin, rolreplication, rolbypassrls, rolconnlimit, + (rolpassword != '') as pass_not_empty, DATE(rolvaliduntil) + FROM pg_authid + WHERE rolname in ('test_role1', 'test_role2-needs\!escape','test_role3') + ORDER BY rolname + ) t +$$); + result +--------------------------------------------------------------------- + + + [{"rolname":"test_role1","rolsuper":false,"rolinherit":true,"rolcreaterole":false,"rolcreatedb":false,"rolcanlogin":true,"rolreplication":false,"rolbypassrls":false,"rolconnlimit":-1,"pass_not_empty":true,"date":null},{"rolname":"test_role2-needs\\!escape","rolsuper":false,"rolinherit":false,"rolcreaterole":false,"rolcreatedb":false,"rolcanlogin":false,"rolreplication":false,"rolbypassrls":false,"rolconnlimit":5,"pass_not_empty":null,"date":"2024-01-01"},{"rolname":"test_role3","rolsuper":false,"rolinherit":true,"rolcreaterole":false,"rolcreatedb":false,"rolcanlogin":false,"rolreplication":false,"rolbypassrls":false,"rolconnlimit":-1,"pass_not_empty":null,"date":null}] +(3 rows) + +-- Clean up: drop the database on worker node 2 +\c regression - - :worker_2_port +DROP ROLE if exists test_role1, "test_role2-needs\!escape", test_role3; +\c regression - - :master_port +select result FROM run_command_on_all_nodes($$ + SELECT array_to_json(array_agg(row_to_json(t))) + FROM ( + SELECT rolname, rolsuper, rolinherit, rolcreaterole, rolcreatedb, + rolcanlogin, rolreplication, rolbypassrls, rolconnlimit, + (rolpassword != '') as pass_not_empty, DATE(rolvaliduntil) + FROM pg_authid + WHERE rolname in ('test_role1', 'test_role2-needs\!escape','test_role3') + ORDER BY rolname + ) t +$$); + result +--------------------------------------------------------------------- + + + +(3 rows) + set citus.enable_create_database_propagation to on; drop database metadata_sync_2pc_db; drop schema metadata_sync_2pc_schema; diff --git a/src/test/regress/expected/multi_extension.out b/src/test/regress/expected/multi_extension.out index 0aecd652f..aaafce715 100644 --- a/src/test/regress/expected/multi_extension.out +++ b/src/test/regress/expected/multi_extension.out @@ -1420,37 +1420,39 @@ SELECT * FROM multi_extension.print_extension_changes(); -- Snapshot of state at 12.2-1 ALTER EXTENSION citus UPDATE TO '12.2-1'; SELECT * FROM multi_extension.print_extension_changes(); - previous_object | current_object + previous_object | current_object --------------------------------------------------------------------- - | function citus_internal.acquire_citus_advisory_object_class_lock(integer,cstring) void - | function citus_internal.add_colocation_metadata(integer,integer,integer,regtype,oid) void - | function citus_internal.add_object_metadata(text,text[],text[],integer,integer,boolean) void - | function citus_internal.add_partition_metadata(regclass,"char",text,integer,"char") void - | function citus_internal.add_placement_metadata(bigint,bigint,integer,bigint) void - | function citus_internal.add_shard_metadata(regclass,bigint,"char",text,text) void - | function citus_internal.add_tenant_schema(oid,integer) void - | function citus_internal.adjust_local_clock_to_remote(cluster_clock) void - | function citus_internal.commit_management_command_2pc() void - | function citus_internal.database_command(text) void - | function citus_internal.delete_colocation_metadata(integer) void - | function citus_internal.delete_partition_metadata(regclass) void - | function citus_internal.delete_placement_metadata(bigint) void - | function citus_internal.delete_shard_metadata(bigint) void - | function citus_internal.delete_tenant_schema(oid) void - | function citus_internal.execute_command_on_remote_nodes_as_user(text,text) void - | function citus_internal.global_blocked_processes() SETOF record - | function citus_internal.is_replication_origin_tracking_active() boolean - | function citus_internal.local_blocked_processes() SETOF record - | function citus_internal.mark_node_not_synced(integer,integer) void - | function citus_internal.mark_object_distributed(oid,text,oid,text) void - | function citus_internal.start_management_transaction(xid8) void - | function citus_internal.start_replication_origin_tracking() void - | function citus_internal.stop_replication_origin_tracking() void - | function citus_internal.unregister_tenant_schema_globally(oid,text) void - | function citus_internal.update_none_dist_table_metadata(oid,"char",bigint,boolean) void - | function citus_internal.update_placement_metadata(bigint,integer,integer) void - | function citus_internal.update_relation_colocation(oid,integer) void -(28 rows) + function citus_unmark_object_distributed(oid,oid,integer) void | + | function citus_internal.acquire_citus_advisory_object_class_lock(integer,cstring) void + | function citus_internal.add_colocation_metadata(integer,integer,integer,regtype,oid) void + | function citus_internal.add_object_metadata(text,text[],text[],integer,integer,boolean) void + | function citus_internal.add_partition_metadata(regclass,"char",text,integer,"char") void + | function citus_internal.add_placement_metadata(bigint,bigint,integer,bigint) void + | function citus_internal.add_shard_metadata(regclass,bigint,"char",text,text) void + | function citus_internal.add_tenant_schema(oid,integer) void + | function citus_internal.adjust_local_clock_to_remote(cluster_clock) void + | function citus_internal.commit_management_command_2pc() void + | function citus_internal.database_command(text) void + | function citus_internal.delete_colocation_metadata(integer) void + | function citus_internal.delete_partition_metadata(regclass) void + | function citus_internal.delete_placement_metadata(bigint) void + | function citus_internal.delete_shard_metadata(bigint) void + | function citus_internal.delete_tenant_schema(oid) void + | function citus_internal.execute_command_on_remote_nodes_as_user(text,text) void + | function citus_internal.global_blocked_processes() SETOF record + | function citus_internal.is_replication_origin_tracking_active() boolean + | function citus_internal.local_blocked_processes() SETOF record + | function citus_internal.mark_node_not_synced(integer,integer) void + | function citus_internal.mark_object_distributed(oid,text,oid,text) void + | function citus_internal.start_management_transaction(xid8) void + | function citus_internal.start_replication_origin_tracking() void + | function citus_internal.stop_replication_origin_tracking() void + | function citus_internal.unregister_tenant_schema_globally(oid,text) void + | function citus_internal.update_none_dist_table_metadata(oid,"char",bigint,boolean) void + | function citus_internal.update_placement_metadata(bigint,integer,integer) void + | function citus_internal.update_relation_colocation(oid,integer) void + | function citus_unmark_object_distributed(oid,oid,integer,boolean) void +(30 rows) DROP TABLE multi_extension.prev_objects, multi_extension.extension_diff; -- show running version diff --git a/src/test/regress/expected/role_operations_from_non_maindb.out b/src/test/regress/expected/role_operations_from_non_maindb.out new file mode 100644 index 000000000..3b51c89b0 --- /dev/null +++ b/src/test/regress/expected/role_operations_from_non_maindb.out @@ -0,0 +1,138 @@ +-- Create a new database +set citus.enable_create_database_propagation to on; +CREATE DATABASE role_operations_test_db; +SET citus.superuser TO 'postgres'; +-- Connect to the new database +\c role_operations_test_db +-- Test CREATE ROLE with various options +CREATE ROLE test_role1 WITH LOGIN PASSWORD 'password1'; +\c role_operations_test_db - - :worker_1_port +CREATE USER "test_role2-needs\!escape" +WITH + SUPERUSER CREATEDB CREATEROLE INHERIT LOGIN REPLICATION BYPASSRLS CONNECTION +LIMIT 10 VALID UNTIL '2023-01-01' IN ROLE test_role1; +\c regression - - :master_port +select result FROM run_command_on_all_nodes($$ + SELECT array_to_json(array_agg(row_to_json(t))) + FROM ( + SELECT rolname, rolsuper, rolinherit, rolcreaterole, rolcreatedb, + rolcanlogin, rolreplication, rolbypassrls, rolconnlimit, + (rolpassword != '') as pass_not_empty, DATE(rolvaliduntil) + FROM pg_authid + WHERE rolname in ('test_role1', 'test_role2-needs\!escape') + ORDER BY rolname + ) t +$$); + result +--------------------------------------------------------------------- + [{"rolname":"test_role1","rolsuper":false,"rolinherit":true,"rolcreaterole":false,"rolcreatedb":false,"rolcanlogin":true,"rolreplication":false,"rolbypassrls":false,"rolconnlimit":-1,"pass_not_empty":true,"date":null},{"rolname":"test_role2-needs\\!escape","rolsuper":true,"rolinherit":true,"rolcreaterole":true,"rolcreatedb":true,"rolcanlogin":true,"rolreplication":true,"rolbypassrls":true,"rolconnlimit":10,"pass_not_empty":null,"date":"2023-01-01"}] + [{"rolname":"test_role1","rolsuper":false,"rolinherit":true,"rolcreaterole":false,"rolcreatedb":false,"rolcanlogin":true,"rolreplication":false,"rolbypassrls":false,"rolconnlimit":-1,"pass_not_empty":true,"date":null},{"rolname":"test_role2-needs\\!escape","rolsuper":true,"rolinherit":true,"rolcreaterole":true,"rolcreatedb":true,"rolcanlogin":true,"rolreplication":true,"rolbypassrls":true,"rolconnlimit":10,"pass_not_empty":null,"date":"2023-01-01"}] + [{"rolname":"test_role1","rolsuper":false,"rolinherit":true,"rolcreaterole":false,"rolcreatedb":false,"rolcanlogin":true,"rolreplication":false,"rolbypassrls":false,"rolconnlimit":-1,"pass_not_empty":true,"date":null},{"rolname":"test_role2-needs\\!escape","rolsuper":true,"rolinherit":true,"rolcreaterole":true,"rolcreatedb":true,"rolcanlogin":true,"rolreplication":true,"rolbypassrls":true,"rolconnlimit":10,"pass_not_empty":null,"date":"2023-01-01"}] +(3 rows) + +select result FROM run_command_on_all_nodes($$ + SELECT array_to_json(array_agg(row_to_json(t))) + FROM ( + SELECT r.rolname + FROM pg_dist_object d + JOIN pg_roles r ON d.objid = r.oid + WHERE r.rolname IN ('test_role1', 'test_role2-needs\!escape') + order by r.rolname + ) t +$$); + result +--------------------------------------------------------------------- + [{"rolname":"test_role1"},{"rolname":"test_role2-needs\\!escape"}] + [{"rolname":"test_role1"},{"rolname":"test_role2-needs\\!escape"}] + [{"rolname":"test_role1"},{"rolname":"test_role2-needs\\!escape"}] +(3 rows) + +\c role_operations_test_db - - :master_port +-- Test ALTER ROLE with various options +ALTER ROLE test_role1 WITH PASSWORD 'new_password1'; +\c role_operations_test_db - - :worker_1_port +ALTER USER "test_role2-needs\!escape" +WITH + NOSUPERUSER NOCREATEDB NOCREATEROLE NOINHERIT NOLOGIN NOREPLICATION NOBYPASSRLS CONNECTION +LIMIT 5 VALID UNTIL '2024-01-01'; +\c regression - - :master_port +select result FROM run_command_on_all_nodes($$ + SELECT array_to_json(array_agg(row_to_json(t))) + FROM ( + SELECT rolname, rolsuper, rolinherit, rolcreaterole, rolcreatedb, + rolcanlogin, rolreplication, rolbypassrls, rolconnlimit, + (rolpassword != '') as pass_not_empty, DATE(rolvaliduntil) + FROM pg_authid + WHERE rolname in ('test_role1', 'test_role2-needs\!escape') + ORDER BY rolname + ) t +$$); + result +--------------------------------------------------------------------- + [{"rolname":"test_role1","rolsuper":false,"rolinherit":true,"rolcreaterole":false,"rolcreatedb":false,"rolcanlogin":true,"rolreplication":false,"rolbypassrls":false,"rolconnlimit":-1,"pass_not_empty":true,"date":null},{"rolname":"test_role2-needs\\!escape","rolsuper":false,"rolinherit":false,"rolcreaterole":false,"rolcreatedb":false,"rolcanlogin":false,"rolreplication":false,"rolbypassrls":false,"rolconnlimit":5,"pass_not_empty":null,"date":"2024-01-01"}] + [{"rolname":"test_role1","rolsuper":false,"rolinherit":true,"rolcreaterole":false,"rolcreatedb":false,"rolcanlogin":true,"rolreplication":false,"rolbypassrls":false,"rolconnlimit":-1,"pass_not_empty":true,"date":null},{"rolname":"test_role2-needs\\!escape","rolsuper":false,"rolinherit":false,"rolcreaterole":false,"rolcreatedb":false,"rolcanlogin":false,"rolreplication":false,"rolbypassrls":false,"rolconnlimit":5,"pass_not_empty":null,"date":"2024-01-01"}] + [{"rolname":"test_role1","rolsuper":false,"rolinherit":true,"rolcreaterole":false,"rolcreatedb":false,"rolcanlogin":true,"rolreplication":false,"rolbypassrls":false,"rolconnlimit":-1,"pass_not_empty":true,"date":null},{"rolname":"test_role2-needs\\!escape","rolsuper":false,"rolinherit":false,"rolcreaterole":false,"rolcreatedb":false,"rolcanlogin":false,"rolreplication":false,"rolbypassrls":false,"rolconnlimit":5,"pass_not_empty":null,"date":"2024-01-01"}] +(3 rows) + +\c role_operations_test_db - - :master_port +-- Test DROP ROLE +DROP ROLE no_such_role; -- fails nicely +ERROR: role "no_such_role" does not exist +DROP ROLE IF EXISTS no_such_role; -- doesn't fail +NOTICE: role "no_such_role" does not exist, skipping +CREATE ROLE new_role; +DROP ROLE IF EXISTS no_such_role, new_role; -- doesn't fail +NOTICE: role "no_such_role" does not exist, skipping +DROP ROLE IF EXISTS test_role1, "test_role2-needs\!escape"; +\c regression - - :master_port +--verify that roles and dist_object are dropped +select result FROM run_command_on_all_nodes($$ + SELECT array_to_json(array_agg(row_to_json(t))) + FROM ( + SELECT rolname, rolsuper, rolinherit, rolcreaterole, rolcreatedb, + rolcanlogin, rolreplication, rolbypassrls, rolconnlimit, + (rolpassword != '') as pass_not_empty, DATE(rolvaliduntil) + FROM pg_authid + WHERE rolname in ('test_role1', 'test_role2-needs\!escape','new_role','no_such_role') + ORDER BY rolname + ) t +$$); + result +--------------------------------------------------------------------- + + + +(3 rows) + +select result FROM run_command_on_all_nodes($$ + SELECT array_to_json(array_agg(row_to_json(t))) + FROM ( + SELECT r.rolname + FROM pg_roles r + WHERE r.rolname IN ('test_role1', 'test_role2-needs\!escape','new_role','no_such_role') + order by r.rolname + ) t +$$); + result +--------------------------------------------------------------------- + + + +(3 rows) + +SELECT result FROM run_command_on_all_nodes($$ + SELECT count(*) leaked_pg_dist_object_records_for_roles + FROM pg_dist_object LEFT JOIN pg_authid ON (objid = oid) + WHERE classid = 1260 AND oid IS NULL +$$); + result +--------------------------------------------------------------------- + 0 + 0 + 0 +(3 rows) + +-- Clean up: drop the database +set citus.enable_create_database_propagation to on; +DROP DATABASE role_operations_test_db; +reset citus.enable_create_database_propagation; diff --git a/src/test/regress/expected/seclabel.out b/src/test/regress/expected/seclabel.out index ae6589734..ca6c6f984 100644 --- a/src/test/regress/expected/seclabel.out +++ b/src/test/regress/expected/seclabel.out @@ -167,9 +167,9 @@ SELECT node_type, result FROM get_citus_tests_label_provider_labels('"user 2"') SET citus.log_remote_commands TO on; SET citus.grep_remote_commands = '%SECURITY LABEL%'; SELECT 1 FROM citus_add_node('localhost', :worker_2_port); -NOTICE: issuing SELECT worker_create_or_alter_role('user1', 'CREATE ROLE user1 NOSUPERUSER NOCREATEDB NOCREATEROLE INHERIT NOLOGIN NOREPLICATION NOBYPASSRLS CONNECTION LIMIT -1 PASSWORD NULL VALID UNTIL ''infinity''', 'ALTER ROLE user1 NOSUPERUSER NOCREATEDB NOCREATEROLE INHERIT NOLOGIN NOREPLICATION NOBYPASSRLS CONNECTION LIMIT -1 PASSWORD NULL VALID UNTIL ''infinity''');SECURITY LABEL FOR "citus '!tests_label_provider" ON ROLE user1 IS 'citus_classified' +NOTICE: issuing SELECT worker_create_or_alter_role('user1', 'CREATE ROLE user1 NOSUPERUSER NOCREATEDB NOCREATEROLE INHERIT NOLOGIN NOREPLICATION NOBYPASSRLS CONNECTION LIMIT -1 PASSWORD NULL', 'ALTER ROLE user1 NOSUPERUSER NOCREATEDB NOCREATEROLE INHERIT NOLOGIN NOREPLICATION NOBYPASSRLS CONNECTION LIMIT -1 PASSWORD NULL');SECURITY LABEL FOR "citus '!tests_label_provider" ON ROLE user1 IS 'citus_classified' DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx -NOTICE: issuing SELECT worker_create_or_alter_role('user 2', 'CREATE ROLE "user 2" NOSUPERUSER NOCREATEDB NOCREATEROLE INHERIT NOLOGIN NOREPLICATION NOBYPASSRLS CONNECTION LIMIT -1 PASSWORD NULL VALID UNTIL ''infinity''', 'ALTER ROLE "user 2" NOSUPERUSER NOCREATEDB NOCREATEROLE INHERIT NOLOGIN NOREPLICATION NOBYPASSRLS CONNECTION LIMIT -1 PASSWORD NULL VALID UNTIL ''infinity''');SECURITY LABEL FOR "citus '!tests_label_provider" ON ROLE "user 2" IS 'citus ''!unclassified' +NOTICE: issuing SELECT worker_create_or_alter_role('user 2', 'CREATE ROLE "user 2" NOSUPERUSER NOCREATEDB NOCREATEROLE INHERIT NOLOGIN NOREPLICATION NOBYPASSRLS CONNECTION LIMIT -1 PASSWORD NULL', 'ALTER ROLE "user 2" NOSUPERUSER NOCREATEDB NOCREATEROLE INHERIT NOLOGIN NOREPLICATION NOBYPASSRLS CONNECTION LIMIT -1 PASSWORD NULL');SECURITY LABEL FOR "citus '!tests_label_provider" ON ROLE "user 2" IS 'citus ''!unclassified' DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx ?column? --------------------------------------------------------------------- diff --git a/src/test/regress/expected/upgrade_list_citus_objects.out b/src/test/regress/expected/upgrade_list_citus_objects.out index 4f17695be..ca31b222b 100644 --- a/src/test/regress/expected/upgrade_list_citus_objects.out +++ b/src/test/regress/expected/upgrade_list_citus_objects.out @@ -174,7 +174,7 @@ ORDER BY 1; function citus_text_send_as_jsonb(text) function citus_total_relation_size(regclass,boolean) function citus_truncate_trigger() - function citus_unmark_object_distributed(oid,oid,integer) + function citus_unmark_object_distributed(oid,oid,integer,boolean) function citus_update_node(integer,text,integer,boolean,integer) function citus_update_shard_statistics(bigint) function citus_update_table_statistics(regclass) diff --git a/src/test/regress/expected/upgrade_post_11_after.out b/src/test/regress/expected/upgrade_post_11_after.out index 422bc846f..49bd20432 100644 --- a/src/test/regress/expected/upgrade_post_11_after.out +++ b/src/test/regress/expected/upgrade_post_11_after.out @@ -67,6 +67,20 @@ SELECT 1 FROM run_command_on_workers($$SELECT pg_reload_conf()$$); 1 (2 rows) +-- In the version that we use for upgrade tests (v10.2.0), we propagate +-- "valid until" to the workers as "infinity" even if it's not set. And +-- given that "postgres" role is created in the older version, "valid until" +-- is set to "infinity" on the workers while this is not the case for +-- coordinator. See https://github.com/citusdata/citus/issues/7533. +-- +-- We're fixing this for new versions of Citus and we'll probably backport +-- this to some older versions too. However, v10.2.0 won't ever have this +-- fix. +-- +-- For this reason, here we set "valid until" to "infinity" for all the +-- nodes so that below query doesn't report any difference between the +-- metadata on coordinator and workers. +ALTER ROLE postgres WITH VALID UNTIL 'infinity'; -- make sure that the metadata is consistent across all nodes -- we exclude the distributed_object_data as they are -- not sorted in the same order (as OIDs differ on the nodes) diff --git a/src/test/regress/multi_schedule b/src/test/regress/multi_schedule index 85de7b8b8..af5921e60 100644 --- a/src/test/regress/multi_schedule +++ b/src/test/regress/multi_schedule @@ -103,13 +103,14 @@ test: multi_dropped_column_aliases foreign_key_restriction_enforcement test: binary_protocol test: alter_table_set_access_method test: alter_distributed_table -test: issue_5248 issue_5099 issue_5763 issue_6543 issue_6758 +test: issue_5248 issue_5099 issue_5763 issue_6543 issue_6758 issue_7477 test: object_propagation_debug test: undistribute_table test: run_command_on_all_nodes test: background_task_queue_monitor -test: other_databases grant_role_from_non_maindb seclabel_non_maindb +test: other_databases grant_role_from_non_maindb role_operations_from_non_maindb seclabel_non_maindb test: citus_internal_access +test: function_with_case_when # Causal clock test test: clock diff --git a/src/test/regress/pg_regress_multi.pl b/src/test/regress/pg_regress_multi.pl index c9a85d523..01e57c469 100755 --- a/src/test/regress/pg_regress_multi.pl +++ b/src/test/regress/pg_regress_multi.pl @@ -1126,16 +1126,33 @@ sub RunVanillaTests system("mkdir", ("-p", "$pgregressOutputdir/sql")) == 0 or die "Could not create vanilla sql dir."; - $exitcode = system("$plainRegress", - ("--dlpath", $dlpath), - ("--inputdir", $pgregressInputdir), - ("--outputdir", $pgregressOutputdir), - ("--schedule", catfile("$pgregressInputdir", "parallel_schedule")), - ("--use-existing"), - ("--host","$host"), - ("--port","$masterPort"), - ("--user","$user"), - ("--dbname", "$dbName")); + if ($majorversion >= "16") + { + $exitcode = system("$plainRegress", + ("--dlpath", $dlpath), + ("--inputdir", $pgregressInputdir), + ("--outputdir", $pgregressOutputdir), + ("--expecteddir", $pgregressOutputdir), + ("--schedule", catfile("$pgregressInputdir", "parallel_schedule")), + ("--use-existing"), + ("--host","$host"), + ("--port","$masterPort"), + ("--user","$user"), + ("--dbname", "$dbName")); + } + else + { + $exitcode = system("$plainRegress", + ("--dlpath", $dlpath), + ("--inputdir", $pgregressInputdir), + ("--outputdir", $pgregressOutputdir), + ("--schedule", catfile("$pgregressInputdir", "parallel_schedule")), + ("--use-existing"), + ("--host","$host"), + ("--port","$masterPort"), + ("--user","$user"), + ("--dbname", "$dbName")); + } } if ($useMitmproxy) { diff --git a/src/test/regress/sql/alter_table_add_column.sql b/src/test/regress/sql/alter_table_add_column.sql index 255e7714f..355667842 100644 --- a/src/test/regress/sql/alter_table_add_column.sql +++ b/src/test/regress/sql/alter_table_add_column.sql @@ -41,6 +41,10 @@ ALTER TABLE referencing ADD COLUMN "test_\'!7" "simple_!\'custom_type"; ALTER TABLE referencing ADD COLUMN test_8 integer CHECK (test_8 > 0); ALTER TABLE referencing ADD COLUMN test_8 integer CONSTRAINT check_test_8 CHECK (test_8 > 0); +-- error out properly even if the REFERENCES does not include the column list of the referenced table +ALTER TABLE referencing ADD COLUMN test_9 bool, ADD COLUMN test_10 int REFERENCES referenced; +ALTER TABLE referencing ADD COLUMN test_9 bool, ADD COLUMN test_10 int REFERENCES referenced(int_col); + -- try to add test_6 again, but with IF NOT EXISTS ALTER TABLE referencing ADD COLUMN IF NOT EXISTS test_6 text; ALTER TABLE referencing ADD COLUMN IF NOT EXISTS test_6 integer; diff --git a/src/test/regress/sql/function_with_case_when.sql b/src/test/regress/sql/function_with_case_when.sql new file mode 100644 index 000000000..03c6678e4 --- /dev/null +++ b/src/test/regress/sql/function_with_case_when.sql @@ -0,0 +1,27 @@ +CREATE SCHEMA function_with_case; +SET search_path TO function_with_case; + +-- create function +CREATE OR REPLACE FUNCTION test_err(v1 text) + RETURNS text + LANGUAGE plpgsql + SECURITY DEFINER +AS $function$ + +begin + return v1 || ' - ok'; +END; +$function$; +do $$ declare + lNewValues text; + val text; +begin + val = 'test'; + lNewValues = test_err(v1 => case when val::text = 'test'::text then 'yes' else 'no' end); + raise notice 'lNewValues= %', lNewValues; +end;$$ ; + +-- call function +SELECT test_err('test'); + +DROP SCHEMA function_with_case CASCADE; diff --git a/src/test/regress/sql/issue_7477.sql b/src/test/regress/sql/issue_7477.sql new file mode 100644 index 000000000..b9c1578e9 --- /dev/null +++ b/src/test/regress/sql/issue_7477.sql @@ -0,0 +1,44 @@ + +--- Test for updating a table that has a foreign key reference to another reference table. +--- Issue #7477: Distributed deadlock after issuing a simple UPDATE statement +--- https://github.com/citusdata/citus/issues/7477 + +CREATE TABLE table1 (id INT PRIMARY KEY); +SELECT create_reference_table('table1'); +INSERT INTO table1 VALUES (1); + +CREATE TABLE table2 ( + id INT, + info TEXT, + CONSTRAINT table1_id_fk FOREIGN KEY (id) REFERENCES table1 (id) + ); +SELECT create_reference_table('table2'); +INSERT INTO table2 VALUES (1, 'test'); + +--- Runs the update command in parallel on workers. +--- Due to bug #7477, before the fix, the result is non-deterministic +--- and have several rows of the form: +--- localhost | 57638 | f | ERROR: deadlock detected +--- localhost | 57637 | f | ERROR: deadlock detected +--- localhost | 57637 | f | ERROR: canceling the transaction since it was involved in a distributed deadlock + +SELECT * FROM master_run_on_worker( + ARRAY['localhost', 'localhost','localhost', 'localhost','localhost', + 'localhost','localhost', 'localhost','localhost', 'localhost']::text[], + ARRAY[57638, 57637, 57637, 57638, 57637, 57638, 57637, 57638, 57638, 57637]::int[], + ARRAY['UPDATE table2 SET info = ''test_update'' WHERE id = 1', + 'UPDATE table2 SET info = ''test_update'' WHERE id = 1', + 'UPDATE table2 SET info = ''test_update'' WHERE id = 1', + 'UPDATE table2 SET info = ''test_update'' WHERE id = 1', + 'UPDATE table2 SET info = ''test_update'' WHERE id = 1', + 'UPDATE table2 SET info = ''test_update'' WHERE id = 1', + 'UPDATE table2 SET info = ''test_update'' WHERE id = 1', + 'UPDATE table2 SET info = ''test_update'' WHERE id = 1', + 'UPDATE table2 SET info = ''test_update'' WHERE id = 1', + 'UPDATE table2 SET info = ''test_update'' WHERE id = 1' + ]::text[], + true); + +--- cleanup +DROP TABLE table2; +DROP TABLE table1; diff --git a/src/test/regress/sql/metadata_sync_from_non_maindb.sql b/src/test/regress/sql/metadata_sync_from_non_maindb.sql index 93445be27..62760c6cc 100644 --- a/src/test/regress/sql/metadata_sync_from_non_maindb.sql +++ b/src/test/regress/sql/metadata_sync_from_non_maindb.sql @@ -80,9 +80,109 @@ revoke CREATE on database metadata_sync_2pc_db from "grant_role2pc'_user1"; \c regression drop user "grant_role2pc'_user1","grant_role2pc'_user2","grant_role2pc'_user3",grant_role2pc_user4,grant_role2pc_user5; +--test for user operations + +--test for create user +\c regression - - :master_port +select 1 from citus_remove_node('localhost', :worker_2_port); + +\c metadata_sync_2pc_db - - :master_port +CREATE ROLE test_role1 WITH LOGIN PASSWORD 'password1'; + +\c metadata_sync_2pc_db - - :worker_1_port +CREATE USER "test_role2-needs\!escape" +WITH + SUPERUSER CREATEDB CREATEROLE INHERIT LOGIN REPLICATION BYPASSRLS CONNECTION +LIMIT 10 VALID UNTIL '2023-01-01' IN ROLE test_role1; + +create role test_role3; + +\c regression - - :master_port +select 1 from citus_add_node('localhost', :worker_2_port); + +select result FROM run_command_on_all_nodes($$ + SELECT array_to_json(array_agg(row_to_json(t))) + FROM ( + SELECT rolname, rolsuper, rolinherit, rolcreaterole, rolcreatedb, + rolcanlogin, rolreplication, rolbypassrls, rolconnlimit, + (rolpassword != '') as pass_not_empty, DATE(rolvaliduntil) + FROM pg_authid + WHERE rolname in ('test_role1', 'test_role2-needs\!escape','test_role3') + ORDER BY rolname + ) t +$$); + +--test for alter user +select 1 from citus_remove_node('localhost', :worker_2_port); +\c metadata_sync_2pc_db - - :master_port +-- Test ALTER ROLE with various options +ALTER ROLE test_role1 WITH PASSWORD 'new_password1'; + +\c metadata_sync_2pc_db - - :worker_1_port +ALTER USER "test_role2-needs\!escape" +WITH + NOSUPERUSER NOCREATEDB NOCREATEROLE NOINHERIT NOLOGIN NOREPLICATION NOBYPASSRLS CONNECTION +LIMIT 5 VALID UNTIL '2024-01-01'; + +\c regression - - :master_port +select 1 from citus_add_node('localhost', :worker_2_port); + +select result FROM run_command_on_all_nodes($$ + SELECT array_to_json(array_agg(row_to_json(t))) + FROM ( + SELECT rolname, rolsuper, rolinherit, rolcreaterole, rolcreatedb, + rolcanlogin, rolreplication, rolbypassrls, rolconnlimit, + (rolpassword != '') as pass_not_empty, DATE(rolvaliduntil) + FROM pg_authid + WHERE rolname in ('test_role1', 'test_role2-needs\!escape','test_role3') + ORDER BY rolname + ) t +$$); + +--test for drop user +select 1 from citus_remove_node('localhost', :worker_2_port); + +\c metadata_sync_2pc_db - - :worker_1_port +DROP ROLE test_role1, "test_role2-needs\!escape"; + +\c metadata_sync_2pc_db - - :master_port +DROP ROLE test_role3; + +\c regression - - :master_port +select 1 from citus_add_node('localhost', :worker_2_port); + +select result FROM run_command_on_all_nodes($$ + SELECT array_to_json(array_agg(row_to_json(t))) + FROM ( + SELECT rolname, rolsuper, rolinherit, rolcreaterole, rolcreatedb, + rolcanlogin, rolreplication, rolbypassrls, rolconnlimit, + (rolpassword != '') as pass_not_empty, DATE(rolvaliduntil) + FROM pg_authid + WHERE rolname in ('test_role1', 'test_role2-needs\!escape','test_role3') + ORDER BY rolname + ) t +$$); + +-- Clean up: drop the database on worker node 2 +\c regression - - :worker_2_port +DROP ROLE if exists test_role1, "test_role2-needs\!escape", test_role3; + +\c regression - - :master_port + +select result FROM run_command_on_all_nodes($$ + SELECT array_to_json(array_agg(row_to_json(t))) + FROM ( + SELECT rolname, rolsuper, rolinherit, rolcreaterole, rolcreatedb, + rolcanlogin, rolreplication, rolbypassrls, rolconnlimit, + (rolpassword != '') as pass_not_empty, DATE(rolvaliduntil) + FROM pg_authid + WHERE rolname in ('test_role1', 'test_role2-needs\!escape','test_role3') + ORDER BY rolname + ) t +$$); + set citus.enable_create_database_propagation to on; drop database metadata_sync_2pc_db; drop schema metadata_sync_2pc_schema; - reset citus.enable_create_database_propagation; reset search_path; diff --git a/src/test/regress/sql/role_operations_from_non_maindb.sql b/src/test/regress/sql/role_operations_from_non_maindb.sql new file mode 100644 index 000000000..5f569208b --- /dev/null +++ b/src/test/regress/sql/role_operations_from_non_maindb.sql @@ -0,0 +1,106 @@ +-- Create a new database +set citus.enable_create_database_propagation to on; +CREATE DATABASE role_operations_test_db; +SET citus.superuser TO 'postgres'; +-- Connect to the new database +\c role_operations_test_db +-- Test CREATE ROLE with various options +CREATE ROLE test_role1 WITH LOGIN PASSWORD 'password1'; + +\c role_operations_test_db - - :worker_1_port +CREATE USER "test_role2-needs\!escape" +WITH + SUPERUSER CREATEDB CREATEROLE INHERIT LOGIN REPLICATION BYPASSRLS CONNECTION +LIMIT 10 VALID UNTIL '2023-01-01' IN ROLE test_role1; + +\c regression - - :master_port + +select result FROM run_command_on_all_nodes($$ + SELECT array_to_json(array_agg(row_to_json(t))) + FROM ( + SELECT rolname, rolsuper, rolinherit, rolcreaterole, rolcreatedb, + rolcanlogin, rolreplication, rolbypassrls, rolconnlimit, + (rolpassword != '') as pass_not_empty, DATE(rolvaliduntil) + FROM pg_authid + WHERE rolname in ('test_role1', 'test_role2-needs\!escape') + ORDER BY rolname + ) t +$$); + +select result FROM run_command_on_all_nodes($$ + SELECT array_to_json(array_agg(row_to_json(t))) + FROM ( + SELECT r.rolname + FROM pg_dist_object d + JOIN pg_roles r ON d.objid = r.oid + WHERE r.rolname IN ('test_role1', 'test_role2-needs\!escape') + order by r.rolname + ) t +$$); + +\c role_operations_test_db - - :master_port +-- Test ALTER ROLE with various options +ALTER ROLE test_role1 WITH PASSWORD 'new_password1'; + +\c role_operations_test_db - - :worker_1_port +ALTER USER "test_role2-needs\!escape" +WITH + NOSUPERUSER NOCREATEDB NOCREATEROLE NOINHERIT NOLOGIN NOREPLICATION NOBYPASSRLS CONNECTION +LIMIT 5 VALID UNTIL '2024-01-01'; + +\c regression - - :master_port +select result FROM run_command_on_all_nodes($$ + SELECT array_to_json(array_agg(row_to_json(t))) + FROM ( + SELECT rolname, rolsuper, rolinherit, rolcreaterole, rolcreatedb, + rolcanlogin, rolreplication, rolbypassrls, rolconnlimit, + (rolpassword != '') as pass_not_empty, DATE(rolvaliduntil) + FROM pg_authid + WHERE rolname in ('test_role1', 'test_role2-needs\!escape') + ORDER BY rolname + ) t +$$); + +\c role_operations_test_db - - :master_port +-- Test DROP ROLE +DROP ROLE no_such_role; -- fails nicely +DROP ROLE IF EXISTS no_such_role; -- doesn't fail + +CREATE ROLE new_role; +DROP ROLE IF EXISTS no_such_role, new_role; -- doesn't fail +DROP ROLE IF EXISTS test_role1, "test_role2-needs\!escape"; + +\c regression - - :master_port +--verify that roles and dist_object are dropped +select result FROM run_command_on_all_nodes($$ + SELECT array_to_json(array_agg(row_to_json(t))) + FROM ( + SELECT rolname, rolsuper, rolinherit, rolcreaterole, rolcreatedb, + rolcanlogin, rolreplication, rolbypassrls, rolconnlimit, + (rolpassword != '') as pass_not_empty, DATE(rolvaliduntil) + FROM pg_authid + WHERE rolname in ('test_role1', 'test_role2-needs\!escape','new_role','no_such_role') + ORDER BY rolname + ) t +$$); + +select result FROM run_command_on_all_nodes($$ + SELECT array_to_json(array_agg(row_to_json(t))) + FROM ( + SELECT r.rolname + FROM pg_roles r + WHERE r.rolname IN ('test_role1', 'test_role2-needs\!escape','new_role','no_such_role') + order by r.rolname + ) t +$$); + +SELECT result FROM run_command_on_all_nodes($$ + SELECT count(*) leaked_pg_dist_object_records_for_roles + FROM pg_dist_object LEFT JOIN pg_authid ON (objid = oid) + WHERE classid = 1260 AND oid IS NULL +$$); + +-- Clean up: drop the database +set citus.enable_create_database_propagation to on; +DROP DATABASE role_operations_test_db; +reset citus.enable_create_database_propagation; diff --git a/src/test/regress/sql/upgrade_post_11_after.sql b/src/test/regress/sql/upgrade_post_11_after.sql index ba9b12f3b..6d948ec34 100644 --- a/src/test/regress/sql/upgrade_post_11_after.sql +++ b/src/test/regress/sql/upgrade_post_11_after.sql @@ -27,6 +27,21 @@ SET datestyle = "ISO, YMD"; SELECT 1 FROM run_command_on_workers($$ALTER SYSTEM SET datestyle = "ISO, YMD";$$); SELECT 1 FROM run_command_on_workers($$SELECT pg_reload_conf()$$); +-- In the version that we use for upgrade tests (v10.2.0), we propagate +-- "valid until" to the workers as "infinity" even if it's not set. And +-- given that "postgres" role is created in the older version, "valid until" +-- is set to "infinity" on the workers while this is not the case for +-- coordinator. See https://github.com/citusdata/citus/issues/7533. +-- +-- We're fixing this for new versions of Citus and we'll probably backport +-- this to some older versions too. However, v10.2.0 won't ever have this +-- fix. +-- +-- For this reason, here we set "valid until" to "infinity" for all the +-- nodes so that below query doesn't report any difference between the +-- metadata on coordinator and workers. +ALTER ROLE postgres WITH VALID UNTIL 'infinity'; + -- make sure that the metadata is consistent across all nodes -- we exclude the distributed_object_data as they are -- not sorted in the same order (as OIDs differ on the nodes)