Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit06931a9

Browse files
committed
Fix handling of container types in find_composite_type_dependencies.
find_composite_type_dependencies correctly found columns that are ofthe specified type, and columns that are of arrays of that type, butnot columns that are domains or ranges over the given type, its arraytype, etc. The most general way to handle this seems to be to assumethat any type that is directly dependent on the specified type can betreated as a container type, and processed recursively (allowing usto handle nested cases such as ranges over domains over arrays ...).Since a type's array type already has such a dependency, we can dropthe existing special case for the array type.The very similar logic in get_rels_with_domain was likewise a fewbricks shy of a load, as it supposed that a directly dependent typecould *only* be a sub-domain. This is already wrong for ranges overdomains, and it'll someday be wrong for arrays over domains.Add test cases illustrating the problems, and back-patch to allsupported branches.Discussion:https://postgr.es/m/15268.1502309024@sss.pgh.pa.us
1 parent962b26b commit06931a9

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
@@ -4135,13 +4135,18 @@ ATTypedTableRecursion(List **wqueue, Relation rel, AlterTableCmd *cmd,
41354135
/*
41364136
* find_composite_type_dependencies
41374137
*
4138-
* Check to see ifa compositetype is being used as a column in some
4139-
*other table(possibly nested several levels deep in composite types!).
4138+
* Check to see ifthetype"typeOid"is being used as a column in some table
4139+
* (possibly nested several levels deep in composite types, arrays, etc!).
41404140
* Eventually, we'd like to propagate the check or rewrite operation
4141-
* intoothersuch tables, but for now, just error out if we find any.
4141+
* into such tables, but for now, just error out if we find any.
41424142
*
4143-
* Caller should provide either a table name or a type name (not both) to
4144-
* report in the error message, if any.
4143+
* Caller should provide either the associated relation of a rowtype,
4144+
* or a type name (not both) for use in the error message, if any.
4145+
*
4146+
* Note that "typeOid" is not necessarily a composite type; it could also be
4147+
* another container type such as an array or range, or a domain over one of
4148+
* these things. The name of this function is therefore somewhat historical,
4149+
* but it's not worth changing.
41454150
*
41464151
* We assume that functions and views depending on the type are not reasons
41474152
* to reject the ALTER. (How safe is this really?)
@@ -4154,11 +4159,13 @@ find_composite_type_dependencies(Oid typeOid, Relation origRelation,
41544159
ScanKeyDatakey[2];
41554160
SysScanDescdepScan;
41564161
HeapTupledepTup;
4157-
OidarrayOid;
4162+
4163+
/* since this function recurses, it could be driven to stack overflow */
4164+
check_stack_depth();
41584165

41594166
/*
4160-
* We scan pg_depend to find those things that depend on therowtype. (We
4161-
* assume we can ignore refobjsubid for arowtype.)
4167+
* We scan pg_depend to find those things that depend on thegiven type.
4168+
*(Weassume we can ignore refobjsubid for atype.)
41624169
*/
41634170
depRel=heap_open(DependRelationId,AccessShareLock);
41644171

@@ -4180,8 +4187,22 @@ find_composite_type_dependencies(Oid typeOid, Relation origRelation,
41804187
Relationrel;
41814188
Form_pg_attributeatt;
41824189

4183-
/* Ignore dependees that aren't user columns of relations */
4184-
/* (we assume system columns are never of rowtypes) */
4190+
/* Check for directly dependent types */
4191+
if (pg_depend->classid==TypeRelationId)
4192+
{
4193+
/*
4194+
* This must be an array, domain, or range containing the given
4195+
* type, so recursively check for uses of this type. Note that
4196+
* any error message will mention the original type not the
4197+
* container; this is intentional.
4198+
*/
4199+
find_composite_type_dependencies(pg_depend->objid,
4200+
origRelation,origTypeName);
4201+
continue;
4202+
}
4203+
4204+
/* Else, ignore dependees that aren't user columns of relations */
4205+
/* (we assume system columns are never of interesting types) */
41854206
if (pg_depend->classid!=RelationRelationId||
41864207
pg_depend->objsubid <=0)
41874208
continue;
@@ -4237,14 +4258,6 @@ find_composite_type_dependencies(Oid typeOid, Relation origRelation,
42374258
systable_endscan(depScan);
42384259

42394260
relation_close(depRel,AccessShareLock);
4240-
4241-
/*
4242-
* If there's an array type for the rowtype, must check for uses of it,
4243-
* too.
4244-
*/
4245-
arrayOid=get_array_type(typeOid);
4246-
if (OidIsValid(arrayOid))
4247-
find_composite_type_dependencies(arrayOid,origRelation,origTypeName);
42484261
}
42494262

42504263

‎src/backend/commands/typecmds.c

Lines changed: 32 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2716,10 +2716,9 @@ validateDomainConstraint(Oid domainoid, char *ccbin)
27162716
* risk by using the weakest suitable lock (ShareLock for most callers).
27172717
*
27182718
* XXX the API for this is not sufficient to support checking domain values
2719-
* that are inside composite types or arrays. Currently we just error out
2720-
* if a composite type containing the target domain is stored anywhere.
2721-
* There are not currently arrays of domains; if there were, we could take
2722-
* the same approach, but it'd be nicer to fix it properly.
2719+
* that are inside container types, such as composite types, arrays, or
2720+
* ranges. Currently we just error out if a container type containing the
2721+
* target domain is stored anywhere.
27232722
*
27242723
* Generally used for retrieving a list of tests when adding
27252724
* new constraints to a domain.
@@ -2728,13 +2727,17 @@ static List *
27282727
get_rels_with_domain(OiddomainOid,LOCKMODElockmode)
27292728
{
27302729
List*result=NIL;
2730+
char*domainTypeName=format_type_be(domainOid);
27312731
RelationdepRel;
27322732
ScanKeyDatakey[2];
27332733
SysScanDescdepScan;
27342734
HeapTupledepTup;
27352735

27362736
Assert(lockmode!=NoLock);
27372737

2738+
/* since this function recurses, it could be driven to stack overflow */
2739+
check_stack_depth();
2740+
27382741
/*
27392742
* We scan pg_depend to find those things that depend on the domain. (We
27402743
* assume we can ignore refobjsubid for a domain.)
@@ -2761,20 +2764,32 @@ get_rels_with_domain(Oid domainOid, LOCKMODE lockmode)
27612764
Form_pg_attributepg_att;
27622765
intptr;
27632766

2764-
/* Check for directly dependent types--- must be domains*/
2767+
/* Check for directly dependent types */
27652768
if (pg_depend->classid==TypeRelationId)
27662769
{
2767-
Assert(get_typtype(pg_depend->objid)==TYPTYPE_DOMAIN);
2768-
2769-
/*
2770-
* Recursively add dependent columns to the output list. This is
2771-
* a bit inefficient since we may fail to combine RelToCheck
2772-
* entries when attributes of the same rel have different derived
2773-
* domain types, but it's probably not worth improving.
2774-
*/
2775-
result=list_concat(result,
2776-
get_rels_with_domain(pg_depend->objid,
2777-
lockmode));
2770+
if (get_typtype(pg_depend->objid)==TYPTYPE_DOMAIN)
2771+
{
2772+
/*
2773+
* This is a sub-domain, so recursively add dependent columns
2774+
* to the output list. This is a bit inefficient since we may
2775+
* fail to combine RelToCheck entries when attributes of the
2776+
* same rel have different derived domain types, but it's
2777+
* probably not worth improving.
2778+
*/
2779+
result=list_concat(result,
2780+
get_rels_with_domain(pg_depend->objid,
2781+
lockmode));
2782+
}
2783+
else
2784+
{
2785+
/*
2786+
* Otherwise, it is some container type using the domain, so
2787+
* fail if there are any columns of this type.
2788+
*/
2789+
find_composite_type_dependencies(pg_depend->objid,
2790+
NULL,
2791+
domainTypeName);
2792+
}
27782793
continue;
27792794
}
27802795

@@ -2811,7 +2826,7 @@ get_rels_with_domain(Oid domainOid, LOCKMODE lockmode)
28112826
if (OidIsValid(rel->rd_rel->reltype))
28122827
find_composite_type_dependencies(rel->rd_rel->reltype,
28132828
NULL,
2814-
format_type_be(domainOid));
2829+
domainTypeName);
28152830

28162831
/*
28172832
* Otherwise, we can ignore relations except those with both

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -662,11 +662,28 @@ insert into ddtest2 values(row(-1));
662662
alter domain posint add constraint c1 check(value >= 0);
663663
ERROR: cannot alter type "posint" because column "ddtest2.f1" uses it
664664
drop table ddtest2;
665+
-- Likewise for domains within arrays of composite
665666
create table ddtest2(f1 ddtest1[]);
666667
insert into ddtest2 values('{(-1)}');
667668
alter domain posint add constraint c1 check(value >= 0);
668669
ERROR: cannot alter type "posint" because column "ddtest2.f1" uses it
669670
drop table ddtest2;
671+
-- Likewise for domains within domains over array of composite
672+
create domain ddtest1d as ddtest1[];
673+
create table ddtest2(f1 ddtest1d);
674+
insert into ddtest2 values('{(-1)}');
675+
alter domain posint add constraint c1 check(value >= 0);
676+
ERROR: cannot alter type "posint" because column "ddtest2.f1" uses it
677+
drop table ddtest2;
678+
drop domain ddtest1d;
679+
-- Doesn't work for ranges, either
680+
create type rposint as range (subtype = posint);
681+
create table ddtest2(f1 rposint);
682+
insert into ddtest2 values('(-1,3]');
683+
alter domain posint add constraint c1 check(value >= 0);
684+
ERROR: cannot alter type "posint" because column "ddtest2.f1" uses it
685+
drop table ddtest2;
686+
drop type rposint;
670687
alter domain posint add constraint c1 check(value >= 0);
671688
create domain posint2 as posint check (value % 2 = 0);
672689
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
alterdomain posint addconstraint c1check(value>=0);
452452
droptable ddtest2;
453453

454+
-- Likewise for domains within arrays of composite
454455
createtableddtest2(f1 ddtest1[]);
455456
insert into ddtest2values('{(-1)}');
456457
alterdomain posint addconstraint c1check(value>=0);
457458
droptable ddtest2;
458459

460+
-- Likewise for domains within domains over array of composite
461+
createdomainddtest1das ddtest1[];
462+
createtableddtest2(f1 ddtest1d);
463+
insert into ddtest2values('{(-1)}');
464+
alterdomain posint addconstraint c1check(value>=0);
465+
droptable ddtest2;
466+
dropdomain ddtest1d;
467+
468+
-- Doesn't work for ranges, either
469+
createtyperposintas range (subtype= posint);
470+
createtableddtest2(f1 rposint);
471+
insert into ddtest2values('(-1,3]');
472+
alterdomain posint addconstraint c1check(value>=0);
473+
droptable ddtest2;
474+
droptype rposint;
475+
459476
alterdomain posint addconstraint c1check(value>=0);
460477

461478
createdomainposint2as posintcheck (value %2=0);

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp