Fix my oversight in enabling domains-of-domains: ALTER DOMAIN ADD CONSTRAINT
authorTom Lane <tgl@sss.pgh.pa.us>
Fri, 11 May 2007 20:17:15 +0000 (20:17 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Fri, 11 May 2007 20:17:15 +0000 (20:17 +0000)
needs to check the new constraint against columns of derived domains too.

Also, make it error out if the domain to be modified is used within any
composite-type columns.  Eventually we should support that case, but it seems
a bit painful, and not suitable for a back-patch.  For the moment just let the
user know we can't do it.

Backpatch to 8.2, which is the only released version that allows nested
domains.  Possibly the other part should be back-patched further.

doc/src/sgml/ref/alter_domain.sgml
src/backend/commands/tablecmds.c
src/backend/commands/typecmds.c
src/include/commands/tablecmds.h
src/test/regress/expected/domain.out
src/test/regress/sql/domain.sql

index 1111da12f8e333ac76c3ef04219e94c3250e08ce..da024558f482b7ae7d49a247f8582f2ff2370dbd 100644 (file)
@@ -1,5 +1,5 @@
 <!--
-$PostgreSQL: pgsql/doc/src/sgml/ref/alter_domain.sgml,v 1.21 2007/01/31 23:26:02 momjian Exp $
+$PostgreSQL: pgsql/doc/src/sgml/ref/alter_domain.sgml,v 1.22 2007/05/11 20:16:32 tgl Exp $
 PostgreSQL documentation
 -->
 
@@ -197,6 +197,19 @@ ALTER DOMAIN <replaceable class="PARAMETER">name</replaceable>
    </para>
   </refsect1>
 
+ <refsect1>
+  <title>Notes</title>
+
+  <para>
+   Currently, <command>ALTER DOMAIN ADD CONSTRAINT</> and
+   <command>ALTER DOMAIN SET NOT NULL</> will fail if the named domain or
+   any derived domain is used within a composite-type column of any
+   table in the database.  They should eventually be improved to be
+   able to verify the new constraint for such nested columns.
+  </para>
+
+ </refsect1>
+
  <refsect1>
   <title>Examples</title>
 
index a1bbf1778932b3c00f2671f6eea3dfdb4b4bbc21..eecd4943ae62b5162876de1e808ab4935ecd3f37 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.220 2007/05/11 17:57:12 tgl Exp $
+ *   $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.221 2007/05/11 20:16:36 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -206,8 +206,6 @@ static void ATSimpleRecursion(List **wqueue, Relation rel,
                  AlterTableCmd *cmd, bool recurse);
 static void ATOneLevelRecursion(List **wqueue, Relation rel,
                    AlterTableCmd *cmd);
-static void find_composite_type_dependencies(Oid typeOid,
-                                const char *origTblName);
 static void ATPrepAddColumn(List **wqueue, Relation rel, bool recurse,
                AlterTableCmd *cmd);
 static void ATExecAddColumn(AlteredTableInfo *tab, Relation rel,
@@ -2410,7 +2408,8 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
     */
    if (newrel)
        find_composite_type_dependencies(oldrel->rd_rel->reltype,
-                                        RelationGetRelationName(oldrel));
+                                        RelationGetRelationName(oldrel),
+                                        NULL);
 
    /*
     * Generate the constraint and default execution states
@@ -2812,16 +2811,21 @@ ATOneLevelRecursion(List **wqueue, Relation rel,
 /*
  * find_composite_type_dependencies
  *
- * Check to see if a table's rowtype is being used as a column in some
+ * 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!).
  * 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.
  *
+ * Caller should provide either a table name or a type name (not both) to
+ * report in the error message, if any.
+ *
  * We assume that functions and views depending on the type are not reasons
  * to reject the ALTER.  (How safe is this really?)
  */
-static void
-find_composite_type_dependencies(Oid typeOid, const char *origTblName)
+void
+find_composite_type_dependencies(Oid typeOid,
+                                const char *origTblName,
+                                const char *origTypeName)
 {
    Relation    depRel;
    ScanKeyData key[2];
@@ -2864,12 +2868,20 @@ find_composite_type_dependencies(Oid typeOid, const char *origTblName)
 
        if (rel->rd_rel->relkind == RELKIND_RELATION)
        {
-           ereport(ERROR,
-                   (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-                    errmsg("cannot alter table \"%s\" because column \"%s\".\"%s\" uses its rowtype",
-                           origTblName,
-                           RelationGetRelationName(rel),
-                           NameStr(att->attname))));
+           if (origTblName)
+               ereport(ERROR,
+                       (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                        errmsg("cannot alter table \"%s\" because column \"%s\".\"%s\" uses its rowtype",
+                               origTblName,
+                               RelationGetRelationName(rel),
+                               NameStr(att->attname))));
+           else
+               ereport(ERROR,
+                       (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                        errmsg("cannot alter type \"%s\" because column \"%s\".\"%s\" uses it",
+                               origTypeName,
+                               RelationGetRelationName(rel),
+                               NameStr(att->attname))));
        }
        else if (OidIsValid(rel->rd_rel->reltype))
        {
@@ -2878,7 +2890,7 @@ find_composite_type_dependencies(Oid typeOid, const char *origTblName)
             * recursively check for indirect dependencies via its rowtype.
             */
            find_composite_type_dependencies(rel->rd_rel->reltype,
-                                            origTblName);
+                                            origTblName, origTypeName);
        }
 
        relation_close(rel, AccessShareLock);
@@ -2894,7 +2906,7 @@ find_composite_type_dependencies(Oid typeOid, const char *origTblName)
     */
    arrayOid = get_array_type(typeOid);
    if (OidIsValid(arrayOid))
-       find_composite_type_dependencies(arrayOid, origTblName);
+       find_composite_type_dependencies(arrayOid, origTblName, origTypeName);
 }
 
 
