Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 3e8235b

Browse files
committed
Fix multiranges to behave more like dependent types.
For most purposes, multiranges act like dependent objects of the associated range type: you can't create them separately or drop them separately. This is like the way that autogenerated array types behave. However, a couple of points were overlooked: array types automatically track the ownership of their base type, and array types do not have their own permissions but use those of the base type, while multiranges didn't emulate those behaviors. This is fairly broken, mainly because pg_dump doesn't think it needs to worry about multiranges as separate objects, and thus it fails to dump/restore ownership or permissions of multiranges. There's no apparent value in letting a multirange diverge from its parent's ownership or permissions, so let's make them act like arrays in these respects. However, we continue to let multiranges be renamed or moved to a different schema independently of their parent, since that doesn't break anything. Discussion: https://postgr.es/m/1580383.1705343264@sss.pgh.pa.us
1 parent bd8fc16 commit 3e8235b

File tree

7 files changed

+155
-23
lines changed

7 files changed

+155
-23
lines changed

src/backend/catalog/aclchk.c

+35
Original file line numberDiff line numberDiff line change
@@ -2447,11 +2447,17 @@ ExecGrant_Type_check(InternalGrant *istmt, HeapTuple tuple)
24472447

24482448
pg_type_tuple = (Form_pg_type) GETSTRUCT(tuple);
24492449

2450+
/* Disallow GRANT on dependent types */
24502451
if (IsTrueArrayType(pg_type_tuple))
24512452
ereport(ERROR,
24522453
(errcode(ERRCODE_INVALID_GRANT_OPERATION),
24532454
errmsg("cannot set privileges of array types"),
24542455
errhint("Set the privileges of the element type instead.")));
2456+
if (pg_type_tuple->typtype == TYPTYPE_MULTIRANGE)
2457+
ereport(ERROR,
2458+
(errcode(ERRCODE_INVALID_GRANT_OPERATION),
2459+
errmsg("cannot set privileges of multirange types"),
2460+
errhint("Set the privileges of the range type instead.")));
24552461

24562462
/* Used GRANT DOMAIN on a non-domain? */
24572463
if (istmt->objtype == OBJECT_DOMAIN &&
@@ -3806,6 +3812,35 @@ pg_type_aclmask_ext(Oid type_oid, Oid roleid, AclMode mask, AclMaskHow how,
38063812
typeForm = (Form_pg_type) GETSTRUCT(tuple);
38073813
}
38083814

3815+
/*
3816+
* Likewise, multirange types don't manage their own permissions; consult
3817+
* the associated range type. (Note we must do this after the array step
3818+
* to get the right answer for arrays of multiranges.)
3819+
*/
3820+
if (typeForm->typtype == TYPTYPE_MULTIRANGE)
3821+
{
3822+
Oid rangetype = get_multirange_range(typeForm->oid);
3823+
3824+
ReleaseSysCache(tuple);
3825+
3826+
tuple = SearchSysCache1(TYPEOID, ObjectIdGetDatum(rangetype));
3827+
if (!HeapTupleIsValid(tuple))
3828+
{
3829+
if (is_missing != NULL)
3830+
{
3831+
/* return "no privileges" instead of throwing an error */
3832+
*is_missing = true;
3833+
return 0;
3834+
}
3835+
else
3836+
ereport(ERROR,
3837+
(errcode(ERRCODE_UNDEFINED_OBJECT),
3838+
errmsg("type with OID %u does not exist",
3839+
rangetype)));
3840+
}
3841+
typeForm = (Form_pg_type) GETSTRUCT(tuple);
3842+
}
3843+
38093844
/*
38103845
* Now get the type's owner and ACL from the tuple
38113846
*/

src/backend/catalog/pg_type.c

+29-12
Original file line numberDiff line numberDiff line change
@@ -326,14 +326,15 @@ TypeCreate(Oid newTypeOid,
326326
errmsg("fixed-size types must have storage PLAIN")));
327327

