8000 Rework internals of changing a type's ownership · home201448/postgres@bc72c3b · GitHub
[go: up one dir, main page]
Skip to content

Commit bc72c3b

Browse files
alvherrealvherre
committed
Rework internals of changing a type's ownership
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 756e7b4 to branches 9.1 -- 9.4.
1 parent 62e6eba commit bc72c3b

File tree

6 files changed

+97
-73
lines changed

6 files changed

+97
-73
lines changed

‎src/backend/catalog/pg_shdepend.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1350,7 +1350,7 @@ shdepReassignOwned(List *roleids, Oid newrole)
13501350
switch (sdepForm->classid)
13511351
{
13521352
case TypeRelationId:
1353-
AlterTypeOwnerInternal(sdepForm->objid, newrole, true);
1353+
AlterTypeOwner_oid(sdepForm->objid, newrole, true);
13541354
break;
13551355

13561356
case NamespaceRelationId:

‎src/backend/commands/tablecmds.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8375,8 +8375,7 @@ ATExecChangeOwner(Oid relationOid, Oid newOwnerId, bool recursing, LOCKMODE lock
83758375
* Also change the ownership of the table's row type, if it has one
83768376
*/
83778377
if (tuple_class->relkind != RELKIND_INDEX)
8378-
AlterTypeOwnerInternal(tuple_class->reltype, newOwnerId,
8379-
tuple_class->relkind == RELKIND_COMPOSITE_TYPE);
8378+
AlterTypeOwnerInternal(tuple_class->reltype, newOwnerId);
83808379

83818380
/*
83828381
* If we are operating on a table or materialized view, also change

‎src/backend/commands/typecmds.c

Lines changed: 47 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -3307,81 +3307,68 @@ AlterTypeOwner(List *names, Oid newOwnerId, ObjectType objecttype)
33073307
get_namespace_name(typTup->typnamespace));
33083308
}
33093309

3310-
/*
3311-
* If it's a composite type, invoke ATExecChangeOwner so that we fix
3312-
* up the pg_class entry properly. That will call back to
3313-
* AlterTypeOwnerInternal to take care of the pg_type entry(s).
3314-
*/
3315-
if (typTup->typtype == TYPTYPE_COMPOSITE)
3316-
ATExecChangeOwner(typTup->typrelid, newOwnerId, true, AccessExclusiveLock);
3317-
else
3318-
{
3319-
Datum repl_val[Natts_pg_type];
3320-
bool repl_null[Natts_pg_type];
3321-
bool repl_repl[Natts_pg_type];
3322-
Acl *newAcl;
3323-
Datum aclDatum;
3324-
bool isNull;
3325-
3326-
memset(repl_null, false, sizeof(repl_null));
3327-
memset(repl_repl, false, sizeof(repl_repl));
3310+
AlterTypeOwner_oid(typeOid, newOwnerId, true);
3311+
}
33283312

3329-
repl_repl[Anum_pg_type_typowner - 1] = true;
3330-
repl_val[Anum_pg_type_typowner - 1] = ObjectIdGetDatum(newOwnerId);
3313+
/* Clean up */
3314+
heap_close(rel, RowExclusiveLock);
33313315

3332-
aclDatum = heap_getattr(tup,
3333-
Anum_pg_type_typacl,
3334-
RelationGetDescr(rel),
3335-
&isNull);
3336-
/* Null ACLs do not require changes */
3337-
if (!isNull)
3338-
{
3339-
newAcl = aclnewowner(DatumGetAclP(aclDatum),
3340-
typTup->typowner, newOwnerId);
3341-
repl_repl[Anum_pg_type_typacl - 1] = true;
3342-
repl_val[Anum_pg_type_typacl - 1] = PointerGetDatum(newAcl);
3343-
}
3316+
return typeOid;
3317+
}
33443318

3345-
tup = heap_modify_tuple(tup, RelationGetDescr(rel), repl_val, repl_null,
3346-
repl_repl);
3319+
/*
3320+
* AlterTypeOwner_oid - change type owner unconditionally
3321+
*
3322+
* This function recurses to handle a pg_class entry, if necessary. It
3323+
* invokes any necessary access object hooks. If hasDependEntry is TRUE, this
3324+
* function modifies the pg_shdepend entry appropriately (this should be
3325+
* passed as FALSE only for table rowtypes and array types).
3326+
*
3327+
* This is used by ALTER TABLE/TYPE OWNER commands, as well as by REASSIGN
3328+
* OWNED BY. It assumes the caller has done all needed check.
3329+
*/
3330+
void
3331+
AlterTypeOwner_oid(Oid typeOid, Oid newOwnerId, bool hasDependEntry)
3332+
{
3333+
Relation rel;
3334+
HeapTuple tup;
3335+
Form_pg_type typTup;
33473336

3348-
simple_heap_update(rel, &tup->t_self, tup);
3337+
rel = heap_open(TypeRelationId, RowExclusiveLock);
33493338

3350-
CatalogUpdateIndexes(rel, tup);
3339+
tup = SearchSysCache1(TYPEOID, ObjectIdGetDatum(typeOid));
3340+
if (!HeapTupleIsValid(tup))
3341+
elog(ERROR, "cache lookup failed for type %u", typeOid);
3342+
typTup = (Form_pg_type) GETSTRUCT(tup);
33513343

3352-
/* Update owner dependency reference */
3353-
changeDependencyOnOwner(TypeRelationId, typeOid, newOwnerId);
3344+
/*
3345+
* If it's a composite type, invoke ATExecChangeOwner so that we fix up the
3346+
* pg_class entry properly. That will call back to AlterTypeOwnerInternal
3347+
* to take care of the pg_type entry(s).
3348+
*/
3349+
if (typTup->typtype == TYPTYPE_COMPOSITE)
3350+
ATExecChangeOwner(typTup->typrelid, newOwnerId, true, AccessExclusiveLock);
3351+
else
3352+
AlterTypeOwnerInternal(typeOid, newOwnerId);
33543353

3355-
InvokeObjectPostAlterHook(TypeRelationId, typeOid, 0);
3354+
/* Update owner dependency reference */
3355+
if (hasDependEntry)
3356+
changeDependencyOnOwner(TypeRelationId, typeOid, newOwnerId);
33563357

3357-
/* If it has an array type, update that too */
3358-
if (OidIsValid(typTup->typarray))
3359-
AlterTypeOwnerInternal(typTup->typarray, newOwnerId, false);
3360-
}
3361-
}
3358+
InvokeObjectPostAlterHook(TypeRelationId, typeOid, 0);
33623359

