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

Commit f9643d0

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 653530c commit f9643d0

File tree

6 files changed

+95
-71
lines changed

6 files changed

+95
-71
lines changed

‎src/backend/catalog/pg_shdepend.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1349,7 +1349,7 @@ shdepReassignOwned(List *roleids, Oid newrole)
13491349
break;
13501350

13511351
case TypeRelationId:
1352-
AlterTypeOwnerInternal(sdepForm->objid, newrole, true);
1352+
AlterTypeOwner_oid(sdepForm->objid, newrole, true);
13531353
break;
13541354

13551355
case OperatorRelationId:

‎src/backend/commands/tablecmds.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8310,8 +8310,7 @@ ATExecChangeOwner(Oid relationOid, Oid newOwnerId, bool recursing, LOCKMODE lock
83108310
* Also change the ownership of the table's row type, if it has one
83118311
*/
83128312
if (tuple_class->relkind != RELKIND_INDEX)
8313-
AlterTypeOwnerInternal(tuple_class->reltype, newOwnerId,
8314-
tuple_class->relkind == RELKIND_COMPOSITE_TYPE);
8313+
AlterTypeOwnerInternal(tuple_class->reltype, newOwnerId);
83158314

83168315
/*
83178316
* If we are operating on a table, also change the ownership of any

‎src/backend/commands/typecmds.c

Lines changed: 45 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -3254,77 +3254,64 @@ AlterTypeOwner(List *names, Oid newOwnerId, ObjectType objecttype)
32543254
get_namespace_name(typTup->typnamespace));
32553255
}
32563256

3257-
/*
3258-
* If it's a composite type, invoke ATExecChangeOwner so that we fix
3259-
* up the pg_class entry properly. That will call back to
3260-
* AlterTypeOwnerInternal to take care of the pg_type entry(s).
3261-
*/
3262-
if (typTup->typtype == TYPTYPE_COMPOSITE)
3263-
ATExecChangeOwner(typTup->typrelid, newOwnerId, true, AccessExclusiveLock);
3264-
else
3265-
{
3266-
Datum repl_val[Natts_pg_type];
3267-
bool repl_null[Natts_pg_type];
3268-
bool repl_repl[Natts_pg_type];
3269-
Acl *newAcl;
3270-
Datum aclDatum;
3271-
bool isNull;
3272-
3273-
memset(repl_null, false, sizeof(repl_null));
3274-
memset(repl_repl, false, sizeof(repl_repl));
3275-
3276-
repl_repl[Anum_pg_type_typowner - 1] = true;
3277-
repl_val[Anum_pg_type_typowner - 1] = ObjectIdGetDatum(newOwnerId);
3257+
AlterTypeOwner_oid(typeOid, newOwnerId, true);
3258+
}
32783259

3279-
aclDatum = heap_getattr(tup,
3280-
Anum_pg_type_typacl,
3281-
RelationGetDescr(rel),
3282-
&isNull);
3283-
/* Null ACLs do not require changes */
3284-
if (!isNull)
3285-
{
3286-
newAcl = aclnewowner(DatumGetAclP(aclDatum),
3287-
typTup->typowner, newOwnerId);
3288-
repl_repl[Anum_pg_type_typacl - 1] = true;
3289-
repl_val[Anum_pg_type_typacl - 1] = PointerGetDatum(newAcl);
3290-
}
3260+
/* Clean up */
3261+
heap_close(rel, RowExclusiveLock);
3262+
}
32913263

3292-
tup = heap_modify_tuple(tup, RelationGetDescr(rel), repl_val, repl_null,
3293-
repl_repl);
3264+
/*
3265+
* AlterTypeOwner_oid - change type owner unconditionally
3266+
*
3267+
* This function recurses to handle a pg_class entry, if necessary. It
3268+
* invokes any necessary access object hooks. If hasDependEntry is TRUE, this
3269+
* function modifies the pg_shdepend entry appropriately (this should be
3270+
* passed as FALSE only for table rowtypes and array types).
3271+
*
3272+
* This is used by ALTER TABLE/TYPE OWNER commands, as well as by REASSIGN
3273+
* OWNED BY. It assumes the caller has done all needed check.
3274+
*/
3275+
void
3276+
AlterTypeOwner_oid(Oid typeOid, Oid newOwnerId, bool hasDependEntry)
3277+
{
3278+
Relation rel;
3279+
HeapTuple tup;
3280+
Form_pg_type typTup;
32943281

3295-
simple_heap_update(rel, &tup->t_self, tup);
3282+
rel = heap_open(TypeRelationId, RowExclusiveLock);
32963283

3297-
CatalogUpdateIndexes(rel, tup);
3284+
tup = SearchSysCache1(TYPEOID, ObjectIdGetDatum(typeOid));
3285+
if (!HeapTupleIsValid(tup))
3286+
elog(ERROR, "cache lookup failed for type %u", typeOid);
3287+
typTup = (Form_pg_type) GETSTRUCT(tup);
32983288

3299-
/* Update owner dependency reference */
3300-
changeDependencyOnOwner(TypeRelationId, typeOid, newOwnerId);
3289+
/*
3290+
* If it's a composite type, invoke ATExecChangeOwner so that we fix up the
3291+
* pg_class entry properly. That will call back to AlterTypeOwnerInternal
3292+
* to take care of the pg_type entry(s).
3293+
*/
3294+
if (typTup->typtype == TYPTYPE_COMPOSITE)
3295+
ATExecChangeOwner(typTup->typrelid, newOwnerId, true, AccessExclusiveLock);
3296+
else
3297+
AlterTypeOwnerInternal(typeOid, newOwnerId);
33013298

3302-
/* If it has an array type, update that too */
3303-
if (OidIsValid(typTup->typarray))
3304-
AlterTypeOwnerInternal(typTup->typarray, newOwnerId, false);
3305-
}
3306-
}
3299+
/* Update owner dependency reference */
3300+
if (hasDependEntry)
3301+
changeDependencyOnOwner(TypeRelationId, typeOid, newOwnerId);
33073302