328328
/*
329-
* This is a dependent type if it's an implicitly-created array type, or
330-
* if it's a relation rowtype that's not a composite type. For such types
331-
* we'll leave the ACL empty, and we'll skip creating some dependency
332-
* records because there will be a dependency already through the
333-
* depended-on type or relation. (Caution: this is closely intertwined
334-
* with some behavior in GenerateTypeDependencies.)
329+
* This is a dependent type if it's an implicitly-created array type or
330+
* multirange type, or if it's a relation rowtype that's not a composite
331+
* type. For such types we'll leave the ACL empty, and we'll skip
332+
* creating some dependency records because there will be a dependency
333+
* already through the depended-on type or relation. (Caution: this is
334+
* closely intertwined with some behavior in GenerateTypeDependencies.)
335335
*/
336336
isDependentType = isImplicitArray ||
337+
typeType == TYPTYPE_MULTIRANGE ||
337338
(OidIsValid(relationOid) && relationKind != RELKIND_COMPOSITE_TYPE);
338339

339340
/*
@@ -534,8 +535,9 @@ TypeCreate(Oid newTypeOid,
534535
* relationKind and isImplicitArray are likewise somewhat expensive to deduce
535536
* from the tuple, so we make callers pass those (they're not optional).
536537
*
537-
* isDependentType is true if this is an implicit array or relation rowtype;
538-
* that means it doesn't need its own dependencies on owner etc.
538+
* isDependentType is true if this is an implicit array, multirange, or
539+
* relation rowtype; that means it doesn't need its own dependencies on owner
540+
* etc.
539541
*
540542
* We make an extension-membership dependency if we're in an extension
541543
* script and makeExtensionDep is true (and isDependentType isn't true).
@@ -601,18 +603,23 @@ GenerateTypeDependencies(HeapTuple typeTuple,
601603
* Make dependencies on namespace, owner, ACL, extension.
602604
*
603605
* Skip these for a dependent type, since it will have such dependencies
604-
* indirectly through its depended-on type or relation.
606+
* indirectly through its depended-on type or relation. An exception is
607+
* that multiranges need their own namespace dependency, since we don't
608+
* force them to be in the same schema as their range type.
605609
*/
606610

607-
/* placeholder for all normal dependencies */
611+
/* collects normal dependencies for bulk recording */
608612
addrs_normal = new_object_addresses();
609613

610-
if (!isDependentType)
614+
if (!isDependentType || typeForm->typtype == TYPTYPE_MULTIRANGE)
611615
{
612616
ObjectAddressSet(referenced, NamespaceRelationId,
613617
typeForm->typnamespace);
614-
recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
618+
add_exact_object_address(&referenced, addrs_normal);
619+
}
615620

621+
if (!isDependentType)
622+
{
616623
recordDependencyOnOwner(TypeRelationId, typeObjectId,
617624
typeForm->typowner);
618625

@@ -727,6 +734,16 @@ GenerateTypeDependencies(HeapTuple typeTuple,
727734
recordDependencyOn(&myself, &referenced,
728735
isImplicitArray ? DEPENDENCY_INTERNAL : DEPENDENCY_NORMAL);
729736
}
737+
738+
/*
739+
* Note: you might expect that we should record an internal dependency of
740+
* a multirange on its range type here, by analogy with the cases above.
741+
* But instead, that is done by RangeCreate(), which also handles
742+
* recording of other range-type-specific dependencies. That's pretty
743+
* bogus. It's okay for now, because there are no cases where we need to
744+
* regenerate the dependencies of a range or multirange type. But someday
745+
* we might need to move that logic here to allow such regeneration.
746+
*/
730747
}
731748

