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

Commite2e3984

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 parentfc2aafe commite2e3984

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
@@ -4473,13 +4473,18 @@ ATTypedTableRecursion(List **wqueue, Relation rel, AlterTableCmd *cmd,
44734473
/*
44744474
* find_composite_type_dependencies
44754475
*
4476-
* Check to see ifa compositetype is being used as a column in some
4477-
*other table(possibly nested several levels deep in composite types!).
4476+
* Check to see ifthetype"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-
* intoothersuch 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
ScanKeyDatakey[2];
44934498
SysScanDescdepScan;
44944499
HeapTupledepTup;
4495-
OidarrayOid;
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 therowtype. (We
4499-
* assume we can ignore refobjsubid for arowtype.)
4505+
* We scan pg_depend to find those things that depend on thegiven type.
4506+
*(Weassume we can ignore refobjsubid for atype.)
45004507
*/
45014508
depRel=heap_open(DependRelationId,AccessShareLock);
45024509

@@ -4518,8 +4525,22 @@ find_composite_type_dependencies(Oid typeOid, Relation origRelation,
45184525
Relationrel;
45194526
Form_pg_attributeatt;
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

Lines changed: 32 additions & 17 deletions
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(OiddomainOid,LOCKMODElockmode)
28112810
{
28122811
List*result=NIL;
2812+
char*domainTypeName=format_type_be(domainOid);
28132813
RelationdepRel;
28142814
ScanKeyDatakey[2];
28152815
SysScanDescdepScan;
28162816
HeapTupledepTup;
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_attributepg_att;
28442847
intptr;
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

Lines changed: 17 additions & 0 deletions
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

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