Propagate ALTER SCHEMA .. OWNER TO .. (#6987)

Propagate `ALTER SCHEMA .. OWNER TO ..` commands to workers

src/backend/distributed/commands/distribute_object_ops.c

@@ -1024,6 +1024,15 @@ static DistributeObjectOps Routine_Rename = {
 	.address = RenameFunctionStmtObjectAddress,
 	.markDistributed = false,
 };
+static DistributeObjectOps Schema_AlterOwner = {
+	.deparse = DeparseAlterSchemaOwnerStmt,
+	.qualify = NULL,
+	.preprocess = PreprocessAlterDistributedObjectStmt,
+	.operationType = DIST_OPS_ALTER,
+	.postprocess = NULL,
+	.address = AlterSchemaOwnerStmtObjectAddress,
+	.markDistributed = false,
+};
 static DistributeObjectOps Schema_Drop = {
 	.deparse = DeparseDropSchemaStmt,
 	.qualify = NULL,
@@ -1457,6 +1466,11 @@ GetDistributeObjectOps(Node *node)
 					return &Routine_AlterOwner;
 				}
 
+				case OBJECT_SCHEMA:
+				{
+					return &Schema_AlterOwner;
+				}
+
 				case OBJECT_STATISTIC_EXT:
 				{
 					return &Statistics_AlterOwner;

src/backend/distributed/commands/schema.c

@@ -250,6 +250,20 @@ CreateSchemaStmtObjectAddress(Node *node, bool missing_ok, bool isPostprocess)
 }
 
 
+/*
+ * AlterSchemaOwnerStmtObjectAddress returns the ObjectAddress of the schema that is
+ * the object of the AlterOwnerStmt. Errors if missing_ok is false.
+ */
+List *
+AlterSchemaOwnerStmtObjectAddress(Node *node, bool missing_ok, bool isPostprocess)
+{
+	AlterOwnerStmt *stmt = castNode(AlterOwnerStmt, node);
+	Assert(stmt->objectType == OBJECT_SCHEMA);
+
+	return GetObjectAddressBySchemaName(strVal(stmt->object), missing_ok);
+}
+
+
 /*
  * AlterSchemaRenameStmtObjectAddress returns the ObjectAddress of the schema that is
  * the object of the RenameStmt. Errors if missing_ok is false.

src/backend/distributed/deparser/deparse_schema_stmts.c

@@ -24,6 +24,7 @@ static void AppendDropSchemaStmt(StringInfo buf, DropStmt *stmt);
 static void AppendGrantOnSchemaStmt(StringInfo buf, GrantStmt *stmt);
 static void AppendGrantOnSchemaSchemas(StringInfo buf, GrantStmt *stmt);
 static void AppendAlterSchemaRenameStmt(StringInfo buf, RenameStmt *stmt);
+static void AppendAlterSchemaOwnerStmt(StringInfo buf, AlterOwnerStmt *stmt);
 
 char *
 DeparseCreateSchemaStmt(Node *node)
@@ -68,6 +69,31 @@ DeparseGrantOnSchemaStmt(Node *node)
 }
 
 
+char *
+DeparseAlterSchemaOwnerStmt(Node *node)
+{
+	AlterOwnerStmt *stmt = castNode(AlterOwnerStmt, node);
+
+	StringInfoData str = { 0 };
+	initStringInfo(&str);
+
+	AppendAlterSchemaOwnerStmt(&str, stmt);
+
+	return str.data;
+}
+
+
+static void
+AppendAlterSchemaOwnerStmt(StringInfo buf, AlterOwnerStmt *stmt)
+{
+	Assert(stmt->objectType == OBJECT_SCHEMA);
+
+	appendStringInfo(buf, "ALTER SCHEMA %s OWNER TO %s;",
+					 quote_identifier(strVal(stmt->object)),
+					 RoleSpecString(stmt->newowner, true));
+}
+
+
 char *
 DeparseAlterSchemaRenameStmt(Node *node)
 {

src/include/distributed/commands.h

@@ -469,6 +469,8 @@ extern List * PreprocessGrantOnSchemaStmt(Node *node, const char *queryString,
 										  ProcessUtilityContext processUtilityContext);
 extern List * CreateSchemaStmtObjectAddress(Node *node, bool missing_ok, bool
 											isPostprocess);
+extern List * AlterSchemaOwnerStmtObjectAddress(Node *node, bool missing_ok,
+												bool isPostprocess);
 extern List * AlterSchemaRenameStmtObjectAddress(Node *node, bool missing_ok, bool
 												 isPostprocess);
 

src/include/distributed/deparser.h

@@ -105,6 +105,7 @@ extern char * DeparseCreateSchemaStmt(Node *node);
 extern char * DeparseDropSchemaStmt(Node *node);
 extern char * DeparseGrantOnSchemaStmt(Node *stmt);
 extern char * DeparseAlterSchemaRenameStmt(Node *stmt);
+extern char * DeparseAlterSchemaOwnerStmt(Node *node);
 
 extern void AppendGrantPrivileges(StringInfo buf, GrantStmt *stmt);
 extern void AppendGrantGrantees(StringInfo buf, GrantStmt *stmt);

src/test/regress/expected/multi_schema_support.out

@@ -1333,6 +1333,31 @@ SELECT * FROM "Citus'Teen123"."TeeNTabLE.1!?!" ORDER BY id;
   4 |         4 |
 (4 rows)
 
+-- test alter owner propagation
+CREATE ROLE test_non_super_user;
+ALTER ROLE test_non_super_user NOSUPERUSER;
+SELECT pg_get_userbyid(nspowner) AS schema_owner
+    FROM pg_namespace
+    WHERE nspname = 'bar';
+ schema_owner
+---------------------------------------------------------------------
+ postgres
+(1 row)
+
+ALTER SCHEMA bar OWNER TO test_non_super_user;
+select result from run_command_on_workers ($$
+  SELECT pg_get_userbyid(nspowner) AS schema_owner
+  FROM pg_namespace
+  WHERE nspname = 'bar'
+$$);
+       result
+---------------------------------------------------------------------
+ test_non_super_user
+ test_non_super_user
+(2 rows)
+
+ALTER SCHEMA bar OWNER TO postgres;
+DROP ROLE test_non_super_user;
 -- test error
 INSERT INTO bar.test VALUES (3,3), (4,4), (5,5), (6,6), (7,7), (8,8), (9,9);
 BEGIN;

src/test/regress/expected/schema_based_sharding.out

@@ -725,14 +725,26 @@ SET citus.enable_schema_based_sharding TO OFF;
 DROP SCHEMA tenant_1 CASCADE;
 CREATE ROLE test_non_super_user;
 ALTER ROLE test_non_super_user NOSUPERUSER;
+ALTER SCHEMA tenant_2 OWNER TO non_existing_role;
+ERROR:  role "non_existing_role" does not exist
 ALTER SCHEMA tenant_2 OWNER TO test_non_super_user;
--- XXX: ALTER SCHEMA .. OWNER TO .. is not propagated to workers,
+SELECT pg_get_userbyid(nspowner) AS schema_owner
---      see https://github.com/citusdata/citus/issues/4812.
+    FROM pg_namespace
-SELECT result FROM run_command_on_workers($$ALTER SCHEMA tenant_2 OWNER TO test_non_super_user$$);
+    WHERE nspname = 'tenant_2';
+    schema_owner
+---------------------------------------------------------------------
+ test_non_super_user
+(1 row)
+
+select result from run_command_on_workers ($$
+  SELECT pg_get_userbyid(nspowner) AS schema_owner
+  FROM pg_namespace
+  WHERE nspname = 'tenant_2'
+$$);
        result
 ---------------------------------------------------------------------
- ALTER SCHEMA
+ test_non_super_user
- ALTER SCHEMA
+ test_non_super_user
 (2 rows)
 
 DROP OWNED BY test_non_super_user CASCADE;

src/test/regress/sql/multi_schema_support.sql

@@ -949,6 +949,25 @@ SELECT COUNT(*) FROM bar.test;
 ALTER SCHEMA "CiTuS.TeeN" RENAME TO "Citus'Teen123";
 SELECT * FROM "Citus'Teen123"."TeeNTabLE.1!?!" ORDER BY id;
 
+-- test alter owner propagation
+CREATE ROLE test_non_super_user;
+ALTER ROLE test_non_super_user NOSUPERUSER;
+
+SELECT pg_get_userbyid(nspowner) AS schema_owner
+    FROM pg_namespace
+    WHERE nspname = 'bar';
+
+ALTER SCHEMA bar OWNER TO test_non_super_user;
+
+select result from run_command_on_workers ($$
+  SELECT pg_get_userbyid(nspowner) AS schema_owner
+  FROM pg_namespace
+  WHERE nspname = 'bar'
+$$);
+
+ALTER SCHEMA bar OWNER TO postgres;
+DROP ROLE test_non_super_user;
+
 -- test error
 INSERT INTO bar.test VALUES (3,3), (4,4), (5,5), (6,6), (7,7), (8,8), (9,9);
 

src/test/regress/sql/schema_based_sharding.sql

@@ -505,10 +505,18 @@ DROP SCHEMA tenant_1 CASCADE;
 CREATE ROLE test_non_super_user;
 ALTER ROLE test_non_super_user NOSUPERUSER;
 
+ALTER SCHEMA tenant_2 OWNER TO non_existing_role;
 ALTER SCHEMA tenant_2 OWNER TO test_non_super_user;
--- XXX: ALTER SCHEMA .. OWNER TO .. is not propagated to workers,
+
---      see https://github.com/citusdata/citus/issues/4812.
+SELECT pg_get_userbyid(nspowner) AS schema_owner
-SELECT result FROM run_command_on_workers($$ALTER SCHEMA tenant_2 OWNER TO test_non_super_user$$);
+    FROM pg_namespace
+    WHERE nspname = 'tenant_2';
+
+select result from run_command_on_workers ($$
+  SELECT pg_get_userbyid(nspowner) AS schema_owner
+  FROM pg_namespace
+  WHERE nspname = 'tenant_2'
+$$);
 
 DROP OWNED BY test_non_super_user CASCADE;