Merge pull request #1412 from citusdata/fix_schema_owner_name

Send correct and quoted owner name while propagating schema creation

src/backend/distributed/metadata/metadata_sync.c

@@ -53,7 +53,7 @@ static List * SequenceDDLCommandsForTable(Oid relationId);
 static void EnsureSupportedSequenceColumnType(Oid sequenceOid);
 static Oid TypeOfColumn(Oid tableId, int16 columnId);
 static char * TruncateTriggerCreateCommand(Oid relationId);
-static char * OwnerName(Oid objectId);
+static char * SchemaOwnerName(Oid objectId);
 static bool HasMetadataWorkers(void);
 
 
@@ -885,7 +885,7 @@ CreateSchemaDDLCommand(Oid schemaId)
 {
 	char *schemaName = get_namespace_name(schemaId);
 	StringInfo schemaNameDef = NULL;
-	char *ownerName = NULL;
+	const char *ownerName = NULL;
 
 	if (strncmp(schemaName, "public", NAMEDATALEN) == 0)
 	{
@@ -893,7 +893,7 @@ CreateSchemaDDLCommand(Oid schemaId)
 	}
 
 	schemaNameDef = makeStringInfo();
-	ownerName = OwnerName(schemaId);
+	ownerName = quote_identifier(SchemaOwnerName(schemaId));
 	appendStringInfo(schemaNameDef, CREATE_SCHEMA_COMMAND, schemaName, ownerName);
 
 	return schemaNameDef->data;
@@ -968,19 +968,19 @@ TruncateTriggerCreateCommand(Oid relationId)
 
 
 /*
- * OwnerName returns the name of the owner of the specified object.
+ * SchemaOwnerName returns the name of the owner of the specified schema.
  */
 static char *
-OwnerName(Oid objectId)
+SchemaOwnerName(Oid objectId)
 {
 	HeapTuple tuple = NULL;
 	Oid ownerId = InvalidOid;
 	char *ownerName = NULL;
 
-	tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(objectId));
+	tuple = SearchSysCache1(NAMESPACEOID, ObjectIdGetDatum(objectId));
 	if (HeapTupleIsValid(tuple))
 	{
-		ownerId = ((Form_pg_class) GETSTRUCT(tuple))->relowner;
+		ownerId = ((Form_pg_namespace) GETSTRUCT(tuple))->nspowner;
 	}
 	else
 	{
@@ -989,6 +989,8 @@ OwnerName(Oid objectId)
 
 	ownerName = GetUserNameFromId(ownerId, false);
 
+	ReleaseSysCache(tuple);
+
 	return ownerName;
 }
 

src/test/regress/expected/multi_schema_support.out

@@ -1084,3 +1084,62 @@ SET search_path TO public;
 ALTER TABLE test_schema_support.nation_hash SET SCHEMA public;
 WARNING:  not propagating ALTER ... SET SCHEMA commands to worker nodes
 HINT:  Connect to worker nodes directly to manually change schemas of affected objects.
+-- we will use this function in next test
+CREATE FUNCTION run_command_on_coordinator_and_workers(p_sql text)
+RETURNS void LANGUAGE plpgsql AS $$
+BEGIN
+     EXECUTE p_sql;
+     PERFORM run_command_on_workers(p_sql);
+END;$$;
+-- test schema propagation with user other than current user
+SELECT run_command_on_coordinator_and_workers('CREATE USER "test-user"');
+NOTICE:  not propagating CREATE ROLE/USER commands to worker nodes
+HINT:  Connect to worker nodes directly to manually create all necessary users and roles.
+CONTEXT:  SQL statement "CREATE USER "test-user""
+PL/pgSQL function run_command_on_coordinator_and_workers(text) line 3 at EXECUTE
+ run_command_on_coordinator_and_workers 
+----------------------------------------
+ 
+(1 row)
+
+SELECT run_command_on_coordinator_and_workers('GRANT ALL ON DATABASE postgres to "test-user"');
+ run_command_on_coordinator_and_workers 
+----------------------------------------
+ 
+(1 row)
+
+CREATE SCHEMA schema_with_user AUTHORIZATION "test-user";
+CREATE TABLE schema_with_user.test_table(column1 int);
+SELECT create_reference_table('schema_with_user.test_table');
+ create_reference_table 
+------------------------
+ 
+(1 row)
+
+-- verify that owner of the created schema is test-user
+\c - - - :worker_1_port
+\dn schema_with_user
+       List of schemas
+       Name       |   Owner   
+------------------+-----------
+ schema_with_user | test-user
+(1 row)
+
+\c - - - :master_port
+-- we do not use run_command_on_coordinator_and_workers here because when there is CASCADE, it causes deadlock
+DROP OWNED BY "test-user" CASCADE;
+NOTICE:  drop cascades to table schema_with_user.test_table
+SELECT run_command_on_workers('DROP OWNED BY "test-user" CASCADE');
+      run_command_on_workers      
+----------------------------------
+ (localhost,57637,t,"DROP OWNED")
+ (localhost,57638,t,"DROP OWNED")
+(2 rows)
+
+SELECT run_command_on_coordinator_and_workers('DROP USER "test-user"');
+ run_command_on_coordinator_and_workers 
+----------------------------------------
+ 
+(1 row)
+
+DROP FUNCTION run_command_on_coordinator_and_workers(p_sql text);

src/test/regress/sql/multi_schema_support.sql

@@ -737,3 +737,31 @@ SET citus.task_executor_type TO "real-time";
 -- we expect that it will warn out
 SET search_path TO public;
 ALTER TABLE test_schema_support.nation_hash SET SCHEMA public;
+
+-- we will use this function in next test
+CREATE FUNCTION run_command_on_coordinator_and_workers(p_sql text)
+RETURNS void LANGUAGE plpgsql AS $$
+BEGIN
+     EXECUTE p_sql;
+     PERFORM run_command_on_workers(p_sql);
+END;$$;
+
+-- test schema propagation with user other than current user
+SELECT run_command_on_coordinator_and_workers('CREATE USER "test-user"');
+SELECT run_command_on_coordinator_and_workers('GRANT ALL ON DATABASE postgres to "test-user"');
+
+CREATE SCHEMA schema_with_user AUTHORIZATION "test-user";
+CREATE TABLE schema_with_user.test_table(column1 int);
+SELECT create_reference_table('schema_with_user.test_table');
+
+-- verify that owner of the created schema is test-user
+\c - - - :worker_1_port
+\dn schema_with_user
+
+\c - - - :master_port
+-- we do not use run_command_on_coordinator_and_workers here because when there is CASCADE, it causes deadlock
+DROP OWNED BY "test-user" CASCADE;
+SELECT run_command_on_workers('DROP OWNED BY "test-user" CASCADE');
+SELECT run_command_on_coordinator_and_workers('DROP USER "test-user"');
+
+DROP FUNCTION run_command_on_coordinator_and_workers(p_sql text);