Merge pull request #3510 from citusdata/alter-role-set-propagation

In PostgreSQL, user defaults for config parameters can be changed by
ALTER ROLE .. SET statements. We wish to propagate those defaults
across the Citus cluster so that the behavior will be similar in
different workers.

The defaults can either be set in a specific database, or the whole
cluster, similarly they can be set for a single role or all roles.

We propagate the ALTER ROLE .. SET if all the conditions below are met:
- The query affects the current database, or all databases
- The user is already created in worker nodes

src/backend/distributed/commands/distribute_object_ops.c

@@ -99,6 +99,13 @@ static DistributeObjectOps Any_AlterRole = {
 	.postprocess = PostprocessAlterRoleStmt,
 	.address = NULL,
 };
+static DistributeObjectOps Any_AlterRoleSet = {
+	.deparse = DeparseAlterRoleSetStmt,
+	.qualify = QualifyAlterRoleSetStmt,
+	.preprocess = PreprocessAlterRoleSetStmt,
+	.postprocess = NULL,
+	.address = NULL,
+};
 static DistributeObjectOps Any_AlterTableMoveAll = {
 	.deparse = NULL,
 	.qualify = NULL,
@@ -598,6 +605,11 @@ GetDistributeObjectOps(Node *node)
 			return &Any_AlterRole;
 		}
 
+		case T_AlterRoleSetStmt:
+		{
+			return &Any_AlterRoleSet;
+		}
+
 		case T_AlterTableStmt:
 		{
 			AlterTableStmt *stmt = castNode(AlterTableStmt, node);

src/backend/distributed/commands/role.c

@@ -17,27 +17,51 @@
 #endif
 #include "catalog/catalog.h"
 #include "catalog/pg_authid.h"
+#include "catalog/pg_db_role_setting.h"
+#include "catalog/pg_type.h"
+#include "commands/dbcommands.h"
 #include "distributed/citus_ruleutils.h"
+#include "distributed/citus_safe_lib.h"
 #include "distributed/commands.h"
 #include "distributed/commands/utility_hook.h"
 #include "distributed/deparser.h"
 #include "distributed/listutils.h"
 #include "distributed/master_protocol.h"
+#include "distributed/metadata_sync.h"
 #include "distributed/worker_transaction.h"
+#include "miscadmin.h"
 #include "nodes/makefuncs.h"
 #include "nodes/parsenodes.h"
+#include "nodes/pg_list.h"
 #include "utils/acl.h"
 #include "utils/builtins.h"
+#include "utils/guc_tables.h"
+#include "utils/guc.h"
 #include "utils/rel.h"
 #include "utils/syscache.h"
 
 static const char * ExtractEncryptedPassword(Oid roleOid);
 static const char * CreateAlterRoleIfExistsCommand(AlterRoleStmt *stmt);
+static const char * CreateAlterRoleSetIfExistsCommand(AlterRoleSetStmt *stmt);
+static bool ShouldPropagateAlterRoleSetQueries(HeapTuple tuple,
+											   TupleDesc DbRoleSettingDescription);
 static DefElem * makeDefElemInt(char *name, int value);
 
+static char * GetRoleNameFromDbRoleSetting(HeapTuple tuple,
+										   TupleDesc DbRoleSettingDescription);
+static char * GetDatabaseNameFromDbRoleSetting(HeapTuple tuple,
+											   TupleDesc DbRoleSettingDescription);
+static Node * makeStringConst(char *str, int location);
+static Node * makeIntConst(int val, int location);
+static Node * makeFloatConst(char *str, int location);
+static const char * WrapQueryInAlterRoleIfExistsCall(const char *query, RoleSpec *role);
+static VariableSetStmt * MakeVariableSetStmt(const char *config);
+static int ConfigGenericNameCompare(const void *lhs, const void *rhs);
+
 /* controlled via GUC */
 bool EnableAlterRolePropagation = false;
 
+
 /*
  * PostprocessAlterRoleStmt actually creates the plan we need to execute for alter
  * role statement. We need to do it this way because we need to use the encrypted
@@ -87,6 +111,33 @@ PostprocessAlterRoleStmt(Node *node, const char *queryString)
 }
 
 
+/*
+ * PreprocessAlterRoleSetStmt actually creates the plan we need to execute for alter
+ * role set statement.
+ */
+List *
+PreprocessAlterRoleSetStmt(Node *node, const char *queryString)
+{
+	if (!EnableAlterRolePropagation)
+	{
+		return NIL;
+	}
+
+	EnsureCoordinator();
+
+	AlterRoleSetStmt *stmt = castNode(AlterRoleSetStmt, node);
+
+	QualifyTreeNode((Node *) stmt);
+	const char *sql = DeparseTreeNode((Node *) stmt);
+
+	List *commandList = list_make3(DISABLE_DDL_PROPAGATION,
+								   (void *) sql,
+								   ENABLE_DDL_PROPAGATION);
+
+	return NodeDDLTaskList(ALL_WORKERS, commandList);
+}
+
+
 /*
  * CreateAlterRoleIfExistsCommand creates ALTER ROLE command, from the alter role node
  *  using the alter_role_if_exists() UDF.
@@ -94,17 +145,54 @@ PostprocessAlterRoleStmt(Node *node, const char *queryString)
 static const char *
 CreateAlterRoleIfExistsCommand(AlterRoleStmt *stmt)
 {
-	StringInfoData alterRoleQueryBuffer = { 0 };
-	const char *roleName = RoleSpecString(stmt->role, false);
 	const char *alterRoleQuery = DeparseTreeNode((Node *) stmt);
+	return WrapQueryInAlterRoleIfExistsCall(alterRoleQuery, stmt->role);
+}
 
-	initStringInfo(&alterRoleQueryBuffer);
+
-	appendStringInfo(&alterRoleQueryBuffer,
+/*
+ * CreateAlterRoleSetIfExistsCommand creates ALTER ROLE .. SET command, from the
+ * AlterRoleSetStmt node.
+ *
+ * If the statement affects a single user, the query is wrapped in a
+ * alter_role_if_exists() to make sure that it is run on workers that has a user
+ * with the same name. If the query is a ALTER ROLE ALL .. SET query, the query
+ * is sent to the workers as is.
+ */
+static const char *
+CreateAlterRoleSetIfExistsCommand(AlterRoleSetStmt *stmt)
+{
+	char *alterRoleSetQuery = DeparseTreeNode((Node *) stmt);
+
+	/* ALTER ROLE ALL .. SET queries should not be wrapped in a alter_role_if_exists() call */
+	if (stmt->role == NULL)
+	{
+		return alterRoleSetQuery;
+	}
+	else
+	{
+		return WrapQueryInAlterRoleIfExistsCall(alterRoleSetQuery, stmt->role);
+	}
+}
+
+
+/*
+ * WrapQueryInAlterRoleIfExistsCall wraps a given query in a alter_role_if_exists()
+ * UDF.
+ */
+static const char *
+WrapQueryInAlterRoleIfExistsCall(const char *query, RoleSpec *role)
+{
+	StringInfoData buffer = { 0 };
+
+	const char *roleName = RoleSpecString(role, false);
+	initStringInfo(&buffer);
+	appendStringInfo(&buffer,
 					 "SELECT alter_role_if_exists(%s, %s)",
 					 quote_literal_cstr(roleName),
-					 quote_literal_cstr(alterRoleQuery));
+					 quote_literal_cstr(query));
 
-	return alterRoleQueryBuffer.data;
+	return buffer.data;
 }
 
 
