diff options
| -rw-r--r-- | src/backend/commands/tablecmds.c | 49 | ||||
| -rw-r--r-- | src/backend/commands/typecmds.c | 49 | ||||
| -rw-r--r-- | src/test/regress/expected/domain.out | 17 | ||||
| -rw-r--r-- | src/test/regress/sql/domain.sql | 17 |
4 files changed, 97 insertions, 35 deletions
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 1b8d4b3d17e..2afde0abd8b 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -4849,13 +4849,18 @@ ATTypedTableRecursion(List **wqueue, Relation rel, AlterTableCmd *cmd, /* * find_composite_type_dependencies * - * Check to see if a composite type is being used as a column in some - * other table (possibly nested several levels deep in composite types!). + * Check to see if the type "typeOid" is being used as a column in some table + * (possibly nested several levels deep in composite types, arrays, etc!). * Eventually, we'd like to propagate the check or rewrite operation - * into other such tables, but for now, just error out if we find any. + * into such tables, but for now, just error out if we find any. * - * Caller should provide either a table name or a type name (not both) to - * report in the error message, if any. + * Caller should provide either the associated relation of a rowtype, + * or a type name (not both) for use in the error message, if any. + * + * Note that "typeOid" is not necessarily a composite type; it could also be + * another container type such as an array or range, or a domain over one of + * these things. The name of this function is therefore somewhat historical, + * but it's not worth changing. * * We assume that functions and views depending on the type are not reasons * to reject the ALTER. (How safe is this really?) @@ -4868,11 +4873,13 @@ find_composite_type_dependencies(Oid typeOid, Relation origRelation, ScanKeyData key[2]; SysScanDesc depScan; HeapTuple depTup; - Oid arrayOid; + + /* since this function recurses, it could be driven to stack overflow */ + check_stack_depth(); /* - * We scan pg_depend to find those things that depend on the rowtype. (We - * assume we can ignore refobjsubid for a rowtype.) + * We scan pg_depend to find those things that depend on the given type. + * (We assume we can ignore refobjsubid for a type.) */ depRel = heap_open(DependRelationId, AccessShareLock); @@ -4894,8 +4901,22 @@ find_composite_type_dependencies(Oid typeOid, Relation origRelation, Relation rel; Form_pg_attribute att; - /* Ignore dependees that aren't user columns of relations */ - /* (we assume system columns are never of rowtypes) */ + /* Check for directly dependent types */ + if (pg_depend->classid == TypeRelationId) + { + /* + * This must be an array, domain, or range containing the given + * type, so recursively check for uses of this type. Note that + * any error message will mention the original type not the + * container; this is intentional. + */ + find_composite_type_dependencies(pg_depend->objid, + origRelation, origTypeName); + continue; + } + + /* Else, ignore dependees that aren't user columns of relations */ + /* (we assume system columns are never of interesting types) */ if (pg_depend->classid != RelationRelationId || pg_depend->objsubid <= 0) continue; @@ -4952,14 +4973,6 @@ find_composite_type_dependencies(Oid typeOid, Relation origRelation, systable_endscan(depScan); relation_close(depRel, AccessShareLock); - - /* - * If there's an array type for the rowtype, must check for uses of it, - * too. - */ - arrayOid = get_array_type(typeOid); - if (OidIsValid(arrayOid)) - find_composite_type_dependencies(arrayOid, origRelation, origTypeName); } diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c index c2fc59d1aa0..29ac5d569d7 100644 --- a/src/backend/commands/typecmds.c +++ b/src/backend/commands/typecmds.c @@ -2787,10 +2787,9 @@ validateDomainConstraint(Oid domainoid, char *ccbin) * risk by using the weakest suitable lock (ShareLock for most callers). * * XXX the API for this is not sufficient to support checking domain values - * that are inside composite types or arrays. Currently we just error out - * if a composite type containing the target domain is stored anywhere. - * There are not currently arrays of domains; if there were, we could take - * the same approach, but it'd be nicer to fix it properly. + * that are inside container types, such as composite types, arrays, or + * ranges. Currently we just error out if a container type containing the + * target domain is stored anywhere. * * Generally used for retrieving a list of tests when adding * new constraints to a domain. @@ -2799,6 +2798,7 @@ static List * get_rels_with_domain(Oid domainOid, LOCKMODE lockmode) { List *result = NIL; + char *domainTypeName = format_type_be(domainOid); Relation depRel; ScanKeyData key[2]; SysScanDesc depScan; @@ -2806,6 +2806,9 @@ get_rels_with_domain(Oid domainOid, LOCKMODE lockmode) Assert(lockmode != NoLock); + /* since this function recurses, it could be driven to stack overflow */ + check_stack_depth(); + /* * We scan pg_depend to find those things that depend on the domain. (We * assume we can ignore refobjsubid for a domain.) @@ -2832,20 +2835,32 @@ get_rels_with_domain(Oid domainOid, LOCKMODE lockmode) Form_pg_attribute pg_att; int ptr; - /* Check for directly dependent types --- must be domains */ + /* Check for directly dependent types */ if (pg_depend->classid == TypeRelationId) { - Assert(get_typtype(pg_depend->objid) == TYPTYPE_DOMAIN); - - /* - * Recursively add dependent columns to the output list. This is - * a bit inefficient since we may fail to combine RelToCheck - * entries when attributes of the same rel have different derived - * domain types, but it's probably not worth improving. - */ - result = list_concat(result, - get_rels_with_domain(pg_depend->objid, - lockmode)); + if (get_typtype(pg_depend->objid) == TYPTYPE_DOMAIN) + { + /* + * This is a sub-domain, so recursively add dependent columns + * to the output list. This is a bit inefficient since we may + * fail to combine RelToCheck entries when attributes of the + * same rel have different derived domain types, but it's + * probably not worth improving. + */ + result = list_concat(result, + get_rels_with_domain(pg_depend->objid, + lockmode)); + } + else + { + /* + * Otherwise, it is some container type using the domain, so + * fail if there are any columns of this type. + */ + find_composite_type_dependencies(pg_depend->objid, + NULL, + domainTypeName); + } continue; } @@ -2882,7 +2897,7 @@ get_rels_with_domain(Oid domainOid, LOCKMODE lockmode) if (OidIsValid(rel->rd_rel->reltype)) find_composite_type_dependencies(rel->rd_rel->reltype, NULL, - format_type_be(domainOid)); + domainTypeName); /* * Otherwise, we can ignore relations except those with both diff --git a/src/test/regress/expected/domain.out b/src/test/regress/expected/domain.out index 5fe999e3082..3acc696863d 100644 --- a/src/test/regress/expected/domain.out +++ b/src/test/regress/expected/domain.out @@ -661,11 +661,28 @@ insert into ddtest2 values(row(-1)); alter domain posint add constraint c1 check(value >= 0); ERROR: cannot alter type "posint" because column "ddtest2.f1" uses it drop table ddtest2; +-- Likewise for domains within arrays of composite create table ddtest2(f1 ddtest1[]); insert into ddtest2 values('{(-1)}'); alter domain posint add constraint c1 check(value >= 0); ERROR: cannot alter type "posint" because column "ddtest2.f1" uses it drop table ddtest2; +-- Likewise for domains within domains over array of composite +create domain ddtest1d as ddtest1[]; +create table ddtest2(f1 ddtest1d); +insert into ddtest2 values('{(-1)}'); +alter domain posint add constraint c1 check(value >= 0); +ERROR: cannot alter type "posint" because column "ddtest2.f1" uses it +drop table ddtest2; +drop domain ddtest1d; +-- Doesn't work for ranges, either +create type rposint as range (subtype = posint); +create table ddtest2(f1 rposint); +insert into ddtest2 values('(-1,3]'); +alter domain posint add constraint c1 check(value >= 0); +ERROR: cannot alter type "posint" because column "ddtest2.f1" uses it +drop table ddtest2; +drop type rposint; alter domain posint add constraint c1 check(value >= 0); create domain posint2 as posint check (value % 2 = 0); create table ddtest2(f1 posint2); diff --git a/src/test/regress/sql/domain.sql b/src/test/regress/sql/domain.sql index 5ec128dd254..0fd383e2721 100644 --- a/src/test/regress/sql/domain.sql +++ b/src/test/regress/sql/domain.sql @@ -451,11 +451,28 @@ insert into ddtest2 values(row(-1)); alter domain posint add constraint c1 check(value >= 0); drop table ddtest2; +-- Likewise for domains within arrays of composite create table ddtest2(f1 ddtest1[]); insert into ddtest2 values('{(-1)}'); alter domain posint add constraint c1 check(value >= 0); drop table ddtest2; +-- Likewise for domains within domains over array of composite +create domain ddtest1d as ddtest1[]; +create table ddtest2(f1 ddtest1d); +insert into ddtest2 values('{(-1)}'); +alter domain posint add constraint c1 check(value >= 0); +drop table ddtest2; +drop domain ddtest1d; + +-- Doesn't work for ranges, either +create type rposint as range (subtype = posint); +create table ddtest2(f1 rposint); +insert into ddtest2 values('(-1,3]'); +alter domain posint add constraint c1 check(value >= 0); +drop table ddtest2; +drop type rposint; + alter domain posint add constraint c1 check(value >= 0); create domain posint2 as posint check (value % 2 = 0); |