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
This commit is contained in:
parent
1b9f774090
commit
3b11247aad
@ -482,7 +482,11 @@ ALTER TABLE <replaceable class="PARAMETER">name</replaceable>
|
|||||||
|
|
||||||
<para>
|
<para>
|
||||||
There must also be matching child-table constraints for all
|
There must also be matching child-table constraints for all
|
||||||
<literal>CHECK</literal> constraints of the parent. Currently
|
<literal>CHECK</literal> constraints of the parent, except those
|
||||||
|
marked non-inheritable (that is, created with <literal>ALTER TABLE ONLY</literal>)
|
||||||
|
in the parent, which are ignored; all child-table constraints matched
|
||||||
|
must not be marked non-inheritable.
|
||||||
|
Currently
|
||||||
<literal>UNIQUE</literal>, <literal>PRIMARY KEY</literal>, and
|
<literal>UNIQUE</literal>, <literal>PRIMARY KEY</literal>, and
|
||||||
<literal>FOREIGN KEY</literal> constraints are not considered, but
|
<literal>FOREIGN KEY</literal> constraints are not considered, but
|
||||||
this might change in the future.
|
this might change in the future.
|
||||||
|
@ -2251,6 +2251,8 @@ AddRelationNewConstraints(Relation rel,
|
|||||||
*
|
*
|
||||||
* Returns TRUE if merged (constraint is a duplicate), or FALSE if it's
|
* Returns TRUE if merged (constraint is a duplicate), or FALSE if it's
|
||||||
* got a so-far-unique name, or throws error if conflict.
|
* got a so-far-unique name, or throws error if conflict.
|
||||||
|
*
|
||||||
|
* XXX See MergeConstraintsIntoExisting too if you change this code.
|
||||||
*/
|
*/
|
||||||
static bool
|
static bool
|
||||||
MergeWithExistingConstraint(Relation rel, char *ccname, Node *expr,
|
MergeWithExistingConstraint(Relation rel, char *ccname, Node *expr,
|
||||||
@ -2307,12 +2309,17 @@ MergeWithExistingConstraint(Relation rel, char *ccname, Node *expr,
|
|||||||
(errcode(ERRCODE_DUPLICATE_OBJECT),
|
(errcode(ERRCODE_DUPLICATE_OBJECT),
|
||||||
errmsg("constraint \"%s\" for relation \"%s\" already exists",
|
errmsg("constraint \"%s\" for relation \"%s\" already exists",
|
||||||
ccname, RelationGetRelationName(rel))));
|
ccname, RelationGetRelationName(rel))));
|
||||||
/* OK to update the tuple */
|
|
||||||
ereport(NOTICE,
|
|
||||||
(errmsg("merging constraint \"%s\" with inherited definition",
|
|
||||||
ccname)));
|
|
||||||
tup = heap_copytuple(tup);
|
tup = heap_copytuple(tup);
|
||||||
con = (Form_pg_constraint) GETSTRUCT(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)
|
if (is_local)
|
||||||
con->conislocal = true;
|
con->conislocal = true;
|
||||||
else
|
else
|
||||||
@ -2322,6 +2329,10 @@ MergeWithExistingConstraint(Relation rel, char *ccname, Node *expr,
|
|||||||
Assert(is_local);
|
Assert(is_local);
|
||||||
con->conisonly = true;
|
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);
|
simple_heap_update(conDesc, &tup->t_self, tup);
|
||||||
CatalogUpdateIndexes(conDesc, tup);
|
CatalogUpdateIndexes(conDesc, tup);
|
||||||
break;
|
break;
|
||||||
|
@ -8818,18 +8818,18 @@ MergeAttributesIntoExisting(Relation child_rel, Relation parent_rel)
|
|||||||
* Check constraints in child table match up with constraints in parent,
|
* Check constraints in child table match up with constraints in parent,
|
||||||
* and increment their coninhcount.
|
* and increment their coninhcount.
|
||||||
*
|
*
|
||||||
|
* Constraints that are marked ONLY in the parent are ignored.
|
||||||
|
*
|
||||||
* Called by ATExecAddInherit
|
* Called by ATExecAddInherit
|
||||||
*
|
*
|
||||||
* Currently all constraints in parent must be present in the child. One day we
|
* 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
|
* may consider adding new constraints like CREATE TABLE does.
|
||||||
* 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.
|
|
||||||
*
|
*
|
||||||
* XXX This is O(N^2) which may be an issue with tables with hundreds of
|
* 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
|
* 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.
|
* 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
|
static void
|
||||||
MergeConstraintsIntoExisting(Relation child_rel, Relation parent_rel)
|
MergeConstraintsIntoExisting(Relation child_rel, Relation parent_rel)
|
||||||
@ -8862,6 +8862,10 @@ MergeConstraintsIntoExisting(Relation child_rel, Relation parent_rel)
|
|||||||
if (parent_con->contype != CONSTRAINT_CHECK)
|
if (parent_con->contype != CONSTRAINT_CHECK)
|
||||||
continue;
|
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 */
|
/* Search for a child constraint matching this one */
|
||||||
ScanKeyInit(&child_key,
|
ScanKeyInit(&child_key,
|
||||||
Anum_pg_constraint_conrelid,
|
Anum_pg_constraint_conrelid,
|
||||||
@ -8889,6 +8893,14 @@ MergeConstraintsIntoExisting(Relation child_rel, Relation parent_rel)
|
|||||||
RelationGetRelationName(child_rel),
|
RelationGetRelationName(child_rel),
|
||||||
NameStr(parent_con->conname))));
|
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
|
* OK, bump the child constraint's inheritance count. (If we fail
|
||||||
* later on, this change will just roll back.)
|
* later on, this change will just roll back.)
|
||||||
|
Loading…
x
Reference in New Issue
Block a user