@@ -140,6 +228,84 @@ ExtractEncryptedPassword(Oid roleOid)
 }
 
 
+/*
+ * GenerateAlterRoleSetIfExistsCommandList generate a list of ALTER ROLE .. SET commands that
+ * copies a role session defaults from the pg_db_role_settings table.
+ */
+static List *
+GenerateAlterRoleSetIfExistsCommandList(HeapTuple tuple,
+										TupleDesc DbRoleSettingDescription)
+{
+	AlterRoleSetStmt *stmt = makeNode(AlterRoleSetStmt);
+	List *commandList = NIL;
+	bool isnull = false;
+
+	char *databaseName =
+		GetDatabaseNameFromDbRoleSetting(tuple, DbRoleSettingDescription);
+
+	if (databaseName != NULL)
+	{
+		stmt->database = databaseName;
+	}
+
+	char *roleName = GetRoleNameFromDbRoleSetting(tuple, DbRoleSettingDescription);
+
+	if (roleName != NULL)
+	{
+		stmt->role = makeNode(RoleSpec);
+		stmt->role->location = -1;
+		stmt->role->roletype = ROLESPEC_CSTRING;
+		stmt->role->rolename = roleName;
+	}
+
+	Datum setconfig = heap_getattr(tuple, Anum_pg_db_role_setting_setconfig,
+								   DbRoleSettingDescription, &isnull);
+
+	Datum *configs;
+	int nconfigs;
+	int i;
+
+	deconstruct_array(DatumGetArrayTypeP(setconfig),
+					  TEXTOID, -1, false, 'i',
+					  &configs, NULL, &nconfigs);
+
+	/*
+	 * A tuple might contain one or more settings that apply to the user-database combination.
+	 * ALTER ROLE ... SET ... only allows to set one at a time. We will create a statement for every
+	 * configuration contained in the tuple.
+	 */
+	for (i = 0; i < nconfigs; i++)
+	{
+		char *config = TextDatumGetCString(configs[i]);
+		stmt->setstmt = MakeVariableSetStmt(config);
+		commandList = lappend(commandList,
+							  (void *) CreateAlterRoleSetIfExistsCommand(stmt));
+	}
+	return commandList;
+}
+
+
+/*
+ * MakeVariableSetStmt takes a "some-option=some value" string and creates a
+ * VariableSetStmt Node.
+ */
+static VariableSetStmt *
+MakeVariableSetStmt(const char *config)
+{
+	char *name = NULL;
+	char *value = NULL;
+
+	ParseLongOption(config, &name, &value);
+
+	VariableSetStmt *variableSetStmt = makeNode(VariableSetStmt);
+	variableSetStmt->kind = VAR_SET_VALUE;
+	variableSetStmt->name = name;
+	variableSetStmt->args = list_make1(MakeSetStatementArgument(name, value));
+
+	return variableSetStmt;
+}
+
+
 /*
  * GenerateAlterRoleIfExistsCommand generate ALTER ROLE command that copies a role from
  * the pg_authid table.
@@ -267,6 +433,43 @@ GenerateAlterRoleIfExistsCommandAllRoles()
 }
 
 
+/*
+ * GenerateAlterRoleSetIfExistsCommands creates ALTER ROLE .. SET commands
+ * that copies all session defaults for roles from the pg_db_role_setting table.
+ */
+List *
+GenerateAlterRoleSetIfExistsCommands()
+{
+	Relation DbRoleSetting = heap_open(DbRoleSettingRelationId, AccessShareLock);
+	TupleDesc DbRoleSettingDescription = RelationGetDescr(DbRoleSetting);
+	HeapTuple tuple = NULL;
+	List *commands = NIL;
+	List *alterRoleSetQueries = NIL;
+
+
+#if PG_VERSION_NUM >= 120000
+	TableScanDesc scan = table_beginscan_catalog(DbRoleSetting, 0, NULL);
+#else
+	HeapScanDesc scan = heap_beginscan_catalog(DbRoleSetting, 0, NULL);
+#endif
+
+	while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL)
+	{
+		if (ShouldPropagateAlterRoleSetQueries(tuple, DbRoleSettingDescription))
+		{
+			alterRoleSetQueries =
+				GenerateAlterRoleSetIfExistsCommandList(tuple, DbRoleSettingDescription);
+			commands = list_concat(commands, alterRoleSetQueries);
+		}
+	}
+
+	heap_endscan(scan);
+	heap_close(DbRoleSetting, AccessShareLock);
+
+	return commands;
+}
+
+
 /*
  * makeDefElemInt creates a DefElem with integer typed value with -1 as location.
  */
