]> granicus.if.org Git - postgresql/commitdiff
Fix handling of inherited check constraints in ALTER COLUMN TYPE.
authorTom Lane <tgl@sss.pgh.pa.us>
Mon, 5 Nov 2012 18:36:31 +0000 (13:36 -0500)
committerTom Lane <tgl@sss.pgh.pa.us>
Mon, 5 Nov 2012 18:36:31 +0000 (13:36 -0500)
This case got broken in 8.4 by the addition of an error check that
complains if ALTER TABLE ONLY is used on a table that has children.
We do use ONLY for this situation, but it's okay because the necessary
recursion occurs at a higher level.  So we need to have a separate
flag to suppress recursion without making the error check.

Reported and patched by Pavan Deolasee, with some editorial adjustments by
me.  Back-patch to 8.4, since this is a regression of functionality that
worked in earlier branches.

src/backend/commands/tablecmds.c
src/include/nodes/parsenodes.h
src/test/regress/expected/alter_table.out
src/test/regress/sql/alter_table.sql

index 91c2c6d51b6ab2256be7792d375ca21ee02401e3..b78ff76ce312101d190906982b5214c1a2d7d51f 100644 (file)
@@ -297,11 +297,11 @@ static void ATExecAddIndex(AlteredTableInfo *tab, Relation rel,
                           IndexStmt *stmt, bool is_rebuild);
 static void ATExecAddConstraint(List **wqueue,
                                        AlteredTableInfo *tab, Relation rel,
-                                       Constraint *newConstraint, bool recurse);
+                                       Constraint *newConstraint, bool recurse, bool is_readd);
 static void ATAddCheckConstraint(List **wqueue,
                                         AlteredTableInfo *tab, Relation rel,
                                         Constraint *constr,
-                                        bool recurse, bool recursing);
+                                        bool recurse, bool recursing, bool is_readd);
 static void ATAddForeignKeyConstraint(AlteredTableInfo *tab, Relation rel,
                                                  Constraint *fkconstraint);
 static void ATExecDropConstraint(Relation rel, const char *constrName,
@@ -2663,11 +2663,15 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
                        break;
                case AT_AddConstraint:  /* ADD CONSTRAINT */
                        ATExecAddConstraint(wqueue, tab, rel, (Constraint *) cmd->def,
-                                                               false);
+                                                               false, false);
                        break;
                case AT_AddConstraintRecurse:   /* ADD CONSTRAINT with recursion */
                        ATExecAddConstraint(wqueue, tab, rel, (Constraint *) cmd->def,
-                                                               true);
+                                                               true, false);
+                       break;
+               case AT_ReAddConstraint:        /* Re-add pre-existing check constraint */
+                       ATExecAddConstraint(wqueue, tab, rel, (Constraint *) cmd->def,
+                                                               false, true);
                        break;
                case AT_DropConstraint: /* DROP CONSTRAINT */
                        ATExecDropConstraint(rel, cmd->name, cmd->behavior,
@@ -4516,7 +4520,7 @@ ATExecAddIndex(AlteredTableInfo *tab, Relation rel,
  */
 static void
 ATExecAddConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
-                                       Constraint *newConstraint, bool recurse)
+                                       Constraint *newConstraint, bool recurse, bool is_readd)
 {
        Assert(IsA(newConstraint, Constraint));
 
@@ -4529,7 +4533,7 @@ ATExecAddConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
        {
                case CONSTR_CHECK:
                        ATAddCheckConstraint(wqueue, tab, rel,
-                                                                newConstraint, recurse, false);
+                                                                newConstraint, recurse, false, is_readd);
                        break;
 
                case CONSTR_FOREIGN:
@@ -4581,10 +4585,18 @@ ATExecAddConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
  * AddRelationNewConstraints would normally assign different names to the
  * child constraints.  To fix that, we must capture the name assigned at
  * the parent table and pass that down.
+ *
+ * When re-adding a previously existing constraint (during ALTER COLUMN TYPE),
+ * we don't need to recurse here, because recursion will be carried out at a
+ * higher level; the constraint name issue doesn't apply because the names
+ * have already been assigned and are just being re-used.  We need a separate
+ * "is_readd" flag for that; just setting recurse=false would result in an
+ * error if there are child tables.
  */
 static void
 ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
-                                        Constraint *constr, bool recurse, bool recursing)
+                                        Constraint *constr, bool recurse, bool recursing,
+                                        bool is_readd)
 {
        List       *newcons;
        ListCell   *lcon;
@@ -4643,6 +4655,13 @@ ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
        if (newcons == NIL)
                return;
 
+       /*
+        * Also, in a re-add operation, we don't need to recurse (that will be
+        * handled at higher levels).
+        */
+       if (is_readd)
+               return;
+
        /*
         * Propagate to children as appropriate.  Unlike most other ALTER
         * routines, we have to do this one level of recursion at a time; we can't
@@ -4675,7 +4694,7 @@ ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
 
                /* Recurse to child */
                ATAddCheckConstraint(wqueue, childtab, childrel,
-                                                        constr, recurse, true);
+                                                        constr, recurse, true, is_readd);
 
                heap_close(childrel, NoLock);
        }