732749
/*

src/backend/commands/typecmds.c

+36-6
Original file line numberDiff line numberDiff line change
@@ -3647,6 +3647,8 @@ RenameType(RenameStmt *stmt)
36473647
errhint("You can alter type %s, which will alter the array type as well.",
36483648
format_type_be(typTup->typelem))));
36493649

3650+
/* we do allow separate renaming of multirange types, though */
3651+
36503652
/*
36513653
* If type is composite we need to rename associated pg_class entry too.
36523654
* RenameRelationInternal will call RenameTypeInternal automatically.
@@ -3730,6 +3732,21 @@ AlterTypeOwner(List *names, Oid newOwnerId, ObjectType objecttype)
37303732
errhint("You can alter type %s, which will alter the array type as well.",
37313733
format_type_be(typTup->typelem))));
37323734

3735+
/* don't allow direct alteration of multirange types, either */
3736+
if (typTup->typtype == TYPTYPE_MULTIRANGE)
3737+
{
3738+
Oid rangetype = get_multirange_range(typeOid);
3739+
3740+
/* We don't expect get_multirange_range to fail, but cope if so */
3741+
ereport(ERROR,
3742+
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
3743+
errmsg("cannot alter multirange type %s",
3744+
format_type_be(typeOid)),
3745+
OidIsValid(rangetype) ?
3746+
errhint("You can alter type %s, which will alter the multirange type as well.",
3747+
format_type_be(rangetype)) : 0));
3748+
}
3749+
37333750
/*
37343751
* If the new owner is the same as the existing owner, consider the
37353752
* command to have succeeded. This is for dump restoration purposes.
@@ -3769,13 +3786,13 @@ AlterTypeOwner(List *names, Oid newOwnerId, ObjectType objecttype)
37693786
/*
37703787
* AlterTypeOwner_oid - change type owner unconditionally
37713788
*
3772-
* This function recurses to handle a pg_class entry, if necessary. It
3773-
* invokes any necessary access object hooks. If hasDependEntry is true, this
3774-
* function modifies the pg_shdepend entry appropriately (this should be
3775-
* passed as false only for table rowtypes and array types).
3789+
* This function recurses to handle dependent types (arrays and multiranges).
3790+
* It invokes any necessary access object hooks. If hasDependEntry is true,
3791+
* this function modifies the pg_shdepend entry appropriately (this should be
3792+
* passed as false only for table rowtypes and dependent types).
37763793
*
37773794
* This is used by ALTER TABLE/TYPE OWNER commands, as well as by REASSIGN
3778-
* OWNED BY. It assumes the caller has done all needed check.
3795+
* OWNED BY. It assumes the caller has done all needed checks.
37793796
*/
37803797
void
37813798
AlterTypeOwner_oid(Oid typeOid, Oid newOwnerId, bool hasDependEntry)
@@ -3815,7 +3832,7 @@ AlterTypeOwner_oid(Oid typeOid, Oid newOwnerId, bool hasDependEntry)
38153832
* AlterTypeOwnerInternal - bare-bones type owner change.
38163833
*
38173834
* This routine simply modifies the owner of a pg_type entry, and recurses
3818-
* to handle a possible array type.
3835+
* to handle any dependent types.
38193836
*/
38203837
void
38213838
AlterTypeOwnerInternal(Oid typeOid, Oid newOwnerId)
@@ -3865,6 +3882,19 @@ AlterTypeOwnerInternal(Oid typeOid, Oid newOwnerId)
38653882
if (OidIsValid(typTup->typarray))
38663883
AlterTypeOwnerInternal(typTup->typarray, newOwnerId);
38673884

3885+
/* If it is a range type, update the associated multirange too */
3886+
if (typTup->typtype == TYPTYPE_RANGE)
3887+
{
3888+
Oid multirange_typeid = get_range_multirange(typeOid);
3889+
3890+
if (!OidIsValid(multirange_typeid))
3891+
ereport(ERROR,
3892+
(errcode(ERRCODE_UNDEFINED_OBJECT),
3893+
errmsg("could not find multirange type for data type %s",
3894+
format_type_be(typeOid))));
3895+
AlterTypeOwnerInternal(multirange_typeid, newOwnerId);
3896+
}
3897+
38683898
/* Clean up */
38693899
table_close(rel, RowExclusiveLock);
38703900
}

src/bin/pg_dump/pg_dump.c

+4-4
Original file line numberDiff line numberDiff line change
@@ -1868,16 +1868,16 @@ selectDumpableType(TypeInfo *tyinfo, Archive *fout)
18681868
return;
18691869
}
18701870

1871-
/* skip auto-generated array types */
1871+
/* skip auto-generated array and multirange types */
18721872
if (tyinfo->isArray || tyinfo->isMultirange)
18731873
{
18741874
tyinfo->dobj.objType = DO_DUMMY_TYPE;
18751875

18761876
/*
18771877
* Fall through to set the dump flag; we assume that the subsequent
18781878
* rules will do the same thing as they would for the array's base
1879-
* type. (We cannot reliably look up the base type here, since
1880-
* getTypes may not have processed it yet.)
1879+
* type or multirange's range type. (We cannot reliably look up the
1880+
* base type here, since getTypes may not have processed it yet.)
18811881
*/
18821882
}
18831883