@@ -275,3 +478,250 @@ makeDefElemInt(char *name, int value)
 {
 	return makeDefElem(name, (Node *) makeInteger(value), -1);
 }
+
+
+/*
+ * GetDatabaseNameFromDbRoleSetting performs a lookup, and finds the database name
+ * associated DbRoleSetting Tuple
+ */
+static char *
+GetDatabaseNameFromDbRoleSetting(HeapTuple tuple, TupleDesc DbRoleSettingDescription)
+{
+	bool isnull;
+
+	Datum setdatabase = heap_getattr(tuple, Anum_pg_db_role_setting_setdatabase,
+									 DbRoleSettingDescription, &isnull);
+
+	if (isnull)
+	{
+		return NULL;
+	}
+
+	Oid databaseId = DatumGetObjectId(setdatabase);
+	char *databaseName = get_database_name(databaseId);
+
+	return databaseName;
+}
+
+
+/*
+ * GetRoleNameFromDbRoleSetting performs a lookup, and finds the role name
+ * associated DbRoleSetting Tuple
+ */
+static char *
+GetRoleNameFromDbRoleSetting(HeapTuple tuple, TupleDesc DbRoleSettingDescription)
+{
+	bool isnull;
+
+	Datum setrole = heap_getattr(tuple, Anum_pg_db_role_setting_setrole,
+								 DbRoleSettingDescription, &isnull);
+
+	if (isnull)
+	{
+		return NULL;
+	}
+
+	Oid roleId = DatumGetObjectId(setrole);
+	char *roleName = GetUserNameFromId(roleId, true);
+
+	return roleName;
+}
+
+
+/*
+ * MakeSetStatementArgs parses a configuraton value and creates an A_Const
+ * with an appropriate type.
+ *
+ * The allowed A_Const types are Integer, Float, and String.
+ */
+Node *
+MakeSetStatementArgument(char *configurationName, char *configurationValue)
+{
+	Node *arg = NULL;
+	char **key = &configurationName;
+
+	/* Perform a lookup on GUC variables to find the config type and units.
+	 * All user-defined GUCs have string values, but we need to perform a search
+	 * on all the GUCs to understand if it is a user-defined one or not.
+	 *
+	 * Note: get_guc_variables() is intended for internal use only, but there
+	 * is no other way to determine allowed units, and value types other than
+	 * using this function
+	 */
+	struct config_generic **gucVariables = get_guc_variables();
+	int numOpts = GetNumConfigOptions();
+	struct config_generic **matchingConfig =
+		(struct config_generic **) SafeBsearch((void *) &key,
+											   (void *) gucVariables,
+											   numOpts,
+											   sizeof(struct config_generic *),
+											   ConfigGenericNameCompare);
+
+	/* If the config is not user-defined, lookup the variable type to contruct the arguments */
+	if (matchingConfig != NULL)
+	{
+		switch ((*matchingConfig)->vartype)
+		{
+			/* We use postgresql parser so that we will parse the units only if
+			 * the configuration paramater allows it.
+			 *
+			 * e.g. `SET statement_timeout = '1min'` will be parsed as 60000 since
+			 * the value is stored in units of ms internally.
+			 */
+			case PGC_INT:
+			{
+				int intValue;
+				parse_int(configurationValue, &intValue,
+						  (*matchingConfig)->flags, NULL);
+				arg = makeIntConst(intValue, -1);
+				break;
+			}
+
+			case PGC_REAL:
+			{
+				arg = makeFloatConst(configurationValue, -1);
+				break;
+			}
+
+			case PGC_BOOL:
+			case PGC_STRING:
+			case PGC_ENUM:
+			{
+				arg = makeStringConst(configurationValue, -1);
+				break;
+			}
+
+			default:
+			{
+				ereport(ERROR, (errmsg("Unrecognized run-time parameter type for %s",
+									   configurationName)));
+				break;
+			}
+		}
+	}
+	else
+	{
+		arg = makeStringConst(configurationValue, -1);
+	}
+	return (Node *) arg;
+}
+
+
+/*
+ * makeStringConst creates a Const Node that stores a given string
+ *
+ * copied from backend/parser/gram.c
+ */
+static Node *
+makeStringConst(char *str, int location)
+{
+	A_Const *n = makeNode(A_Const);
+
+	n->val.type = T_String;
+	n->val.val.str = str;
+	n->location = location;
+
+	return (Node *) n;
+}
+
+
+/*
+ * makeIntConst creates a Const Node that stores a given integer
+ *
+ * copied from backend/parser/gram.c
+ */
+static Node *
+makeIntConst(int val, int location)
+{
+	A_Const *n = makeNode(A_Const);
+
+	n->val.type = T_Integer;
+	n->val.val.ival = val;
+	n->location = location;
+
+	return (Node *) n;
+}
+
+
+/*
+ * makeIntConst creates a Const Node that stores a given Float
+ *
+ * copied from backend/parser/gram.c
+ */
+static Node *
+makeFloatConst(char *str, int location)
+{
+	A_Const *n = makeNode(A_Const);
+
+	n->val.type = T_Float;
+	n->val.val.str = str;
+	n->location = location;
+
+	return (Node *) n;
+}
+
+
+/*
+ * ConfigGenericNameCompare compares two config_generic structs based on their
+ * name fields. If the name fields contain the same strings two structs are
+ * considered to be equal.
+ *
+ * copied from guc_var_compare in utils/misc/guc.c
+ */
+static int
+ConfigGenericNameCompare(const void *a, const void *b)
+{
+	const struct config_generic *confa = *(struct config_generic *const *) a;
+	const struct config_generic *confb = *(struct config_generic *const *) b;
+
+	/*
+	 * guc_var_compare used a custom comparison function here to allow stable
+	 * ordering, but we do not need it here as we only perform a lookup, and do
+	 * not use this function to order the guc list.
+	 */
+	return pg_strcasecmp(confa->name, confb->name);
+}
+
+
+/*
+ * ShouldPropagateAlterRoleSetQueries decides if the set of AlterRoleSetStmt
+ * queries should be propagated to worker nodes
+ *
+ * A single DbRoleSetting tuple can be used to create multiple AlterRoleSetStmt
+ * queries as all of the configs are stored in a text[] column and each entry
+ * creates a seperate statement
+ */
+static bool
+ShouldPropagateAlterRoleSetQueries(HeapTuple tuple,
+								   TupleDesc DbRoleSettingDescription)
+{
+	if (!ShouldPropagate())
+	{
+		return false;
+	}
+
+	const char *currentDatabaseName = CurrentDatabaseName();
+	const char *databaseName =
+		GetDatabaseNameFromDbRoleSetting(tuple, DbRoleSettingDescription);
+	const char *roleName = GetRoleNameFromDbRoleSetting(tuple, DbRoleSettingDescription);
+
+	/*
+	 * session defaults for databases other than the current one are not propagated
+	 */
+	if (databaseName != NULL &&
+		pg_strcasecmp(databaseName, currentDatabaseName) != 0)
+	{
+		return false;
+	}
+
+	/*
+	 * default roles are skipped, because reserved roles
+	 * cannot be altered.
+	 */
+	if (roleName != NULL && IsReservedName(roleName))
+	{
+		return false;
+	}
+
+	return true;
+}

