8000 Fix handling of container types in find_composite_type_dependencies. · lelisa2016/postgres@4e704aa · GitHub
[go: up one dir, main page]
Skip to content

Commit 4e704aa

Browse files
tglsfdctglsfdc
committed
Fix handling of container types in find_composite_type_dependencies.
find_composite_type_dependencies correctly found columns that are of the specified type, and columns that are of arrays of that type, but not columns that are domains or ranges over the given type, its array type, etc. The most general way to handle this seems to be to assume that any type that is directly dependent on the specified type can be treated as a container type, and processed recursively (allowing us to handle nested cases such as ranges over domains over arrays ...). Since a type's array type already has such a dependency, we can drop the existing special case for the array type. The very similar logic in get_rels_with_domain was likewise a few bricks shy of a load, as it supposed that a directly dependent type could *only* be a sub-domain. This is already wrong for ranges over domains, and it'll someday be wrong for arrays over domains. Add test cases illustrating the problems, and back-patch to all supported branches. Discussion: https://postgr.es/m/15268.1502309024@sss.pgh.pa.us
1 parent 56fc4f7 commit 4e704aa

File tree

4 files changed

+97
-35
lines changed

4 files changed

+97
-35
lines changed

‎src/backend/commands/tablecmds.c

Lines changed: 31 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4080,13 +4080,18 @@ ATTypedTableRecursion(List **wqueue, Relation rel, AlterTableCmd *cmd,
40804080
/*
40814081
* find_composite_type_dependencies
40824082
*
4083-
* Check to see if a composite type is being used as a column in some
4084-
* other table (possibly nested several levels deep in composite types!).
4083+
* Check to see if the type "typeOid" is being used as a column in some table
4084+
* (possibly nested several levels deep in composite types, arrays, etc!).
40854085
* Eventually, we'd like to propagate the check or rewrite operation
4086-
* into other such tables, but for now, just error out if we find any.
4086+
* into such tables, but for now, just error out if we find any.
40874087
*
4088-
* Caller should provide either a table name or a type name (not both) to
4089-
* report in the error message, if any.
4088+
* Caller should provide either the associated relation of a rowtype,
4089+
* or a type name (not both) for use in the error message, if any.
4090+
*
4091+
* Note that "typeOid" is not necessarily a composite type; it could also be
4092+
* another container type such as an array or range, or a domain over one of
4093+
* these things. The name of this function is therefore somewhat historical,
4094+
* but it's not worth changing.
40904095
*
40914096
* We assume that functions and views depending on the type are not reasons
40924097
* to reject the ALTER. (How safe is this really?)
@@ -4099,11 +4104,13 @@ find_composite_type_dependencies(Oid typeOid, Relation origRelation,
40994104
ScanKeyData key[2];
41004105
SysScanDesc depScan;
41014106
HeapTuple depTup;
4102-
Oid arrayOid;
4107+
4108+
/* since this function recurses, it could be driven to stack overflow */
4109+
check_stack_depth();
41034110

41044111
/*
4105-
* We scan pg_depend to find those things that depend on the rowtype. (We
4106-
* assume we can ignore refobjsubid for a rowtype.)
4112+
* We scan pg_depend to find those things that depend on the given type.
4113+
* (We assume we can ignore refobjsubid for a type.)
41074114
*/
41084115
depRel = heap_open(DependRelationId, AccessShareLock);
41094116

@@ -4125,8 +4132,22 @@ find_composite_type_dependencies(Oid typeOid, Relation origRelation,
41254132
Relation rel;
41264133
Form_pg_attribute att;
41274134

4128-
/* Ignore dependees that aren't user columns of relations */
4129-
/* (we assume system columns are never of rowtypes) */
4135+
/* Check for directly dependent types */
4136+
if (pg_depend->classid == TypeRelationId)
4137+
{
4138+
/*
4139+
* This must be an array, domain, or range containing the given
4140+
* type, so recursively check for uses of this type. Note that
4141+
* any error message will mention the original type not the
4142+
* container; this is intentional.
4143+
*/
4144+
find_composite_type_dependencies(pg_depend->objid,
4145+
origRelation, origTypeName);
4146+
continue;
4147+
}
4148+
4149+
/* Else, ignore dependees that aren't user columns of relations */
4150+
/* (we assume system columns are never of interesting types) */
41304151
if (pg_depend->classid != RelationRelationId ||
41314152
pg_depend->objsubid <= 0)
41324153
continue;
@@ -4181,14 +4202,6 @@ find_composite_type_dependencies(Oid typeOid, Relation origRelation,
41814202
systable_endscan(depScan);
41824203

41834204
relation_close(depRel, AccessShareLock);
4184-
4185-
/*
4186-
* If there's an array type for the rowtype, must check for uses of it,
4187-
* too.
4188-
*/
4189-
arrayOid = get_array_type(typeOid);
4190-
if (OidIsValid(arrayOid))
4191-
find_composite_type_dependencies(arrayOid, origRelation, origTypeName);
41924205
}
41934206

41944207

‎src/backend/commands/typecmds.c

Lines changed: 32 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2646,10 +2646,9 @@ validateDomainConstraint(Oid domainoid, char *ccbin)
26462646
* risk by using the weakest suitable lock (ShareLock for most callers).
26472647
*
26482648
* XXX the API for this is not sufficient to support checking domain values
2649-
* that are inside composite types or arrays. Currently we just error out
2650-
* if a composite type containing the target domain is stored anywhere.
2651-
* There are not currently arrays of domains; if there were, we could take
2652-
* the same approach, but it'd be nicer to fix it properly.
2649+
* that are inside container types, such as composite types, arrays, or
2650+
* ranges. Currently we just error out if a container type containing the
2651+
* target domain is stored anywhere.
26532652
*
26542653
* Generally used for retrieving a list of tests when adding
26552654
* new constraints to a domain.
@@ -2658,13 +2657,17 @@ static List *
26582657
get_rels_with_domain(Oid domainOid, LOCKMODE lockmode)
26592658
{
26602659
List *result = NIL;
2660+
char *domainTypeName = format_type_be(domainOid);
26612661
Relation depRel;
26622662
ScanKeyData key[2];
26632663
SysScanDesc depScan;
26642664
HeapTuple depTup;
26652665

26662666
Assert(lockmode != NoLock);
26672667

2668+
/* since this function recurses, it could be driven to stack overflow */
2669+
check_stack_depth();
2670+
26682671
/*
26692672
* We scan pg_depend to find those things that depend on the domain. (We
26702673
* assume we can ignore refobjsubid for a domain.)
@@ -2691,20 +2694,32 @@ get_rels_with_domain(Oid domainOid, LOCKMODE lockmode)
26912694
Form_pg_attribute pg_att;
26922695
int ptr;
26932696

2694-
/* Check for directly dependent types --- must be domains */
2697+
/* Check for directly dependent types */
26952698
if (pg_depend->classid == TypeRelationId)
26962699
{
2697-
Assert(get_typtype(pg_depend->objid) == TYPTYPE_DOMAIN);
2698-
2699-
/*
2700-
* Recursively add dependent columns to the output list. This is
2701-
* a bit inefficient since we may fail to combine RelToCheck
2702-
* entries when attributes of the same rel have different derived
2703-
* domain types, but it's probably not worth improving.
2704-
*/
2705-
result = list_concat(result,
2706-
get_rels_with_domain(pg_depend->objid,
2707-
lockmode));
2700+
if (get_typtype(pg_depend->objid) == TYPTYPE_DOMAIN)
2701+
{
2702+
/*
2703+
* This is a sub-domain, so recursively add dependent columns
2704+
* to the output list. This is a bit inefficient since we may
2705+
* fail to combine RelToCheck entries when attributes of the
2706+
* same rel have different derived domain types, but it's
2707+
* probably not worth improving.
2708+
*/
2709+
result = list_concat(result,
2710+
get_rels_with_domain(pg_depend->objid,
2711+
lockmode));
2712+
}
2713+
else
2714+
{
2715+
/*
2716+
* Otherwise, it is some container type using the domain, so
2717+
* fail if there are any columns of this type.
2718+
*/
2719+
find_composite_type_dependencies(pg_depend->objid,
2720+
NULL,
2721+
domainTypeName);
2722+
}
27082723
continue;
27092724
}
27102725

@@ -2741,7 +2756,7 @@ get_rels_with_domain(Oid domainOid, LOCKMODE lockmode)
27412756
if (OidIsValid(rel->rd_rel->reltype))
27422757
find_composite_type_dependencies(rel->rd_rel->reltype,
27432758
NULL,
2744-
format_type_be(domainOid));
2759+
domainTypeName);
27452760

27462761
/* Otherwise we can ignore views, composite types, etc */
27472762
if (rel->rd_rel->relkind != RELKIND_RELATION)

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -664,11 +664,28 @@ insert into ddtest2 values(row(-1));
664664
alter domain posint add constraint c1 check(value >= 0);
665665
ERROR: cannot alter type "posint" because column "ddtest2.f1" uses it
666666
drop table ddtest2;
667+
-- Likewise for domains within arrays of composite
667668
create table ddtest2(f1 ddtest1[]);
668669
insert into ddtest2 values('{(-1)}');
669670
alter domain posint add constraint c1 check(value >= 0);
670671
ERROR: cannot alter type "posint" because column "ddtest2.f1" uses it
671672
drop table ddtest2;
673+
-- Likewise for domains within domains over array of composite
674+
create domain ddtest1d as ddtest1[];
675+
create table ddtest2(f1 ddtest1d);
676+
insert into ddtest2 values('{(-1)}');
677+
alter domain posint add constraint c1 check(value >= 0);
678+
ERROR: cannot alter type "posint" because column "ddtest2.f1" uses it
679+
drop table ddtest2;
680+
drop domain ddtest1d;
681+
-- Doesn't work for ranges, either
682+
create type rposint as range (subtype = posint);
683+
create table ddtest2(f1 rposint);
684+
insert into ddtest2 values('(-1,3]');
685+
alter domain posint add constraint c1 check(value >= 0);
686+
ERROR: cannot alter type "posint" because column "ddtest2.f1" uses it
687+
drop table ddtest2;
688+
drop type rposint;
672689
alter domain posint add constraint c1 check(value >= 0);
673690
create domain posint2 as posint check (value % 2 = 0);
674691
create table ddtest2(f1 posint2);

‎src/test/regress/sql/domain.sql

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -451,11 +451,28 @@ insert into ddtest2 values(row(-1));
451451
alter domain posint add constraint c1 check(value >= 0);
452452
drop table ddtest2;
453453

454+
-- Likewise for domains within arrays of composite
454455
create table ddtest2(f1 ddtest1[]);
455456
insert into ddtest2 values('{(-1)}');
456457
alter domain posint add constraint c1 check(value >= 0);
457458
drop table ddtest2;
458459

460+
-- Likewise for domains within domains over array of composite
461+
create domain ddtest1d as ddtest1[];
462+
create table ddtest2(f1 ddtest1d);
463+
insert into ddtest2 values('{(-1)}');
464+
alter domain posint add constraint c1 check(value >= 0);
465+
drop table ddtest2;
466+
drop domain ddtest1d;
467+
468+
-- Doesn't work for ranges, either
469+
create type rposint as range (subtype = posint);
470+
create table ddtest2(f1 rposint);
471+
insert into ddtest2 values('(-1,3]');
472+
alter domain posint add constraint c1 check(value >= 0);
473+
drop table ddtest2;
474+
drop type rposint;
475+
459476
alter domain posint add constraint c1 check(value >= 0);
460477

461478
create domain posint2 as posint check (value % 2 = 0);

0 commit comments

Comments
 (0)
0