3363-
/* Clean up */
3360+
ReleaseSysCache(tup);
33643361
heap_close(rel, RowExclusiveLock);
3365-
3366-
return typeOid;
33673362
}
33683363

33693364
/*
3370-
* AlterTypeOwnerInternal - change type owner unconditionally
3371-
*
3372-
* This is currently only used to propagate ALTER TABLE/TYPE OWNER to a
3373-
* table's rowtype or an array type, and to implement REASSIGN OWNED BY.
3374-
* It assumes the caller has done all needed checks. The function will
3375-
* automatically recurse to an array type if the type has one.
3365+
* AlterTypeOwnerInternal - bare-bones type owner change.
33763366
*
3377-
* hasDependEntry should be TRUE if type is expected to have a pg_shdepend
3378-
* entry (ie, it's not a table rowtype nor an array type).
3379-
* is_primary_ops should be TRUE if this function is invoked with user's
3380-
* direct operation (e.g, shdepReassignOwned). Elsewhere,
3367+
* This routine simply modifies the owner of a pg_type entry, and recurses
3368+
* to handle a possible array type.
33813369
*/
33823370
void
3383-
AlterTypeOwnerInternal(Oid typeOid, Oid newOwnerId,
3384-
bool hasDependEntry)
3371+
AlterTypeOwnerInternal(Oid typeOid, Oid newOwnerId)
33853372
{
33863373
Relation rel;
33873374
HeapTuple tup;
@@ -3426,15 +3413,9 @@ AlterTypeOwnerInternal(Oid typeOid, Oid newOwnerId,
34263413

34273414
CatalogUpdateIndexes(rel, tup);
34283415

3429-
/* Update owner dependency reference, if it has one */
3430-
if (hasDependEntry)
3431-
changeDependencyOnOwner(TypeRelationId, typeOid, newOwnerId);
3432-
34333416
/* If it has an array type, update that too */
34343417
if (OidIsValid(typTup->typarray))
3435-
AlterTypeOwnerInternal(typTup->typarray, newOwnerId, false);
3436-
3437-
InvokeObjectPostAlterHook(TypeRelationId, typeOid, 0);
3418+
AlterTypeOwnerInternal(typTup->typarray, newOwnerId);
34383419

34393420
/* Clean up */
34403421
heap_close(rel, RowExclusiveLock);

‎src/include/commands/typecmds.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,8 @@ extern List *GetDomainConstraints(Oid typeOid);
4343

4444
extern Oid RenameType(RenameStmt *stmt);
4545
extern Oid AlterTypeOwner(List *names, Oid newOwnerId, ObjectType objecttype);
46-
extern void AlterTypeOwnerInternal(Oid typeOid, Oid newOwnerId,
47-
bool hasDependEntry);
46+
extern void AlterTypeOwner_oid(Oid typeOid, Oid newOwnerId, bool hasDependEntry);
47+
extern void AlterTypeOwnerInternal(Oid typeOid, Oid newOwnerId);
4848
extern Oid AlterTypeNamespace(List *names, const char *newschema, ObjectType objecttype);
4949
extern Oid AlterTypeNamespace_oid(Oid typeOid, Oid nspOid, ObjectAddresses *objsMoved);
5050
extern Oid AlterTypeNamespaceInternal(Oid typeOid, Oid nspOid,

‎src/test/regress/expected/dependency.out

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,13 +84,31 @@ DROP OWNED BY regression_user1;
8484
\d deptest
8585
-- Test REASSIGN OWNED
8686
GRANT ALL ON deptest1 TO regression_user1;
87+
GRANT CREATE ON DATABASE regression TO regression_user1;
8788
SET SESSION AUTHORIZATION regression_user1;
89+
CREATE SCHEMA deptest;
8890
CREATE TABLE deptest (a serial primary key, b text);
91+
ALTER DEFAULT PRIVILEGES FOR ROLE regression_user1 IN SCHEMA deptest
92+
GRANT ALL ON TABLES TO regression_user2;
93+
CREATE FUNCTION deptest_func() RETURNS void LANGUAGE plpgsql
94+
AS $$ BEGIN END; $$;
95+
CREATE TYPE deptest_enum AS ENUM ('red');
96+
CREATE TYPE deptest_range AS RANGE (SUBTYPE = int4);
8997
CREATE TABLE deptest2 (f1 int);
9098
-- make a serial column the hard way
9199
CREATE SEQUENCE ss1;
92100
ALTER TABLE deptest2 ALTER f1 SET DEFAULT nextval('ss1');
93101
ALTER SEQUENCE ss1 OWNED BY deptest2.f1;
102+
-- When reassigning ownership of a composite type, its pg_class entry
103+
-- should match
104+
CREATE TYPE deptest_t AS (a int);
105+
SELECT typowner = relowner
106+
FROM pg_type JOIN pg_class c ON typrelid = c.oid WHERE typname = 'deptest_t';
107+
?column?
108+
----------
109+
t
110+
(1 row)
111+
94112
RESET SESSION AUTHORIZATION;
95113
REASSIGN OWNED BY regression_user1 TO regression_user2;
96114
\dt deptest
@@ -100,10 +118,19 @@ REASSIGN OWNED BY regression_user1 TO regression_user2;
100118
public | deptest | table | regression_user2
101119
(1 row)
102120

121+
SELECT typowner = relowner
122+
FROM pg_type JOIN pg_class c ON typrelid = c.oid WHERE typname = 'deptest_t';
123+
?column?
124+
----------
125+
t
126+
(1 row)
127+
103128
-- doesn't work: grant still exists
104129
DROP USER regression_user1;
105130
ERROR: role "regression_user1" cannot be dropped because some objects depend on it
106-
DETAIL: privileges for table deptest1
131+
DETAIL: owner of default privileges on new relations belonging to role regression_user1 in schema deptest
132+
privileges for database regression
133+
privileges for table deptest1
107134
DROP OWNED BY regression_user1;
108135
DROP USER regression_user1;
109136
\set VERBOSITY terse

‎src/test/regress/sql/dependency.sql

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,20 +74,37 @@ DROP OWNED BY regression_user1;
7474

7575
-- Test REASSIGN OWNED
7676
GRANT ALL ON deptest1 TO regression_user1;
77+
GRANT CREATE ON DATABASE regression TO regression_user1;
7778

7879
SET SESSION AUTHORIZATION regression_user1;
80+
CREATE SCHEMA deptest;
7981
CREATE TABLE deptest (a serial primary key, b text);
82+
ALTER DEFAULT PRIVILEGES FOR ROLE regression_user1 IN SCHEMA deptest
83+
GRANT ALL ON TABLES TO regression_user2;
84+
CREATE FUNCTION deptest_func() RETURNS void LANGUAGE plpgsql
85+
AS $$ BEGIN END; $$;
86+
CREATE TYPE deptest_enum AS ENUM ('red');
87+
CREATE TYPE deptest_range AS RANGE (SUBTYPE = int4);
8088

8189
CREATE TABLE deptest2 (f1 int);
8290
-- make a serial column the hard way
8391
CREATE SEQUENCE ss1;
8492
ALTER TABLE deptest2 ALTER f1 SET DEFAULT nextval('ss1');
8593
ALTER SEQUENCE ss1 OWNED BY deptest2.f1;
86-
RESET SESSION AUTHORIZATION;
8794

95+
-- When reassigning ownership of a composite type, its pg_class entry
96+
-- should match
97+
CREATE TYPE deptest_t AS (a int);
98+
SELECT typowner = relowner
99+
FROM pg_type JOIN pg_class c ON typrelid = c.oid WHERE typname = 'deptest_t';
100+
101+
RESET SESSION AUTHORIZATION;
88102
REASSIGN OWNED BY regression_user1 TO regression_user2;
89103
\dt deptest
90104

105+
SELECT typowner = relowner
106+
FROM pg_type JOIN pg_class c ON typrelid = c.oid WHERE typname = 'deptest_t';
107+
91108
-- doesn't work: grant still exists
92109
DROP USER regression_user1;
93110
DROP OWNED BY regression_user1;

0 commit comments

Comments
 (0)
0