src/backend/distributed/commands/utility_hook.c

@@ -555,18 +555,6 @@ multi_ProcessUtility(PlannedStmt *pstmt,
 			}
 		}
 
-		if (IsA(parsetree, AlterRoleSetStmt) && EnableAlterRolePropagation)
-		{
-			ereport(NOTICE, (errmsg("Citus partially supports ALTER ROLE for "
-									"distributed databases"),
-
-							 errdetail(
-								 "Citus does not propagate ALTER ROLE ... SET/RESET "
-								 "commands to workers"),
-
-							 errhint("You can manually alter roles on workers.")));
-		}
-
 		if (IsA(parsetree, RenameStmt) && ((RenameStmt *) parsetree)->renameType ==
 			OBJECT_ROLE && EnableAlterRolePropagation)
 		{

src/backend/distributed/deparser/deparse_function_stmts.c

@@ -278,6 +278,16 @@ AppendDefElemSet(StringInfo buf, DefElem *def)
 {
 	VariableSetStmt *setStmt = castNode(VariableSetStmt, def->arg);
 
+	AppendVariableSet(buf, setStmt);
+}
+
+
+/*
+ * AppendVariableSet appends a string representing the VariableSetStmt to a buffer
+ */
+void
+AppendVariableSet(StringInfo buf, VariableSetStmt *setStmt)
+{
 	switch (setStmt->kind)
 	{
 		case VAR_SET_VALUE:

src/backend/distributed/deparser/deparse_role_stmts.c

@@ -20,6 +20,7 @@
 #include "utils/builtins.h"
 
 static void AppendAlterRoleStmt(StringInfo buf, AlterRoleStmt *stmt);
+static void AppendAlterRoleSetStmt(StringInfo buf, AlterRoleSetStmt *stmt);
 
 
 /*
@@ -39,6 +40,23 @@ DeparseAlterRoleStmt(Node *node)
 }
 
 
+/*
+ * DeparseAlterRoleSetStmt builds and returns a string representing of the
+ * AlterRoleSetStmt for application on a remote server.
+ */
+char *
+DeparseAlterRoleSetStmt(Node *node)
+{
+	AlterRoleSetStmt *stmt = castNode(AlterRoleSetStmt, node);
+	StringInfoData buf = { 0 };
+	initStringInfo(&buf);
+
+	AppendAlterRoleSetStmt(&buf, stmt);
+
+	return buf.data;
+}
+
+
 /*
  * AppendAlterRoleStmt generates the string representation of the
  * AlterRoleStmt and appends it to the buffer.
@@ -138,3 +156,45 @@ AppendAlterRoleStmt(StringInfo buf, AlterRoleStmt *stmt)
 		}
 	}
 }
+
+
+/*
+ * AppendAlterRoleStmt generates the string representation of the
+ * AlterRoleStmt and appends it to the buffer.
+ */
+static void
+AppendAlterRoleSetStmt(StringInfo buf, AlterRoleSetStmt *stmt)
+{
+	RoleSpec *role = stmt->role;
+	const char *roleSpecStr = NULL;
+
+	if (role == NULL)
+	{
+		/*
+		 * If all roles are be affected, role field is left blank in an
+		 * AlterRoleSetStmt.
+		 */
+		roleSpecStr = "ALL";
+	}
+	else
+	{
+		/*
+		 * If the role_specification used is CURRENT_USER or SESSION_USER,
+		 * it will be converted to thats roles role name.
+		 *
+		 * We also set withQuoteIdentifier parameter to true. Since the
+		 * roleSpecStr will be used in a query, the quotes are needed.
+		 */
+		roleSpecStr = RoleSpecString(role, true);
+	}
+
+	appendStringInfo(buf, "ALTER ROLE %s", roleSpecStr);
+
+	if (stmt->database != NULL)
+	{
+		appendStringInfo(buf, " IN DATABASE %s", quote_identifier(stmt->database));
+	}
+
+	VariableSetStmt *setStmt = castNode(VariableSetStmt, stmt->setstmt);
+	AppendVariableSet(buf, setStmt);
+}

src/backend/distributed/deparser/qualify_role_stmt.c

@@ -0,0 +1,60 @@
+/*-------------------------------------------------------------------------
+ *
+ * qualify_role_stmt.c
+ *	  Functions specialized in fully qualifying all role statements. These
+ *	  functions are dispatched from qualify.c
+ *
+ *	  Fully qualifying role statements consists of adding the current database
+ *    name, session user, current use, and current configuration values.
+ *
+ *	  Goal would be that the deparser functions for these statements can
+ *	  serialize the statement without any external lookups.
+ *
+ * Copyright (c), Citus Data, Inc.
+ *
+ *-------------------------------------------------------------------------
+ */
+
+#include "postgres.h"
+
+#include "distributed/deparser.h"
+#include "nodes/nodes.h"
+#include "utils/guc.h"
+
+
+static void QualifyVarSetCurrent(VariableSetStmt *setStmt);
+
+/*
+ * QualifyAlterRoleSetStmt transforms a
+ * ALTER ROLE .. SET ..
+ * statement in place and makes the settings fully qualified.
+ */
+void
+QualifyAlterRoleSetStmt(Node *node)
+{
+	AlterRoleSetStmt *stmt = castNode(AlterRoleSetStmt, node);
+	VariableSetStmt *setStmt = stmt->setstmt;
+
+	if (setStmt->kind == VAR_SET_CURRENT)
+	{
+		QualifyVarSetCurrent(setStmt);
+	}
+}
+
+
+/*
+ * QualifyVarSetCurrent transforms a
+ * FROM CURRENT
+ * into a
+ * SET config_name TO 'config_value'
+ * VariableSetStmt in place and hence makes it fully qualified.
+ */
+static void
+QualifyVarSetCurrent(VariableSetStmt *setStmt)
+{
+	char *configurationName = setStmt->name;
+	char *configValue = GetConfigOptionByName(configurationName, NULL, false);
+
+	setStmt->kind = VAR_SET_VALUE;
+	setStmt->args = list_make1(MakeSetStatementArgument(configurationName, configValue));
+}

src/backend/distributed/metadata/node_metadata.c

@@ -401,7 +401,12 @@ PropagateRolesToNewNode(WorkerNode *newWorkerNode)
 		return;
 	}
 
-	List *ddlCommands = GenerateAlterRoleIfExistsCommandAllRoles();
+	List *ddlCommands = NIL;
+	List *alterRoleCommands = GenerateAlterRoleIfExistsCommandAllRoles();
+	List *alterRoleSetCommands = GenerateAlterRoleSetIfExistsCommands();
+
+	ddlCommands = list_concat(ddlCommands, alterRoleCommands);
+	ddlCommands = list_concat(ddlCommands, alterRoleSetCommands);
 
 	SendCommandListToWorkerInSingleTransaction(newWorkerNode->workerName,
 											   newWorkerNode->workerPort,

src/include/distributed/commands.h

@@ -184,7 +184,9 @@ extern List * PreprocessRenameAttributeStmt(Node *stmt, const char *queryString)
 
 /* role.c - forward declarations*/
 extern List * PostprocessAlterRoleStmt(Node *stmt, const char *queryString);
+extern List * PreprocessAlterRoleSetStmt(Node *stmt, const char *queryString);
 extern List * GenerateAlterRoleIfExistsCommandAllRoles(void);
+extern List * GenerateAlterRoleSetIfExistsCommands(void);
 
 
 /* schema.c - forward declarations */

src/include/distributed/deparser.h

@@ -87,6 +87,8 @@ extern char * DeparseAlterFunctionSchemaStmt(Node *stmt);
 extern char * DeparseAlterFunctionOwnerStmt(Node *stmt);
 extern char * DeparseAlterFunctionDependsStmt(Node *stmt);
 
+extern void AppendVariableSet(StringInfo buf, VariableSetStmt *setStmt);
+
 extern void QualifyAlterFunctionStmt(Node *stmt);
 extern void QualifyRenameFunctionStmt(Node *stmt);
 extern void QualifyAlterFunctionSchemaStmt(Node *stmt);
@@ -95,6 +97,10 @@ extern void QualifyAlterFunctionDependsStmt(Node *stmt);
 
 /* forward declarations for deparse_role_stmts.c */
 extern char * DeparseAlterRoleStmt(Node *stmt);
+extern char * DeparseAlterRoleSetStmt(Node *stmt);
+
+extern Node * MakeSetStatementArgument(char *configurationName, char *configurationValue);
+extern void QualifyAlterRoleSetStmt(Node *stmt);
 
 /* forward declarations for deparse_extension_stmts.c */
 extern DefElem * GetExtensionOption(List *extensionOptions,

src/test/regress/expected/alter_role_propagation.out

@@ -237,12 +237,152 @@ SELECT run_command_on_workers($$SELECT row(rolname, rolsuper, rolinherit,  rolcr
  (localhost,57638,t,"(alter_role_1,f,f,f,f,f,f,f,0,md5be308f25c7b1a2d50c85cf7e6f074df9,2052)")
 (2 rows)
 
--- table belongs to a role
+-- give login permissions so that we can connect and check if the previous queries were propagated
--- we don't support propagation of configuration_parameters and notice the users
+ALTER ROLE alter_role_1 LOGIN CONNECTION LIMIT 10;
-ALTER ROLE alter_role_1 SET enable_hashagg TO FALSE;
+-- alter configuration_parameter defaults for a user
-NOTICE:  Citus partially supports ALTER ROLE for distributed databases
+ALTER ROLE CURRENT_USER SET enable_hashagg TO FALSE;
-DETAIL:  Citus does not propagate ALTER ROLE ... SET/RESET commands to workers
+SELECT run_command_on_workers('SHOW enable_hashagg');
-HINT:  You can manually alter roles on workers.
+ run_command_on_workers
+---------------------------------------------------------------------
+ (localhost,57637,t,off)
+ (localhost,57638,t,off)
+(2 rows)
+
+-- reset to default values
+ALTER ROLE CURRENT_USER RESET enable_hashagg;
+SELECT run_command_on_workers('SHOW enable_hashagg');
+ run_command_on_workers
+---------------------------------------------------------------------
+ (localhost,57637,t,on)
+ (localhost,57638,t,on)
+(2 rows)
+
+-- provide role and database names
+ALTER ROLE alter_role_1 IN DATABASE regression SET enable_hashjoin TO 0;
+SET ROLE alter_role_1;
+SELECT run_command_on_workers('SHOW enable_hashjoin');
+ run_command_on_workers
+---------------------------------------------------------------------
+ (localhost,57637,t,off)
+ (localhost,57638,t,off)
+(2 rows)
+
+-- make sure that only alter_role_1 was affected
+RESET ROLE;
+SELECT run_command_on_workers('SHOW enable_hashjoin');
+ run_command_on_workers
+---------------------------------------------------------------------
+ (localhost,57637,t,on)
+ (localhost,57638,t,on)
+(2 rows)
+
+-- RESET ALL with IN DATABASE clause
+ALTER ROLE alter_role_1 IN DATABASE regression RESET ALL;
+ALTER ROLE alter_role_1 RESET ALL;
+ALTER ROLE ALL RESET ALL;
+-- FROM CURRENT clauses
+SET statement_timeout TO '1min';
+ALTER ROLE alter_role_1 SET statement_timeout FROM CURRENT;
+SET ROLE alter_role_1;
+SELECT run_command_on_workers('SHOW statement_timeout');
+  run_command_on_workers
+---------------------------------------------------------------------
+ (localhost,57637,t,1min)
+ (localhost,57638,t,1min)
+(2 rows)
+
+RESET statement_timeout;
+RESET ROLE;
+-- the session defaults should be updated on master_add_node
+SELECT master_remove_node('localhost', :worker_1_port);
+ master_remove_node
+---------------------------------------------------------------------
+
+(1 row)
+
+ALTER ROLE SESSION_USER SET enable_mergejoin TO false;
+ALTER ROLE CURRENT_USER SET statement_timeout TO '2min';
+ALTER ROLE CURRENT_USER SET log_min_duration_statement TO '123s';
+ALTER ROLE CURRENT_USER SET "app.dev""" TO 'a\nb';
+ALTER ROLE CURRENT_USER SET myvar.foobar TO "007";
+SELECT 1 FROM master_add_node('localhost', :worker_1_port);
+ ?column?
+---------------------------------------------------------------------
+        1
+(1 row)
+
+SELECT run_command_on_workers('SHOW enable_mergejoin');
+ run_command_on_workers
+---------------------------------------------------------------------
+ (localhost,57637,t,off)
+ (localhost,57638,t,off)
+(2 rows)
+
+SELECT run_command_on_workers('SHOW statement_timeout');
+  run_command_on_workers
+---------------------------------------------------------------------
+ (localhost,57637,t,2min)
+ (localhost,57638,t,2min)
+(2 rows)
+
+SELECT run_command_on_workers('SHOW log_min_duration_statement');
+  run_command_on_workers
+---------------------------------------------------------------------
+ (localhost,57637,t,123s)
+ (localhost,57638,t,123s)
+(2 rows)
+
+SELECT run_command_on_workers('SHOW "app.dev"""');
+   run_command_on_workers
+---------------------------------------------------------------------
+ (localhost,57637,t,"a\\nb")
+ (localhost,57638,t,"a\\nb")
+(2 rows)
+
+SELECT run_command_on_workers('SHOW myvar.foobar');
+ run_command_on_workers
+---------------------------------------------------------------------
+ (localhost,57637,t,007)
+ (localhost,57638,t,007)
+(2 rows)
+
+-- revert back to defaults
+ALTER ROLE SESSION_USER RESET ALL;
+SELECT run_command_on_workers('SHOW enable_mergejoin');
+ run_command_on_workers
+---------------------------------------------------------------------
+ (localhost,57637,t,on)
+ (localhost,57638,t,on)
+(2 rows)
+
+SELECT run_command_on_workers('SHOW statement_timeout');
+ run_command_on_workers
+---------------------------------------------------------------------
+ (localhost,57637,t,0)
+ (localhost,57638,t,0)
+(2 rows)
+
+SELECT run_command_on_workers('SHOW log_min_duration_statement');
+ run_command_on_workers
+---------------------------------------------------------------------
+ (localhost,57637,t,-1)
+ (localhost,57638,t,-1)
+(2 rows)
+
+SELECT run_command_on_workers('SHOW "app.dev"""');
+                              run_command_on_workers
+---------------------------------------------------------------------
+ (localhost,57637,f,"ERROR:  unrecognized configuration parameter ""app.dev""""")
+ (localhost,57638,f,"ERROR:  unrecognized configuration parameter ""app.dev""""")
+(2 rows)
+
+SELECT run_command_on_workers('SHOW myvar.foobar');
+                               run_command_on_workers
+---------------------------------------------------------------------
+ (localhost,57637,f,"ERROR:  unrecognized configuration parameter ""myvar.foobar""")
+ (localhost,57638,f,"ERROR:  unrecognized configuration parameter ""myvar.foobar""")
+(2 rows)
+
 -- we don't support propagation of ALTER ROLE ... RENAME TO commands.
 ALTER ROLE alter_role_1 RENAME TO alter_role_1_new;
 NOTICE:  MD5 password cleared because of role rename

src/test/regress/sql/alter_role_propagation.sql

@@ -78,11 +78,65 @@ SELECT row(rolname, rolsuper, rolinherit,  rolcreaterole, rolcreatedb, rolcanlog
 SELECT run_command_on_workers($$SELECT row(rolname, rolsuper, rolinherit,  rolcreaterole, rolcreatedb, rolcanlogin, rolreplication, rolbypassrls, rolconnlimit, rolpassword, EXTRACT (year FROM rolvaliduntil)) FROM pg_authid WHERE rolname = 'alter_role_1'$$);
 
 
--- table belongs to a role
+-- give login permissions so that we can connect and check if the previous queries were propagated
+ALTER ROLE alter_role_1 LOGIN CONNECTION LIMIT 10;
 
 
--- we don't support propagation of configuration_parameters and notice the users
+-- alter configuration_parameter defaults for a user
-ALTER ROLE alter_role_1 SET enable_hashagg TO FALSE;
+ALTER ROLE CURRENT_USER SET enable_hashagg TO FALSE;
+SELECT run_command_on_workers('SHOW enable_hashagg');
+
+-- reset to default values
+ALTER ROLE CURRENT_USER RESET enable_hashagg;
+SELECT run_command_on_workers('SHOW enable_hashagg');
+
+-- provide role and database names
+ALTER ROLE alter_role_1 IN DATABASE regression SET enable_hashjoin TO 0;
+
+SET ROLE alter_role_1;
+SELECT run_command_on_workers('SHOW enable_hashjoin');
+
+-- make sure that only alter_role_1 was affected
+RESET ROLE;
+SELECT run_command_on_workers('SHOW enable_hashjoin');
+
+-- RESET ALL with IN DATABASE clause
+ALTER ROLE alter_role_1 IN DATABASE regression RESET ALL;
+ALTER ROLE alter_role_1 RESET ALL;
+ALTER ROLE ALL RESET ALL;
+
+-- FROM CURRENT clauses
+SET statement_timeout TO '1min';
+ALTER ROLE alter_role_1 SET statement_timeout FROM CURRENT;
+
+SET ROLE alter_role_1;
+SELECT run_command_on_workers('SHOW statement_timeout');
+
+RESET statement_timeout;
+RESET ROLE;
+
+-- the session defaults should be updated on master_add_node
+SELECT master_remove_node('localhost', :worker_1_port);
+ALTER ROLE SESSION_USER SET enable_mergejoin TO false;
+ALTER ROLE CURRENT_USER SET statement_timeout TO '2min';
+ALTER ROLE CURRENT_USER SET log_min_duration_statement TO '123s';
+ALTER ROLE CURRENT_USER SET "app.dev""" TO 'a\nb';
+ALTER ROLE CURRENT_USER SET myvar.foobar TO "007";
+
+SELECT 1 FROM master_add_node('localhost', :worker_1_port);
+SELECT run_command_on_workers('SHOW enable_mergejoin');
+SELECT run_command_on_workers('SHOW statement_timeout');
+SELECT run_command_on_workers('SHOW log_min_duration_statement');
+SELECT run_command_on_workers('SHOW "app.dev"""');
+SELECT run_command_on_workers('SHOW myvar.foobar');
+
+-- revert back to defaults
+ALTER ROLE SESSION_USER RESET ALL;
+SELECT run_command_on_workers('SHOW enable_mergejoin');
+SELECT run_command_on_workers('SHOW statement_timeout');
+SELECT run_command_on_workers('SHOW log_min_duration_statement');
+SELECT run_command_on_workers('SHOW "app.dev"""');
+SELECT run_command_on_workers('SHOW myvar.foobar');
 
 -- we don't support propagation of ALTER ROLE ... RENAME TO commands.
 ALTER ROLE alter_role_1 RENAME TO alter_role_1_new;