3308-
/* Clean up */
3303+
ReleaseSysCache(tup);
33093304
heap_close(rel, RowExclusiveLock);
33103305
}
33113306

33123307
/*
3313-
* AlterTypeOwnerInternal - change type owner unconditionally
3308+
* AlterTypeOwnerInternal - bare-bones type owner change.
33143309
*
3315-
* This is currently only used to propagate ALTER TABLE/TYPE OWNER to a
3316-
* table's rowtype or an array type, and to implement REASSIGN OWNED BY.
3317-
* It assumes the caller has done all needed checks. The function will
3318-
* automatically recurse to an array type if the type has one.
3319-
*
3320-
* hasDependEntry should be TRUE if type is expected to have a pg_shdepend
3321-
* entry (ie, it's not a table rowtype nor an array type).
3322-
* is_primary_ops should be TRUE if this function is invoked with user's
3323-
* direct operation (e.g, shdepReassignOwned). Elsewhere,
3310+
* This routine simply modifies the owner of a pg_type entry, and recurses
3311+
* to handle a possible array type.
33243312
*/
33253313
void
3326-
AlterTypeOwnerInternal(Oid typeOid, Oid newOwnerId,
3327-
bool hasDependEntry)
3314+
AlterTypeOwnerInternal(Oid typeOid, Oid newOwnerId)
33283315
{
33293316
Relation rel;
33303317
HeapTuple tup;
@@ -3369,15 +3356,9 @@ AlterTypeOwnerInterna 10000 l(Oid typeOid, Oid newOwnerId,
33693356

33703357
CatalogUpdateIndexes(rel, tup);
33713358

3372-
/* Update owner dependency reference, if it has one */
3373-
if (hasDependEntry)
3374-
changeDependencyOnOwner(TypeRelationId, typeOid, newOwnerId);
3375-
33763359
/* If it has an array type, update that too */
33773360
if (OidIsValid(typTup->typarray))
3378-
AlterTypeOwnerInternal(typTup->typarray, newOwnerId, false);
3379-
3380-
InvokeObjectPostAlterHook(TypeRelationId, typeOid, 0);
3361+
AlterTypeOwnerInternal(typTup->typarray, newOwnerId);
33813362

33823363
/* Clean up */
33833364
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 void RenameType(RenameStmt *stmt);
4545
extern void 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 void 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
@@ -89,15 +89,33 @@ DROP OWNED BY regression_user1;
8989
\d deptest
9090
-- Test REASSIGN OWNED
9191
GRANT ALL ON deptest1 TO regression_user1;
92+
GRANT CREATE ON DATABASE regression TO regression_user1;
9293
SET SESSION AUTHORIZATION regression_user1;
94+
CREATE SCHEMA deptest;
9395
CREATE TABLE deptest (a serial primary key, b text);
9496
NOTICE: CREATE TABLE will create implicit sequence "deptest_a_seq" for serial column "deptest.a"
9597
NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "deptest_pkey" for table "deptest"
98+
ALTER DEFAULT PRIVILEGES FOR ROLE regression_user1 IN SCHEMA deptest
99+
GRANT ALL ON TABLES TO regression_user2;
100+
CREATE FUNCTION deptest_func() RETURNS void LANGUAGE plpgsql
101+
AS $$ BEGIN END; $$;
102+
CREATE TYPE deptest_enum AS ENUM ('red');
103+
CREATE TYPE deptest_range AS RANGE (SUBTYPE = int4);
96104
CREATE TABLE deptest2 (f1 int);
97105
-- make a serial column the hard way
98106
CREATE SEQUENCE ss1;
99107
ALTER TABLE deptest2 ALTER f1 SET DEFAULT nextval('ss1');
100108
ALTER SEQUENCE ss1 OWNED BY deptest2.f1;
109+
-- When reassigning ownership of a composite type, its pg_class entry
110+
-- should match
111+
CREATE TYPE deptest_t AS (a int);
112+
SELECT typowner = relowner
113+
FROM pg_type JOIN pg_class c ON typrelid = c.oid WHERE typname = 'deptest_t';
114+
?column?
115+
----------
116+
t
117+
(1 row)
118+
101119
RESET SESSION AUTHORIZATION;
102120
REASSIGN OWNED BY regression_user1 TO regression_user2;
103121
\dt deptest
@@ -107,10 +125,19 @@ REASSIGN OWNED BY regression_user1 TO regression_user2;
107125
public | deptest | table | regression_user2
108126
(1 row)
109127

128+
SELECT typowner = relowner
129+
FROM pg_type JOIN pg_class c ON typrelid = c.oid WHERE typname = 'deptest_t';
130+
?column?
131+
----------
132+
t
133+
(1 row)
134+
110135
-- doesn't work: grant still exists
111136
DROP USER regression_user1;
112137
ERROR: role "regression_user1" cannot be dropped because some objects depend on it
113-
DETAIL: privileges for table deptest1
138+
DETAIL: owner of default privileges on new relations belonging to role regression_user1 in schema deptest
139+
privileges for database regression
140+
privileges for table deptest1
114141
DROP OWNED BY regression_user1;
115142
DROP USER regression_user1;
116143
\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