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

Commit e2e3984

Browse files
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 fc2aafe commit e2e3984

File tree

4 files changed

+97
-35
lines changed

4 files changed

+97
-35
lines changed

src/backend/commands/tablecmds.c

+31-18
Original file line numberDiff line numberDiff line change
@@ -4473,13 +4473,18 @@ ATTypedTableRecursion(List **wqueue, Relation rel, AlterTableCmd *cmd,
44734473
/*
44744474
* find_composite_type_dependencies
44754475
*
4476-
* Check to see if a composite type is being used as a column in some
4477-
* other table (possibly nested several levels deep in composite types!).
4476+
* Check to see if the type "typeOid" is being used as a column in some table
4477+
* (possibly nested several levels deep in composite types, arrays, etc!).
44784478
* Eventually, we'd like to propagate the check or rewrite operation
4479-
* into other such tables, but for now, just error out if we find any.
4479+
* into such tables, but for now, just error out if we find any.
44804480
*
4481-
* Caller should provide either a table name or a type name (not both) to
4482-
* report in the error message, if any.
4481+
* Caller should provide either the associated relation of a rowtype,
4482+
* or a type name (not both) for use in the error message, if any.
4483+
*
4484+
* Note that "typeOid" is not necessarily a composite type; it could also be
4485+
* another container type such as an array or range, or a domain over one of
4486+
* these things. The name of this function is therefore somewhat historical,
4487+
* but it's not worth changing.
44834488
*
44844489
* We assume that functions and views depending on the type are not reasons
44854490
* to reject the ALTER. (How safe is this really?)
@@ -4492,11 +4497,13 @@ find_composite_type_dependencies(Oid typeOid, Relation origRelation,
44924497
ScanKeyData key[2];
44934498
SysScanDesc depScan;
44944499
HeapTuple depTup;
4495-
Oid arrayOid;
4500+
4501+
/* since this function recurses, it could be driven to stack overflow */
4502+
check_stack_depth();
44964503

44974504
/*
4498-
* We scan pg_depend to find those things that depend on the rowtype. (We
4499-
* assume we can ignore refobjsubid for a rowtype.)
4505+
* We scan pg_depend to find those things that depend on the given type.
4506+
* (We assume we can ignore refobjsubid for a type.)
45004507
*/
45014508
depRel = heap_open(DependRelationId, AccessShareLock);
45024509

@@ -4518,8 +4525,22 @@ find_composite_type_dependencies(Oid typeOid, Relation origRelation,
45184525
Relation rel;
45194526
Form_pg_attribute att;
45204527

4521-
/* Ignore dependees that aren't user columns of relations */
4522-
/* (we assume system columns are never of rowtypes) */
4528+
/* Check for directly dependent types */
4529+
if (pg_depend->classid == TypeRelationId)
4530+
{
4531+
/*
4532+
* This must be an array, domain, or range containing the given
4533+
* type, so recursively check for uses of this type. Note that
4534+
* any error message will mention the original type not the
4535+
* container; this is intentional.
4536+
*/
4537+
find_composite_type_dependencies(pg_depend->objid,
4538+
origRelation, origTypeName);
4539+
continue;
4540+
}
4541+
4542+
/* Else, ignore dependees that aren't user columns of relations */
4543+
/* (we assume system columns are never of interesting types) */
45234544
if (pg_depend->classid != RelationRelationId ||
45244545
pg_depend->objsubid <= 0)
45254546
continue;
@@ -4575,14 +4596,6 @@ find_composite_type_dependencies(Oid typeOid, Relation origRelation,
45754596
systable_endscan(depScan);
45764597

45774598
relation_close(depRel, AccessShareLock);
4578-
4579-
/*
4580-
* If there's an array type for the rowtype, must check for uses of it,
4581-
* too.
4582-
*/
4583-
arrayOid = get_array_type(typeOid);
4584-
if (OidIsValid(arrayOid))
4585-
find_composite_type_dependencies(arrayOid, origRelation, origTypeName);
45864599
}
45874600

45884601

src/backend/commands/typecmds.c

+32-17
Original file line numberDiff line numberDiff line change
@@ -2798,10 +2798,9 @@ validateDomainConstraint(Oid domainoid, char *ccbin)
27982798
* risk by using the weakest suitable lock (ShareLock for most callers).
27992799
*
28002800
* XXX the API for this is not sufficient to support checking domain values
2801-
* that are inside composite types or arrays. Currently we just error out
2802-
* if a composite type containing the target domain is stored anywhere.
2803-
* There are not currently arrays of domains; if there were, we could take
2804-
* the same approach, but it'd be nicer to fix it properly.
2801+
* that are inside container types, such as composite types, arrays, or
2802+
* ranges. Currently we just error out if a container type containing the
2803+
* target domain is stored anywhere.
28052804
*
28062805
* Generally used for retrieving a list of tests when adding
28072806
* new constraints to a domain.
@@ -2810,13 +2809,17 @@ static List *
28102809
get_rels_with_domain(Oid domainOid, LOCKMODE lockmode)
28112810
{
28122811
List *result = NIL;
2812+
char *domainTypeName = format_type_be(domainOid);
28132813
Relation depRel;
28142814
ScanKeyData key[2];
28152815
SysScanDesc depScan;
28162816
HeapTuple depTup;
28172817

28182818
Assert(lockmode != NoLock);
28192819

2820+
/* since this function recurses, it could be driven to stack overflow */
2821+
check_stack_depth();
2822+
28202823
/*
28212824
* We scan pg_depend to find those things that depend on the domain. (We
28222825
* assume we can ignore refobjsubid for a domain.)
@@ -2843,20 +2846,32 @@ get_rels_with_domain(Oid domainOid, LOCKMODE lockmode)
28432846
Form_pg_attribute pg_att;
28442847
int ptr;
28452848

2846-
/* Check for directly dependent types --- must be domains */
2849+
/* Check for directly dependent types */
28472850
if (pg_depend->classid == TypeRelationId)
28482851
{
2849-
Assert(get_typtype(pg_depend->objid) == TYPTYPE_DOMAIN);
2850-
2851-
/*
2852-
* Recursively add dependent columns to the output list. This is
2853-
* a bit inefficient since we may fail to combine RelToCheck
2854-
* entries when attributes of the same rel have different derived
2855-
* domain types, but it's probably not worth improving.
2856-
*/
2857-
result = list_concat(result,
2858-
get_rels_with_domain(pg_depend->objid,
2859-
lockmode));
2852+
if (get_typtype(pg_depend->objid) == TYPTYPE_DOMAIN)
2853+
{
2854+
/*
2855+
* This is a sub-domain, so recursively add dependent columns
2856+
* to the output list. This is a bit inefficient since we may
2857+
* fail to combine RelToCheck entries when attributes of the
2858+
* same rel have different derived domain types, but it's
2859+
* probably not worth improving.
2860+
*/
2861+
result = list_concat(result,
2862+
get_rels_with_domain(pg_depend->objid,
2863+
lockmode));
2864+
}
2865+
else
2866+
{
2867+
/*
2868+
* Otherwise, it is some container type using the domain, so
2869+
* fail if there are any columns of this type.
2870+
*/
2871+
find_composite_type_dependencies(pg_depend->objid,
2872+
NULL,
2873+
domainTypeName);
2874+
}
28602875
continue;
28612876
}
28622877

@@ -2893,7 +2908,7 @@ get_rels_with_domain(Oid domainOid, LOCKMODE lockmode)
28932908
if (OidIsValid(rel->rd_rel->reltype))
28942909
find_composite_type_dependencies(rel->rd_rel->reltype,
28952910
NULL,
2896-
format_type_be(domainOid));
2911+
domainTypeName);
28972912

28982913
/*
28992914
* Otherwise, we can ignore relations except those with both

src/test/regress/expected/domain.out

+17
Original file line numberDiff line numberDiff line change
@@ -661,11 +661,28 @@ insert into ddtest2 values(row(-1));
661661
alter domain posint add constraint c1 check(value >= 0);
662662
ERROR: cannot alter type "posint" because column "ddtest2.f1" uses it
663663
drop table ddtest2;
664+
-- Likewise for domains within arrays of composite
664665
create table ddtest2(f1 ddtest1[]);
665666
insert into ddtest2 values('{(-1)}');
666667
alter domain posint add constraint c1 check(value >= 0);
667668
ERROR: cannot alter type "posint" because column "ddtest2.f1" uses it
668669
drop table ddtest2;
670+
-- Likewise for domains within domains over array of composite
671+
create domain ddtest1d as ddtest1[];
672+
create table ddtest2(f1 ddtest1d);
673+
insert into ddtest2 values('{(-1)}');
674+
alter domain posint add constraint c1 check(value >= 0);
675+
ERROR: cannot alter type "posint" because column "ddtest2.f1" uses it
676+
drop table ddtest2;
677+
drop domain ddtest1d;
678+
-- Doesn't work for ranges, either
679+
create type rposint as range (subtype = posint);
680+
create table ddtest2(f1 rposint);
681+
insert into ddtest2 values('(-1,3]');
682+
alter domain posint add constraint c1 check(value >= 0);
683+
ERROR: cannot alter type "posint" because column "ddtest2.f1" uses it
684+
drop table ddtest2;
685+
drop type rposint;
669686
alter domain posint add constraint c1 check(value >= 0);
670687
create domain posint2 as posint check (value % 2 = 0);
671688
create table ddtest2(f1 posint2);

src/test/regress/sql/domain.sql

+17
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)