index 7911f6df3a559b89b1b2da64232e2107388c7408..915c669c9718930c9ea07090634ff0e0a3a47fa3 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/commands/typecmds.c,v 1.102 2007/05/11 17:57:12 tgl Exp $
+ *   $PostgreSQL: pgsql/src/backend/commands/typecmds.c,v 1.103 2007/05/11 20:16:54 tgl Exp $
  *
  * DESCRIPTION
  *   The "DefineFoo" routines take the parse tree and pick out the
@@ -1805,6 +1805,10 @@ AlterDomainAddConstraint(List *names, Node *newConstraint)
  * the domain type.  We have opened each rel and acquired the specified lock
  * type on it.
  *
+ * We support nested domains by including attributes that are of derived
+ * domain types.  Current callers do not need to distinguish between attributes
+ * that are of exactly the given domain and those that are of derived domains.
+ *
  * XXX this is completely broken because there is no way to lock the domain
  * to prevent columns from being added or dropped while our command runs.
  * We can partially protect against column drops by locking relations as we
@@ -1814,9 +1818,11 @@ AlterDomainAddConstraint(List *names, Node *newConstraint)
  * trivial risk of deadlock.  We can minimize but not eliminate the deadlock
  * risk by using the weakest suitable lock (ShareLock for most callers).
  *
- * XXX to support domains over domains, we'd need to make this smarter,
- * or make its callers smarter, so that we could find columns of derived
- * domains.  Arrays of domains would be a problem too.
+ * 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.
  *
  * Generally used for retrieving a list of tests when adding
  * new constraints to a domain.
@@ -1858,7 +1864,23 @@ get_rels_with_domain(Oid domainOid, LOCKMODE lockmode)
        Form_pg_attribute pg_att;
        int         ptr;
 
-       /* Ignore dependees that aren't user columns of relations */
+       /* Check for directly dependent types --- must be domains */
+       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));
+           continue;
+       }
+
+       /* Else, ignore dependees that aren't user columns of relations */
        /* (we assume system columns are never of domain types) */
        if (pg_depend->classid != RelationRelationId ||
            pg_depend->objsubid <= 0)
@@ -1884,7 +1906,16 @@ get_rels_with_domain(Oid domainOid, LOCKMODE lockmode)
            /* Acquire requested lock on relation */
            rel = relation_open(pg_depend->objid, lockmode);
 
