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

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index f17fbc7e36f4f5ed782cea3a619f18c923399fef..b3b6bca6310218f06735467f3431f9d5afb5f186 100644 (file)
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -4249,13 +4249,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?)
@@ -4268,11 +4273,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);
 
@@ -4294,8 +4301,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;
@@ -4351,14 +4372,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 e52643780b44da4e826fa0d286e0452c5f0ce0a2..30988d5f1ddc108d927c7ec7de25318e4efc636e 100644 (file)
--- a/src/backend/commands/typecmds.c
+++ b/src/backend/commands/typecmds.c
@@ -2724,10 +2724,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.
@@ -2736,6 +2735,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;
@@ -2743,6 +2743,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.)
@@ -2769,20 +2772,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;
        }
 
@@ -2819,7 +2834,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 966e77ff3318bcbc267352ac6fd4f0a72a4f27b0..3f91a87940d5fe9b9feb1274bc08dc79b939ae82 100644 (file)
--- 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 c149efc347c862e9ee41d2da31a8b05d89cd6380..b2ea440a933e5e33220026ee873210be19afad03 100644 (file)
--- 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);