@@ -5770,7 +5770,7 @@ getTypes(Archive *fout, int *numTypes)
57705770
else
57715771
tyinfo[i].isArray = false;
57725772

5773-
if (tyinfo[i].typtype == 'm')
5773+
if (tyinfo[i].typtype == TYPTYPE_MULTIRANGE)
57745774
tyinfo[i].isMultirange = true;
57755775
else
57765776
tyinfo[i].isMultirange = false;

src/test/regress/expected/dependency.out

-1
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,6 @@ owner of sequence deptest_a_seq
144144
owner of table deptest
145145
owner of function deptest_func()
146146
owner of type deptest_enum
147-
owner of type deptest_multirange
148147
owner of type deptest_range
149148
owner of table deptest2
150149
owner of sequence ss1

src/test/regress/expected/multirangetypes.out

+30
Original file line numberDiff line numberDiff line change
@@ -3115,6 +3115,36 @@ select _textrange1(textrange2('a','z')) @> 'b'::text;
31153115
drop type textrange1;
31163116
drop type textrange2;
31173117
--
3118+
-- Multiranges don't have their own ownership or permissions.
3119+
--
3120+
create type textrange1 as range(subtype=text, multirange_type_name=multitextrange1, collation="C");
3121+
create role regress_multirange_owner;
3122+
alter type multitextrange1 owner to regress_multirange_owner; -- fail
3123+
ERROR: cannot alter multirange type multitextrange1
3124+
HINT: You can alter type textrange1, which will alter the multirange type as well.
3125+
alter type textrange1 owner to regress_multirange_owner;
3126+
set role regress_multirange_owner;
3127+
revoke usage on type multitextrange1 from public; -- fail
3128+
ERROR: cannot set privileges of multirange types
3129+
HINT: Set the privileges of the range type instead.
3130+
revoke usage on type textrange1 from public;
3131+
\dT+ *textrange1*
3132+
List of data types
3133+
Schema | Name | Internal name | Size | Elements | Owner | Access privileges | Description
3134+
--------+-----------------+-----------------+------+----------+--------------------------+-----------------------------------------------------+-------------
3135+
public | multitextrange1 | multitextrange1 | var | | regress_multirange_owner | |
3136+
public | textrange1 | textrange1 | var | | regress_multirange_owner | regress_multirange_owner=U/regress_multirange_owner |
3137+
(2 rows)
3138+
3139+
create temp table test1(f1 multitextrange1[]);
3140+
revoke usage on type textrange1 from regress_multirange_owner;
3141+
create temp table test2(f1 multitextrange1[]); -- fail
3142+
ERROR: permission denied for type multitextrange1
3143+
drop table test1;
3144+
drop type textrange1;
3145+
reset role;
3146+
drop role regress_multirange_owner;
3147+
--
31183148
-- Test polymorphic type system
31193149
--
31203150
create function anyarray_anymultirange_func(a anyarray, r anymultirange)

src/test/regress/sql/multirangetypes.sql

+21
Original file line numberDiff line numberDiff line change
@@ -700,6 +700,27 @@ select _textrange1(textrange2('a','z')) @> 'b'::text;
700700
drop type textrange1;
701701
drop type textrange2;
702702

703+
--
704+
-- Multiranges don't have their own ownership or permissions.
705+
--
706+
create type textrange1 as range(subtype=text, multirange_type_name=multitextrange1, collation="C");
707+
create role regress_multirange_owner;
708+
709+
alter type multitextrange1 owner to regress_multirange_owner; -- fail
710+
alter type textrange1 owner to regress_multirange_owner;
711+
set role regress_multirange_owner;
712+
revoke usage on type multitextrange1 from public; -- fail
713+
revoke usage on type textrange1 from public;
714+
\dT+ *textrange1*
715+
create temp table test1(f1 multitextrange1[]);
716+
revoke usage on type textrange1 from regress_multirange_owner;
717+
create temp table test2(f1 multitextrange1[]); -- fail
718+
719+
drop table test1;
720+
drop type textrange1;
721+
reset role;
722+
drop role regress_multirange_owner;
723+
703724
--
704725
-- Test polymorphic type system
705726
--

0 commit comments

Comments
 (0)