-           /* It could be a view or composite type; if so ignore it */
+           /*
+            * Check to see if rowtype is stored anyplace as a composite-type
+            * column; if so we have to fail, for now anyway.
+            */
+           if (OidIsValid(rel->rd_rel->reltype))
+               find_composite_type_dependencies(rel->rd_rel->reltype,
+                                                NULL,
+                                                format_type_be(domainOid));
+
+           /* Otherwise we can ignore views, composite types, etc */
            if (rel->rd_rel->relkind != RELKIND_RELATION)
            {
                relation_close(rel, lockmode);
index fc428aad917de987b76ff4ed811cb04ef66f7a71..3acbe84b522cef18de802bf602985104cad2368f 100644 (file)
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2007, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/commands/tablecmds.h,v 1.32 2007/01/05 22:19:54 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/commands/tablecmds.h,v 1.33 2007/05/11 20:17:10 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -45,6 +45,10 @@ extern void renameatt(Oid myrelid,
 extern void renamerel(Oid myrelid,
          const char *newrelname);
 
+extern void find_composite_type_dependencies(Oid typeOid,
+                                            const char *origTblName,
+                                            const char *origTypeName);
+
 extern AttrNumber *varattnos_map(TupleDesc old, TupleDesc new);
 extern AttrNumber *varattnos_map_schema(TupleDesc old, List *schema);
 extern void change_varattnos_of_a_node(Node *node, const AttrNumber *newattno);
index e01fc7cadcc1499fc460b7df324529f6b5e5435b..c7d5b50334537ccd1db499bb75b4d3b0eb1a1e6d 100644 (file)
@@ -438,3 +438,32 @@ select doubledecrement(3); -- good
                1
 (1 row)
 
+-- Check that ALTER DOMAIN tests columns of derived types
+create domain posint as int4;
+-- Currently, this doesn't work for composite types, but verify it complains
+create type ddtest1 as (f1 posint);
+create table ddtest2(f1 ddtest1);
+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;
+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;
+alter domain posint add constraint c1 check(value >= 0);
+create domain posint2 as posint check (value % 2 = 0);
+create table ddtest2(f1 posint2);
+insert into ddtest2 values(11); -- fail
+ERROR:  value for domain posint2 violates check constraint "posint2_check"
+insert into ddtest2 values(-2); -- fail
+ERROR:  value for domain posint2 violates check constraint "c1"
+insert into ddtest2 values(2);
+alter domain posint add constraint c2 check(value >= 10); -- fail
+ERROR:  column "f1" of table "ddtest2" contains values that violate the new constraint
+alter domain posint add constraint c2 check(value > 0); -- OK
+drop table ddtest2;
+drop type ddtest1;
+drop domain posint cascade;
+NOTICE:  drop cascades to type posint2
index 4e103172edfb2d11bd4e64a5863f66ae037957d0..7da99719911348c134f4fe58c73e711d0f896da6 100644 (file)
@@ -351,3 +351,34 @@ select doubledecrement(0); -- fail before call
 select doubledecrement(1); -- fail at assignment to v
 select doubledecrement(2); -- fail at return
 select doubledecrement(3); -- good
+
+-- Check that ALTER DOMAIN tests columns of derived types
+
+create domain posint as int4;
+
+-- Currently, this doesn't work for composite types, but verify it complains
+create type ddtest1 as (f1 posint);
+create table ddtest2(f1 ddtest1);
+insert into ddtest2 values(row(-1));
+alter domain posint add constraint c1 check(value >= 0);
+drop table ddtest2;
+
+create table ddtest2(f1 ddtest1[]);
+insert into ddtest2 values('{(-1)}');
+alter domain posint add constraint c1 check(value >= 0);
+drop table ddtest2;
+
+alter domain posint add constraint c1 check(value >= 0);
+
+create domain posint2 as posint check (value % 2 = 0);
+create table ddtest2(f1 posint2);
+insert into ddtest2 values(11); -- fail
+insert into ddtest2 values(-2); -- fail
+insert into ddtest2 values(2);
+
+alter domain posint add constraint c2 check(value >= 10); -- fail
+alter domain posint add constraint c2 check(value > 0); -- OK
+
+drop table ddtest2;
+drop type ddtest1;
+drop domain posint cascade;