]> granicus.if.org Git - postgresql/commitdiff
Rework internals of changing a type's ownership
authorAlvaro Herrera <alvherre@alvh.no-ip.org>
Mon, 21 Dec 2015 22:49:15 +0000 (19:49 -0300)
committerAlvaro Herrera <alvherre@alvh.no-ip.org>
Mon, 21 Dec 2015 22:49:15 +0000 (19:49 -0300)
This is necessary so that REASSIGN OWNED does the right thing with
composite types, to wit, that it also alters ownership of the type's
pg_class entry -- previously, the pg_class entry remained owned by the
original user, which caused later other failures such as the new owner's
inability to use ALTER TYPE to rename an attribute of the affected
composite.  Also, if the original owner is later dropped, the pg_class
entry becomes owned by a non-existant user which is bogus.

To fix, create a new routine AlterTypeOwner_oid which knows whether to
pass the request to ATExecChangeOwner or deal with it directly, and use
that in shdepReassignOwner rather than calling AlterTypeOwnerInternal
directly.  AlterTypeOwnerInternal is now simpler in that it only
modifies the pg_type entry and recurses to handle a possible array type;
higher-level tasks are handled by either AlterTypeOwner directly or
AlterTypeOwner_oid.

I took the opportunity to add a few more objects to the test rig for
REASSIGN OWNED, so that more cases are exercised.  Additional ones could
be added for superuser-only-ownable objects (such as FDWs and event
triggers) but I didn't want to push my luck by adding a new superuser to
the tests on a backpatchable bug fix.

Per bug #13666 reported by Chris Pacejo.

This is a backpatch of commit 756e7b4c9db1 to branches 9.1 -- 9.4.

src/backend/catalog/pg_shdepend.c
src/backend/commands/tablecmds.c
src/backend/commands/typecmds.c
src/include/commands/typecmds.h
src/test/regress/expected/dependency.out
src/test/regress/sql/dependency.sql

index 4e052d3cb0d97cef73ff8eb1fd2b189f005785a6..0c61d7bf632e6966b655c79f915b189a6b660c26 100644 (file)
@@ -1349,7 +1349,7 @@ shdepReassignOwned(List *roleids, Oid newrole)
                                        break;
 
                                case TypeRelationId:
-                                       AlterTypeOwnerInternal(sdepForm->objid, newrole, true);
+                                       AlterTypeOwner_oid(sdepForm->objid, newrole, true);
                                        break;
 
                                case OperatorRelationId:
index 36b252340a606aafbdc35545c9126b97513739be..0c89d81707ba32eaa29b32503ac0051af8f4a8d8 100644 (file)
@@ -8310,8 +8310,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);
+                       AlterTypeOwnerInternal(tuple_class->reltype, newOwnerId);
 
                /*
                 * If we are operating on a table, also change the ownership of any
index c6ec67dcbd43bb50a93d805fd419e76f5447550f..b3b3c6bdf07f4ee07ccc559a4cdde96ac125dbc0 100644 (file)
@@ -3254,77 +3254,64 @@ 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);
+               AlterTypeOwner_oid(typeOid, newOwnerId, true);
+       }
 
-                       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);
-                       }
+       /* Clean up */
+       heap_close(rel, RowExclusiveLock);
+}
 
-                       tup = heap_modify_tuple(tup, RelationGetDescr(rel), repl_val, repl_null,
-                                                                       repl_repl);
+/*
+ * AlterTypeOwner_oid - change type owner unconditionally
+ *
+ * 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).
+ *
+ * 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
+AlterTypeOwner_oid(Oid typeOid, Oid newOwnerId, bool hasDependEntry)
+{
+       Relation        rel;
+       HeapTuple       tup;
+       Form_pg_type typTup;
 
-                       simple_heap_update(rel, &tup->t_self, tup);
+       rel = heap_open(TypeRelationId, RowExclusiveLock);
 
-                       CatalogUpdateIndexes(rel, tup);
+       tup = SearchSysCache1(TYPEOID, ObjectIdGetDatum(typeOid));
+       if (!HeapTupleIsValid(tup))
+               elog(ERROR, "cache lookup failed for type %u", typeOid);
+       typTup = (Form_pg_type) GETSTRUCT(tup);
 
-                       /* Update owner dependency reference */
-                       changeDependencyOnOwner(TypeRelationId, typeOid, newOwnerId);
+       /*
+        * 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
+               AlterTypeOwnerInternal(typeOid, newOwnerId);
 
-                       /* If it has an array type, update that too */
-                       if (OidIsValid(typTup->typarray))
-                               AlterTypeOwnerInternal(typTup->typarray, newOwnerId, false);
-               }
-       }
+       /* Update owner dependency reference */
+       if (hasDependEntry)
+               changeDependencyOnOwner(TypeRelationId, typeOid, newOwnerId);
 
-       /* Clean up */
+       ReleaseSysCache(tup);
        heap_close(rel, RowExclusiveLock);
 }
 
 /*
- * AlterTypeOwnerInternal - change type owner unconditionally
+ * AlterTypeOwnerInternal - bare-bones type owner change.
  *
- * 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.
- *
- * 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 routine simply modifies the owner of a pg_type entry, and recurses
+ * to handle a possible array type.
  */
 void
