diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index d61b583..045ed3f 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -9057,8 +9057,7 @@ ATExecChangeOwner(Oid relationOid, Oid newOwnerId, bool recursing, LOCKMODE lock * Also change the ownership of the table's row type, if it has one */ if (tuple_class->relkind != RELKIND_INDEX) - AlterTypeOwnerInternal(tuple_class->reltype, newOwnerId, - tuple_class->relkind == RELKIND_COMPOSITE_TYPE); + AlterTypeOwnerBare(tuple_class->reltype, newOwnerId); /* * If we are operating on a table or materialized view, also change diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c index de91353..6d22fdf 100644 --- a/src/backend/commands/typecmds.c +++ b/src/backend/commands/typecmds.c @@ -3272,57 +3272,7 @@ AlterTypeOwner(List *names, Oid newOwnerId, ObjectType objecttype) get_namespace_name(typTup->typnamespace)); } - /* - * If it's a composite type, invoke ATExecChangeOwner so that we fix - * up the pg_class entry properly. That will call back to - * AlterTypeOwnerInternal to take care of the pg_type entry(s). - */ - if (typTup->typtype == TYPTYPE_COMPOSITE) - ATExecChangeOwner(typTup->typrelid, newOwnerId, true, AccessExclusiveLock); - else - { - Datum repl_val[Natts_pg_type]; - bool repl_null[Natts_pg_type]; - bool repl_repl[Natts_pg_type]; - Acl *newAcl; - Datum aclDatum; - bool isNull; - - memset(repl_null, false, sizeof(repl_null)); - memset(repl_repl, false, sizeof(repl_repl)); - - repl_repl[Anum_pg_type_typowner - 1] = true; - repl_val[Anum_pg_type_typowner - 1] = ObjectIdGetDatum(newOwnerId); - - aclDatum = heap_getattr(tup, - Anum_pg_type_typacl, - RelationGetDescr(rel), - &isNull); - /* Null ACLs do not require changes */ - if (!isNull) - { - newAcl = aclnewowner(DatumGetAclP(aclDatum), - typTup->typowner, newOwnerId); - repl_repl[Anum_pg_type_typacl - 1] = true; - repl_val[Anum_pg_type_typacl - 1] = PointerGetDatum(newAcl); - } - - tup = heap_modify_tuple(tup, RelationGetDescr(rel), repl_val, repl_null, - repl_repl); - - simple_heap_update(rel, &tup->t_self, tup); - - CatalogUpdateIndexes(rel, tup); - - /* Update owner dependency reference */ - changeDependencyOnOwner(TypeRelationId, typeOid, newOwnerId); - - InvokeObjectPostAlterHook(TypeRelationId, typeOid, 0); - - /* If it has an array type, update that too */ - if (OidIsValid(typTup->typarray)) - AlterTypeOwnerInternal(typTup->typarray, newOwnerId, false); - } + AlterTypeOwnerInternal(typeOid, newOwnerId, true); } ObjectAddressSet(address, TypeRelationId, typeOid); @@ -3336,15 +3286,13 @@ AlterTypeOwner(List *names, Oid newOwnerId, ObjectType objecttype) /* * AlterTypeOwnerInternal - change type owner unconditionally * - * This is currently only used to propagate ALTER TABLE/TYPE OWNER to a - * table's rowtype or an array type, and to implement REASSIGN OWNED BY. - * It assumes the caller has done all needed checks. The function will - * automatically recurse to an array type if the type has one. + * This function recurses to handle a pg_class entry, if necessary. It + * invokes any necessary access object hooks. If hasDependEntry is TRUE, this + * function modifies the pg_shdepend entry appropriately (this should be + * passed as FALSE only for table rowtypes and array types). * - * hasDependEntry should be TRUE if type is expected to have a pg_shdepend - * entry (ie, it's not a table rowtype nor an array type). - * is_primary_ops should be TRUE if this function is invoked with user's - * direct operation (e.g, shdepReassignOwned). Elsewhere, + * This is used by ALTER TABLE/TYPE OWNER commands, as well as by REASSIGN + * OWNED BY. It assumes the caller has done all needed check. */ void AlterTypeOwnerInternal(Oid typeOid, Oid newOwnerId, @@ -3353,6 +3301,46 @@ AlterTypeOwnerInternal(Oid typeOid, Oid newOwnerId, Relation rel; HeapTuple tup; Form_pg_type typTup; + + rel = heap_open(TypeRelationId, RowExclusiveLock); + + tup = SearchSysCache1(TYPEOID, ObjectIdGetDatum(typeOid)); + if (!HeapTupleIsValid(tup)) + elog(ERROR, "cache lookup failed for type %u", typeOid); + typTup = (Form_pg_type) GETSTRUCT(tup); + + /* + * If it's a composite type, invoke ATExecChangeOwner so that we fix up the + * pg_class entry properly. That will call back to AlterTypeOwnerBare to + * take care of the pg_type entry(s). + */ + if (typTup->typtype == TYPTYPE_COMPOSITE) + ATExecChangeOwner(typTup->typrelid, newOwnerId, true, AccessExclusiveLock); + else + AlterTypeOwnerBare(typeOid, newOwnerId); + + /* Update owner dependency reference */ + if (hasDependEntry) + changeDependencyOnOwner(TypeRelationId, typeOid, newOwnerId); + + InvokeObjectPostAlterHook(TypeRelationId, typeOid, 0); + + ReleaseSysCache(tup); + heap_close(rel, RowExclusiveLock); +} + +/* + * AlterTypeOwnerBare - bare-bones type owner change. + * + * This routine simply modifies the owner of a pg_type entry, and recurses + * to handle a possible array type. + */ +void +AlterTypeOwnerBare(Oid typeOid, Oid newOwnerId) +{ + Relation rel; + HeapTuple tup; + Form_pg_type typTup; Datum repl_val[Natts_pg_type]; bool repl_null[Natts_pg_type]; bool repl_repl[Natts_pg_type]; @@ -3393,15 +3381,9 @@ AlterTypeOwnerInternal(Oid typeOid, Oid newOwnerId, CatalogUpdateIndexes(rel, tup); - /* Update owner dependency reference, if it has one */ - if (hasDependEntry) - changeDependencyOnOwner(TypeRelationId, typeOid, newOwnerId); - /* If it has an array type, update that too */ if (OidIsValid(typTup->typarray)) - AlterTypeOwnerInternal(typTup->typarray, newOwnerId, false); - - InvokeObjectPostAlterHook(TypeRelationId, typeOid, 0); + AlterTypeOwnerBare(typTup->typarray, newOwnerId); /* Clean up */ heap_close(rel, RowExclusiveLock); diff --git a/src/include/commands/typecmds.h b/src/include/commands/typecmds.h index ef4b41b..eda8898 100644 --- a/src/include/commands/typecmds.h +++ b/src/include/commands/typecmds.h @@ -41,9 +41,11 @@ extern ObjectAddress AlterDomainDropConstraint(List *names, const char *constrNa extern void checkDomainOwner(HeapTuple tup); extern ObjectAddress RenameType(RenameStmt *stmt); + extern ObjectAddress AlterTypeOwner(List *names, Oid newOwnerId, ObjectType objecttype); -extern void AlterTypeOwnerInternal(Oid typeOid, Oid newOwnerId, - bool hasDependEntry); +extern void AlterTypeOwnerInternal(Oid typeOid, Oid newOwnerId, bool hasDependEntry); +extern void AlterTypeOwnerBare(Oid typeOid, Oid newOwnerId); + extern ObjectAddress AlterTypeNamespace(List *names, const char *newschema, ObjectType objecttype, Oid *oldschema); extern Oid AlterTypeNamespace_oid(Oid typeOid, Oid nspOid, ObjectAddresses *objsMoved); diff --git a/src/test/regress/expected/dependency.out b/src/test/regress/expected/dependency.out index 09b0212..e18230b 100644 --- a/src/test/regress/expected/dependency.out +++ b/src/test/regress/expected/dependency.out @@ -91,6 +91,18 @@ CREATE TABLE deptest2 (f1 int); CREATE SEQUENCE ss1; ALTER TABLE deptest2 ALTER f1 SET DEFAULT nextval('ss1'); ALTER SEQUENCE ss1 OWNED BY deptest2.f1; +-- When reassigning ownership of a composite type, its pg_class entry +-- should match +CREATE TYPE deptest_t AS (a int); +-- throw in an enum for good measure +CREATE TYPE deptest_enum AS ENUM ('red'); +SELECT typowner = relowner, typowner::regrole +FROM pg_type JOIN pg_class c ON typrelid = c.oid WHERE typname = 'deptest_t'; + ?column? | typowner +----------+------------------ + t | regression_user1 +(1 row) + RESET SESSION AUTHORIZATION; REASSIGN OWNED BY regression_user1 TO regression_user2; \dt deptest @@ -100,6 +112,13 @@ REASSIGN OWNED BY regression_user1 TO regression_user2; public | deptest | table | regression_user2 (1 row) +SELECT typowner = relowner, typowner::regrole +FROM pg_type JOIN pg_class c ON typrelid = c.oid WHERE typname = 'deptest_t'; + ?column? | typowner +----------+------------------ + t | regression_user2 +(1 row) + -- doesn't work: grant still exists DROP USER regression_user1; ERROR: role "regression_user1" cannot be dropped because some objects depend on it diff --git a/src/test/regress/sql/dependency.sql b/src/test/regress/sql/dependency.sql index c1d8156..1552bd4 100644 --- a/src/test/regress/sql/dependency.sql +++ b/src/test/regress/sql/dependency.sql @@ -83,11 +83,22 @@ CREATE TABLE deptest2 (f1 int); CREATE SEQUENCE ss1; ALTER TABLE deptest2 ALTER f1 SET DEFAULT nextval('ss1'); ALTER SEQUENCE ss1 OWNED BY deptest2.f1; -RESET SESSION AUTHORIZATION; +-- When reassigning ownership of a composite type, its pg_class entry +-- should match +CREATE TYPE deptest_t AS (a int); +-- throw in an enum for good measure +CREATE TYPE deptest_enum AS ENUM ('red'); +SELECT typowner = relowner, typowner::regrole +FROM pg_type JOIN pg_class c ON typrelid = c.oid WHERE typname = 'deptest_t'; + +RESET SESSION AUTHORIZATION; REASSIGN OWNED BY regression_user1 TO regression_user2; \dt deptest +SELECT typowner = relowner, typowner::regrole +FROM pg_type JOIN pg_class c ON typrelid = c.oid WHERE typname = 'deptest_t'; + -- doesn't work: grant still exists DROP USER regression_user1; DROP OWNED BY regression_user1;