Fix handling of inherited check constraints in ALTER COLUMN TYPE.

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.

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 7588b422a3a110559d1962173306e7fa60f98d0d..becbe19627b9178b8e5b4812b45346de24bdf943 100644 (file)
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -291,11 +291,11 @@ static void ATExecAddIndex(AlteredTableInfo *tab, Relation rel,
                           IndexStmt *stmt, bool is_rebuild);
 static void ATExecAddConstraint(List **wqueue,
                                        AlteredTableInfo *tab, Relation rel,
-                                       Node *newConstraint, bool recurse);
+                                       Node *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,
                                                  FkConstraint *fkconstraint);
 static void ATExecDropConstraint(Relation rel, const char *constrName,
@@ -2559,10 +2559,13 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
                        ATExecAddIndex(tab, rel, (IndexStmt *) cmd->def, true);
                        break;
                case AT_AddConstraint:  /* ADD CONSTRAINT */
-                       ATExecAddConstraint(wqueue, tab, rel, cmd->def, false);
+                       ATExecAddConstraint(wqueue, tab, rel, cmd->def, false, false);
                        break;
                case AT_AddConstraintRecurse:   /* ADD CONSTRAINT with recursion */
-                       ATExecAddConstraint(wqueue, tab, rel, cmd->def, true);
+                       ATExecAddConstraint(wqueue, tab, rel, cmd->def, true, false);
+                       break;
+               case AT_ReAddConstraint:        /* Re-add pre-existing check constraint */
+                       ATExecAddConstraint(wqueue, tab, rel, cmd->def, false, true);
                        break;
                case AT_DropConstraint: /* DROP CONSTRAINT */
                        ATExecDropConstraint(rel, cmd->name, cmd->behavior, false, false);
@@ -4309,7 +4312,7 @@ ATExecAddIndex(AlteredTableInfo *tab, Relation rel,
  */
 static void
 ATExecAddConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
-                                       Node *newConstraint, bool recurse)
+                                       Node *newConstraint, bool recurse, bool is_readd)
 {
        switch (nodeTag(newConstraint))
        {
@@ -4327,7 +4330,7 @@ ATExecAddConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
                                {
                                        case CONSTR_CHECK:
                                                ATAddCheckConstraint(wqueue, tab, rel,
-                                                                                        constr, recurse, false);
+                                                                                        constr, recurse, false, is_readd);
                                                break;
                                        default:
                                                elog(ERROR, "unrecognized constraint type: %d",
@@ -4387,10 +4390,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;
@@ -4449,6 +4460,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
@@ -4481,7 +4499,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);
        }
@@ -6057,6 +6075,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)
        {
@@ -6100,6 +6122,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;

diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index f27d9a26bda9ea5e6e3c9a228dc82325151a463f..7123a5a130f1bb349df7ce7abc68eae3a8598eb2 100644 (file)
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -1131,7 +1131,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 */

diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index e8887db00e9718a8f97441ed786132e681915166..f7f9e631b875b8da8657bd9654146bd974df50e6 100644 (file)
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -1456,6 +1456,27 @@ 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)
+
+\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

diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 26b5f03c179a5cd9d124fe02bbd2273086d1b461..4375867f977854c9217b284c4b71140591434998 100644 (file)
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -1081,6 +1081,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