-AlterTypeOwnerInternal(Oid typeOid, Oid newOwnerId,
-                                          bool hasDependEntry)
+AlterTypeOwnerInternal(Oid typeOid, Oid newOwnerId)
 {
        Relation        rel;
        HeapTuple       tup;
@@ -3369,15 +3356,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);
+               AlterTypeOwnerInternal(typTup->typarray, newOwnerId);
 
        /* Clean up */
        heap_close(rel, RowExclusiveLock);
index 2351024c2212199dac3563c222aa171a3dacc8cd..65c19a0bf9169b3960d2fb4f98b980e63b14738d 100644 (file)
@@ -43,8 +43,8 @@ extern List *GetDomainConstraints(Oid typeOid);
 
 extern void RenameType(RenameStmt *stmt);
 extern void AlterTypeOwner(List *names, Oid newOwnerId, ObjectType objecttype);
-extern void AlterTypeOwnerInternal(Oid typeOid, Oid newOwnerId,
-                                          bool hasDependEntry);
+extern void AlterTypeOwner_oid(Oid typeOid, Oid newOwnerId, bool hasDependEntry);
+extern void AlterTypeOwnerInternal(Oid typeOid, Oid newOwnerId);
 extern void AlterTypeNamespace(List *names, const char *newschema, ObjectType objecttype);
 extern Oid     AlterTypeNamespace_oid(Oid typeOid, Oid nspOid, ObjectAddresses *objsMoved);
 extern Oid     AlterTypeNamespaceInternal(Oid typeOid, Oid nspOid,
index 700def75b1fbce446925897e932886609e9390d6..c687342df29190b260a64cfe9513dd3b7630e676 100644 (file)
@@ -89,15 +89,33 @@ DROP OWNED BY regression_user1;
 \d deptest
 -- Test REASSIGN OWNED
 GRANT ALL ON deptest1 TO regression_user1;
+GRANT CREATE ON DATABASE regression TO regression_user1;
 SET SESSION AUTHORIZATION regression_user1;
+CREATE SCHEMA deptest;
 CREATE TABLE deptest (a serial primary key, b text);
 NOTICE:  CREATE TABLE will create implicit sequence "deptest_a_seq" for serial column "deptest.a"
 NOTICE:  CREATE TABLE / PRIMARY KEY will create implicit index "deptest_pkey" for table "deptest"
+ALTER DEFAULT PRIVILEGES FOR ROLE regression_user1 IN SCHEMA deptest
+  GRANT ALL ON TABLES TO regression_user2;
+CREATE FUNCTION deptest_func() RETURNS void LANGUAGE plpgsql
+  AS $$ BEGIN END; $$;
+CREATE TYPE deptest_enum AS ENUM ('red');
+CREATE TYPE deptest_range AS RANGE (SUBTYPE = int4);
 CREATE TABLE deptest2 (f1 int);
 -- make a serial column the hard way
 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);
+SELECT typowner = relowner
+FROM pg_type JOIN pg_class c ON typrelid = c.oid WHERE typname = 'deptest_t';
+ ?column? 
+----------
+ t
+(1 row)
+
 RESET SESSION AUTHORIZATION;
 REASSIGN OWNED BY regression_user1 TO regression_user2;
 \dt deptest
@@ -107,10 +125,19 @@ REASSIGN OWNED BY regression_user1 TO regression_user2;
  public | deptest | table | regression_user2
 (1 row)
 
+SELECT typowner = relowner
+FROM pg_type JOIN pg_class c ON typrelid = c.oid WHERE typname = 'deptest_t';
+ ?column? 
+----------
+ t
+(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
-DETAIL:  privileges for table deptest1
+DETAIL:  owner of default privileges on new relations belonging to role regression_user1 in schema deptest
+privileges for database regression
+privileges for table deptest1
 DROP OWNED BY regression_user1;
 DROP USER regression_user1;
 \set VERBOSITY terse
index c1d81569c69136fa4b0c610b1ff30d8f0e0f30f6..599a359b4fed985ca248d6c1a94b4ebe527a279f 100644 (file)
@@ -74,20 +74,37 @@ DROP OWNED BY regression_user1;
 
 -- Test REASSIGN OWNED
 GRANT ALL ON deptest1 TO regression_user1;
+GRANT CREATE ON DATABASE regression TO regression_user1;
 
 SET SESSION AUTHORIZATION regression_user1;
+CREATE SCHEMA deptest;
 CREATE TABLE deptest (a serial primary key, b text);
+ALTER DEFAULT PRIVILEGES FOR ROLE regression_user1 IN SCHEMA deptest
+  GRANT ALL ON TABLES TO regression_user2;
+CREATE FUNCTION deptest_func() RETURNS void LANGUAGE plpgsql
+  AS $$ BEGIN END; $$;
+CREATE TYPE deptest_enum AS ENUM ('red');
+CREATE TYPE deptest_range AS RANGE (SUBTYPE = int4);
 
 CREATE TABLE deptest2 (f1 int);
 -- make a serial column the hard way
 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);
+SELECT typowner = relowner
+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
+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;