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

Commit749c7c4

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 parenta76200d commit749c7c4

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
@@ -4849,13 +4849,18 @@ ATTypedTableRecursion(List **wqueue, Relation rel, AlterTableCmd *cmd,
48494849
/*
48504850
* find_composite_type_dependencies
48514851
*
4852-
* Check to see ifa compositetype is being used as a column in some
4853-
*other table(possibly nested several levels deep in composite types!).
4852+
* Check to see ifthetype"typeOid"is being used as a column in some table
4853+
* (possibly nested several levels deep in composite types, arrays, etc!).
48544854
* Eventually, we'd like to propagate the check or rewrite operation
4855-
* intoothersuch tables, but for now, just error out if we find any.
4855+
* into such tables, but for now, just error out if we find any.
48564856
*
4857-
* Caller should provide either a table name or a type name (not both) to
4858-
* report in the error message, if any.
4857+
* Caller should provide either the associated relation of a rowtype,
4858+
* or a type name (not both) for use in the error message, if any.
4859+
*
4860+
* Note that "typeOid" is not necessarily a composite type; it could also be
4861+
* another container type such as an array or range, or a domain over one of
4862+
* these things. The name of this function is therefore somewhat historical,
4863+
* but it's not worth changing.
48594864
*
48604865
* We assume that functions and views depending on the type are not reasons
48614866
* to reject the ALTER. (How safe is this really?)
@@ -4868,11 +4873,13 @@ find_composite_type_dependencies(Oid typeOid, Relation origRelation,
48684873
ScanKeyDatakey[2];
48694874
SysScanDescdepScan;
48704875
HeapTupledepTup;
4871-
OidarrayOid;
4876+
4877+
/* since this function recurses, it could be driven to stack overflow */
4878+
check_stack_depth();
48724879

48734880
/*
4874-
* We scan pg_depend to find those things that depend on therowtype. (We
4875-
* assume we can ignore refobjsubid for arowtype.)
4881+
* We scan pg_depend to find those things that depend on thegiven type.
4882+
*(Weassume we can ignore refobjsubid for atype.)
48764883
*/
48774884
depRel=heap_open(DependRelationId,AccessShareLock);
48784885

@@ -4894,8 +4901,22 @@ find_composite_type_dependencies(Oid typeOid, Relation origRelation,
48944901
Relationrel;
48954902
Form_pg_attributeatt;
48964903

4897-
/* Ignore dependees that aren't user columns of relations */
4898-
/* (we assume system columns are never of rowtypes) */
4904+
/* Check for directly dependent types */
4905+
if (pg_depend->classid==TypeRelationId)
4906+
{
4907+
/*
4908+
* This must be an array, domain, or range containing the given
4909+
* type, so recursively check for uses of this type. Note that
4910+
* any error message will mention the original type not the
4911+
* container; this is intentional.
4912+
*/
4913+
find_composite_type_dependencies(pg_depend->objid,
4914+
origRelation,origTypeName);
4915+
continue;
4916+
}
4917+
4918+
/* Else, ignore dependees that aren't user columns of relations */
4919+
/* (we assume system columns are never of interesting types) */
48994920
if (pg_depend->classid!=RelationRelationId||
49004921
pg_depend->objsubid <=0)
49014922
continue;
@@ -4952,14 +4973,6 @@ find_composite_type_dependencies(Oid typeOid, Relation origRelation,
49524973
systable_endscan(depScan);
49534974

49544975
relation_close(depRel,AccessShareLock);
4955-
4956-
/*
4957-
* If there's an array type for the rowtype, must check for uses of it,
4958-
* too.
4959-
*/
4960-
arrayOid=get_array_type(typeOid);
4961-
if (OidIsValid(arrayOid))
4962-
find_composite_type_dependencies(arrayOid,origRelation,origTypeName);
49634976
}
49644977

49654978

‎src/backend/commands/typecmds.c

Lines changed: 32 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2787,10 +2787,9 @@ validateDomainConstraint(Oid domainoid, char *ccbin)
27872787
* risk by using the weakest suitable lock (ShareLock for most callers).
27882788
*
27892789
* XXX the API for this is not sufficient to support checking domain values
2790-
* that are inside composite types or arrays. Currently we just error out
2791-
* if a composite type containing the target domain is stored anywhere.
2792-
* There are not currently arrays of domains; if there were, we could take
2793-
* the same approach, but it'd be nicer to fix it properly.
2790+
* that are inside container types, such as composite types, arrays, or
2791+
* ranges. Currently we just error out if a container type containing the
2792+
* target domain is stored anywhere.
27942793
*
27952794
* Generally used for retrieving a list of tests when adding
27962795
* new constraints to a domain.
@@ -2799,13 +2798,17 @@ static List *
27992798
get_rels_with_domain(OiddomainOid,LOCKMODElockmode)
28002799
{
28012800
List*result=NIL;
2801+
char*domainTypeName=format_type_be(domainOid);
28022802
RelationdepRel;
28032803
ScanKeyDatakey[2];
28042804
SysScanDescdepScan;
28052805
HeapTupledepTup;
28062806

28072807
Assert(lockmode!=NoLock);
28082808

2809+
/* since this function recurses, it could be driven to stack overflow */
2810+
check_stack_depth();
2811+
28092812
/*
28102813
* We scan pg_depend to find those things that depend on the domain. (We
28112814
* assume we can ignore refobjsubid for a domain.)
@@ -2832,20 +2835,32 @@ get_rels_with_domain(Oid domainOid, LOCKMODE lockmode)
28322835
Form_pg_attributepg_att;
28332836
intptr;
28342837

2835-
/* Check for directly dependent types--- must be domains*/
2838+
/* Check for directly dependent types */
28362839
if (pg_depend->classid==TypeRelationId)
28372840
{
2838-
Assert(get_typtype(pg_depend->objid)==TYPTYPE_DOMAIN);
2839-
2840-
/*
2841-
* Recursively add dependent columns to the output list. This is
2842-
* a bit inefficient since we may fail to combine RelToCheck
2843-
* entries when attributes of the same rel have different derived
2844-
* domain types, but it's probably not worth improving.
2845-
*/
2846-
result=list_concat(result,
2847-
get_rels_with_domain(pg_depend->objid,
2848-
lockmode));
2841+
if (get_typtype(pg_depend->objid)==TYPTYPE_DOMAIN)
2842+
{
2843+
/*
2844+
* This is a sub-domain, so recursively add dependent columns
2845+
* to the output list. This is a bit inefficient since we may
2846+
* fail to combine RelToCheck entries when attributes of the
2847+
* same rel have different derived domain types, but it's
2848+
* probably not worth improving.
2849+
*/
2850+
result=list_concat(result,
2851+
get_rels_with_domain(pg_depend->objid,
2852+
lockmode));
2853+
}
2854+
else
2855+
{
2856+
/*
2857+
* Otherwise, it is some container type using the domain, so
2858+
* fail if there are any columns of this type.
2859+
*/
2860+
find_composite_type_dependencies(pg_depend->objid,
2861+
NULL,
2862+
domainTypeName);
2863+
}
28492864
continue;
28502865
}
28512866

@@ -2882,7 +2897,7 @@ get_rels_with_domain(Oid domainOid, LOCKMODE lockmode)
28822897
if (OidIsValid(rel->rd_rel->reltype))
28832898
find_composite_type_dependencies(rel->rd_rel->reltype,
28842899
NULL,
2885-
format_type_be(domainOid));
2900+
domainTypeName);
28862901

28872902
/*
28882903
* 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