From 3b11247aadf857bbcbfc765191273973d9ca9dd7 Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Mon, 16 Jan 2012 19:19:42 -0300 Subject: [PATCH] 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 --- doc/src/sgml/ref/alter_table.sgml | 6 +++++- src/backend/catalog/heap.c | 19 +++++++++++++++---- src/backend/commands/tablecmds.c | 22 +++++++++++++++++----- 3 files changed, 37 insertions(+), 10 deletions(-) diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index 1976f6dced..6f1917f5c4 100644 --- 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 dc801ae039..204236f550 100644 --- 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 d0843b2f58..cc210f06d3 100644 --- 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.) -- 2.40.0