Disallow merging ONLY constraints in children tables

When creating a child table, or when attaching an existing table as
child of another, we must not allow inheritable constraints to be
merged with non-inheritable ones, because then grandchildren would not
properly get the constraint.  This would violate the grandparent's
expectations.

Bugs noted by Robert Haas.

Author: Nikhil Sontakke

diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index 1976f6dcedeaf9053e364331dff49723a18f3288..6f1917f5c47042d933102d904f10011c22952461 100644 (file)
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -482,7 +482,11 @@ ALTER TABLE name
 
      
       There must also be matching child-table constraints for all
-      CHECK constraints of the parent. Currently
+      CHECK constraints of the parent, except those
+      marked non-inheritable (that is, created with ALTER TABLE ONLY)
+      in the parent, which are ignored; all child-table constraints matched
+      must not be marked non-inheritable.
+      Currently
       UNIQUE, PRIMARY KEY, and
       FOREIGN KEY constraints are not considered, but
       this might change in the future.

diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index dc801ae0396f24ce1d369bfe2c92221d91235a9b..204236f550eb096fa5ea850835f89b515ee5bc83 100644 (file)
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -2251,6 +2251,8 @@ AddRelationNewConstraints(Relation rel,
  *
  * Returns TRUE if merged (constraint is a duplicate), or FALSE if it's
  * got a so-far-unique name, or throws error if conflict.
+ *
+ * XXX See MergeConstraintsIntoExisting too if you change this code.
  */
 static bool
 MergeWithExistingConstraint(Relation rel, char *ccname, Node *expr,
@@ -2307,12 +2309,17 @@ MergeWithExistingConstraint(Relation rel, char *ccname, Node *expr,
                        (errcode(ERRCODE_DUPLICATE_OBJECT),
                errmsg("constraint \"%s\" for relation \"%s\" already exists",
                       ccname, RelationGetRelationName(rel))));
-           /* OK to update the tuple */
-           ereport(NOTICE,
-              (errmsg("merging constraint \"%s\" with inherited definition",
-                      ccname)));
+
            tup = heap_copytuple(tup);
            con = (Form_pg_constraint) GETSTRUCT(tup);
+
+           /* If the constraint is "only" then cannot merge */
+           if (con->conisonly)
+               ereport(ERROR,
+                       (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                        errmsg("constraint \"%s\" conflicts with non-inherited constraint on relation \"%s\"",
+                               ccname, RelationGetRelationName(rel))));
+
            if (is_local)
                con->conislocal = true;
            else
@@ -2322,6 +2329,10 @@ MergeWithExistingConstraint(Relation rel, char *ccname, Node *expr,
                Assert(is_local);
                con->conisonly = true;
            }
+           /* OK to update the tuple */
+           ereport(NOTICE,
+                   (errmsg("merging constraint \"%s\" with inherited definition",
+                           ccname)));
            simple_heap_update(conDesc, &tup->t_self, tup);
            CatalogUpdateIndexes(conDesc, tup);
            break;

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index d0843b2f5889f5319b70b08b1853a1203c5b0111..cc210f06d3037f1fa57e4308d806b30806dac7ba 100644 (file)
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -8818,18 +8818,18 @@ MergeAttributesIntoExisting(Relation child_rel, Relation parent_rel)
  * Check constraints in child table match up with constraints in parent,
  * and increment their coninhcount.
  *
+ * Constraints that are marked ONLY in the parent are ignored.
+ *
  * Called by ATExecAddInherit
  *
  * Currently all constraints in parent must be present in the child. One day we
- * may consider adding new constraints like CREATE TABLE does. We may also want
- * to allow an optional flag on parent table constraints indicating they are
- * intended to ONLY apply to the master table, not to the children. That would
- * make it possible to ensure no records are mistakenly inserted into the
- * master in partitioned tables rather than the appropriate child.
+ * may consider adding new constraints like CREATE TABLE does.
  *
  * XXX This is O(N^2) which may be an issue with tables with hundreds of
  * constraints. As long as tables have more like 10 constraints it shouldn't be
  * a problem though. Even 100 constraints ought not be the end of the world.
+ *
+ * XXX See MergeWithExistingConstraint too if you change this code.
  */
 static void
 MergeConstraintsIntoExisting(Relation child_rel, Relation parent_rel)
@@ -8862,6 +8862,10 @@ MergeConstraintsIntoExisting(Relation child_rel, Relation parent_rel)
        if (parent_con->contype != CONSTRAINT_CHECK)
            continue;
 
+       /* if the parent's constraint is marked ONLY, it's not inherited */
+       if (parent_con->conisonly)
+           continue;
+
        /* Search for a child constraint matching this one */
        ScanKeyInit(&child_key,
                    Anum_pg_constraint_conrelid,
@@ -8889,6 +8893,14 @@ MergeConstraintsIntoExisting(Relation child_rel, Relation parent_rel)
                                RelationGetRelationName(child_rel),
                                NameStr(parent_con->conname))));
 
+           /* If the constraint is "only" then cannot merge */
+           if (child_con->conisonly)
+               ereport(ERROR,
+                       (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                        errmsg("constraint \"%s\" conflicts with non-inherited constraint on child table \"%s\"",
+                               NameStr(child_con->conname),
+                               RelationGetRelationName(child_rel))));
+
            /*
             * OK, bump the child constraint's inheritance count.  (If we fail
             * later on, this change will just roll back.)