@@ -6330,6 +6349,10 @@ ATPostAlterTypeParse(char *cmd, List **wqueue)
        /*
         * Attach each generated command to the proper place in the work queue.
         * Note this could result in creation of entirely new work-queue entries.
+        *
+        * Also note that we have to tweak the command subtypes, because it turns
+        * out that re-creation of indexes and constraints has to act a bit
+        * differently from initial creation.
         */
        foreach(list_item, querytree_list)
        {
@@ -6373,6 +6396,7 @@ ATPostAlterTypeParse(char *cmd, List **wqueue)
                                                                        lappend(tab->subcmds[AT_PASS_OLD_INDEX], cmd);
                                                                break;
                                                        case AT_AddConstraint:
+                                                               cmd->subtype = AT_ReAddConstraint;
                                                                tab->subcmds[AT_PASS_OLD_CONSTR] =
                                                                        lappend(tab->subcmds[AT_PASS_OLD_CONSTR], cmd);
                                                                break;
index 54eebbc4a5ea84a779e653141e741f8276e70b91..cbc016352ceae6547d1cba83fe12af4a04512c99 100644 (file)
@@ -1150,7 +1150,9 @@ typedef enum AlterTableType
        AT_EnableReplicaRule,           /* ENABLE REPLICA RULE name */
        AT_DisableRule,                         /* DISABLE RULE name */
        AT_AddInherit,                          /* INHERIT parent */
-       AT_DropInherit                          /* NO INHERIT parent */
+       AT_DropInherit,                         /* NO INHERIT parent */
+       /* this will be in a more natural position in 9.3: */
+       AT_ReAddConstraint                      /* internal to commands/tablecmds.c */
 } AlterTableType;
 
 typedef struct AlterTableCmd   /* one subcommand of an ALTER TABLE */
index a460371a112412c70bdf578b27f40adc45b164c1..18ac1a1ac2de82d9043143a4ee07b3bfdec32cd8 100644 (file)
@@ -1476,6 +1476,28 @@ select * from another;
 (3 rows)
 
 drop table another;
+-- ALTER TYPE with a check constraint and a child table (bug before Nov 2012)
+CREATE TABLE test_inh_check (a float check (a > 10.2));
+CREATE TABLE test_inh_check_child() INHERITS(test_inh_check);
+ALTER TABLE test_inh_check ALTER COLUMN a TYPE numeric;
+\d test_inh_check
+Table "public.test_inh_check"
+ Column |  Type   | Modifiers 
+--------+---------+-----------
+ a      | numeric | 
+Check constraints:
+    "test_inh_check_a_check" CHECK (a::double precision > 10.2::double precision)
+Number of child tables: 1 (Use \d+ to list them.)
+
+\d test_inh_check_child
+Table "public.test_inh_check_child"
+ Column |  Type   | Modifiers 
+--------+---------+-----------
+ a      | numeric | 
+Check constraints:
+    "test_inh_check_a_check" CHECK (a::double precision > 10.2::double precision)
+Inherits: test_inh_check
+
 -- disallow recursive containment of row types
 create temp table recur1 (f1 int);
 alter table recur1 add column f2 recur1; -- fails
index 8f472cff1e5e48026ca407a88f12669a205823e3..c9c3f7e0e09d51659e603af6ac81043cb349d88b 100644 (file)
@@ -1092,6 +1092,13 @@ select * from another;
 
 drop table another;
 
+-- ALTER TYPE with a check constraint and a child table (bug before Nov 2012)
+CREATE TABLE test_inh_check (a float check (a > 10.2));
+CREATE TABLE test_inh_check_child() INHERITS(test_inh_check);
+ALTER TABLE test_inh_check ALTER COLUMN a TYPE numeric;
+\d test_inh_check
+\d test_inh_check_child
+
 -- disallow recursive containment of row types
 create temp table recur1 (f1 int);
 alter table recur1 add column f2 recur1; -- fails