diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index 7d7d062c06..5f3cf97be4 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -104,6 +104,7 @@ static void StoreConstraints(Relation rel, List *cooked_constraints, bool is_internal); static bool MergeWithExistingConstraint(Relation rel, char *ccname, Node *expr, bool allow_merge, bool is_local, + bool is_initially_valid, bool is_no_inherit); static void SetRelationNumChecks(Relation rel, int numchecks); static Node *cookConstraint(ParseState *pstate, @@ -2302,6 +2303,7 @@ AddRelationNewConstraints(Relation rel, */ if (MergeWithExistingConstraint(rel, ccname, expr, allow_merge, is_local, + cdef->initially_valid, cdef->is_no_inherit)) continue; } @@ -2392,6 +2394,7 @@ AddRelationNewConstraints(Relation rel, static bool MergeWithExistingConstraint(Relation rel, char *ccname, Node *expr, bool allow_merge, bool is_local, + bool is_initially_valid, bool is_no_inherit) { bool found; @@ -2439,22 +2442,47 @@ MergeWithExistingConstraint(Relation rel, char *ccname, Node *expr, if (equal(expr, stringToNode(TextDatumGetCString(val)))) found = true; } + + /* + * If the existing constraint is purely inherited (no local + * definition) then interpret addition of a local constraint as a + * legal merge. This allows ALTER ADD CONSTRAINT on parent and + * child tables to be given in either order with same end state. + */ + if (is_local && !con->conislocal) + allow_merge = true; + if (!found || !allow_merge) ereport(ERROR, (errcode(ERRCODE_DUPLICATE_OBJECT), errmsg("constraint \"%s\" for relation \"%s\" already exists", ccname, RelationGetRelationName(rel)))); - tup = heap_copytuple(tup); - con = (Form_pg_constraint) GETSTRUCT(tup); - - /* If the constraint is "no inherit" then cannot merge */ + /* If the child constraint is "no inherit" then cannot merge */ if (con->connoinherit) ereport(ERROR, (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), errmsg("constraint \"%s\" conflicts with non-inherited constraint on relation \"%s\"", ccname, RelationGetRelationName(rel)))); + /* + * If the child constraint is "not valid" then cannot merge with a + * valid parent constraint + */ + if (is_initially_valid && !con->convalidated) + ereport(ERROR, + (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("constraint \"%s\" conflicts with NOT VALID constraint on relation \"%s\"", + 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 (is_local) con->conislocal = true; else @@ -2464,10 +2492,6 @@ MergeWithExistingConstraint(Relation rel, char *ccname, Node *expr, Assert(is_local); con->connoinherit = 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 7351a5edef..10e18b9fe3 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -10342,7 +10342,7 @@ MergeConstraintsIntoExisting(Relation child_rel, Relation parent_rel) RelationGetRelationName(child_rel), NameStr(parent_con->conname)))); - /* If the constraint is "no inherit" then cannot merge */ + /* If the child constraint is "no inherit" then cannot merge */ if (child_con->connoinherit) ereport(ERROR, (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), @@ -10350,6 +10350,17 @@ MergeConstraintsIntoExisting(Relation child_rel, Relation parent_rel) NameStr(child_con->conname), RelationGetRelationName(child_rel)))); + /* + * If the child constraint is "not valid" then cannot merge with a + * valid parent constraint + */ + if (parent_con->convalidated && !child_con->convalidated) + ereport(ERROR, + (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("constraint \"%s\" conflicts with NOT VALID 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.) diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out index 89b6c1caf4..46442e8986 100644 --- a/src/test/regress/expected/inherit.out +++ b/src/test/regress/expected/inherit.out @@ -1090,6 +1090,56 @@ Inherits: test_foreign_constraints DROP TABLE test_foreign_constraints_inh; DROP TABLE test_foreign_constraints; DROP TABLE test_primary_constraints; +-- Test that parent and child CHECK constraints can be created in either order +create table p1(f1 int); +create table p1_c1() inherits(p1); +alter table p1 add constraint inh_check_constraint1 check (f1 > 0); +alter table p1_c1 add constraint inh_check_constraint1 check (f1 > 0); +NOTICE: merging constraint "inh_check_constraint1" with inherited definition +alter table p1_c1 add constraint inh_check_constraint2 check (f1 < 10); +alter table p1 add constraint inh_check_constraint2 check (f1 < 10); +NOTICE: merging constraint "inh_check_constraint2" with inherited definition +select conrelid::regclass::text as relname, conname, conislocal, coninhcount +from pg_constraint where conname like 'inh\_check\_constraint%' +order by 1, 2; + relname | conname | conislocal | coninhcount +---------+-----------------------+------------+------------- + p1 | inh_check_constraint1 | t | 0 + p1 | inh_check_constraint2 | t | 0 + p1_c1 | inh_check_constraint1 | t | 1 + p1_c1 | inh_check_constraint2 | t | 1 +(4 rows) + +drop table p1 cascade; +NOTICE: drop cascades to table p1_c1 +-- Test that a valid child can have not-valid parent, but not vice versa +create table invalid_check_con(f1 int); +create table invalid_check_con_child() inherits(invalid_check_con); +alter table invalid_check_con_child add constraint inh_check_constraint check(f1 > 0) not valid; +alter table invalid_check_con add constraint inh_check_constraint check(f1 > 0); -- fail +ERROR: constraint "inh_check_constraint" conflicts with NOT VALID constraint on relation "invalid_check_con_child" +alter table invalid_check_con_child drop constraint inh_check_constraint; +insert into invalid_check_con values(0); +alter table invalid_check_con_child add constraint inh_check_constraint check(f1 > 0); +alter table invalid_check_con add constraint inh_check_constraint check(f1 > 0) not valid; +NOTICE: merging constraint "inh_check_constraint" with inherited definition +insert into invalid_check_con values(0); -- fail +ERROR: new row for relation "invalid_check_con" violates check constraint "inh_check_constraint" +DETAIL: Failing row contains (0). +insert into invalid_check_con_child values(0); -- fail +ERROR: new row for relation "invalid_check_con_child" violates check constraint "inh_check_constraint" +DETAIL: Failing row contains (0). +select conrelid::regclass::text as relname, conname, + convalidated, conislocal, coninhcount, connoinherit +from pg_constraint where conname like 'inh\_check\_constraint%' +order by 1, 2; + relname | conname | convalidated | conislocal | coninhcount | connoinherit +-------------------------+----------------------+--------------+------------+-------------+-------------- + invalid_check_con | inh_check_constraint | f | t | 0 | f + invalid_check_con_child | inh_check_constraint | t | t | 1 | f +(2 rows) + +-- We don't drop the invalid_check_con* tables, to test dump/reload with -- -- Test parameterized append plans for inheritance trees -- diff --git a/src/test/regress/expected/sanity_check.out b/src/test/regress/expected/sanity_check.out index eb0bc88ef1..e0f3af0782 100644 --- a/src/test/regress/expected/sanity_check.out +++ b/src/test/regress/expected/sanity_check.out @@ -62,6 +62,8 @@ int2_tbl|f int4_tbl|f int8_tbl|f interval_tbl|f +invalid_check_con|f +invalid_check_con_child|f iportaltest|f kd_point_tbl|t line_tbl|f diff --git a/src/test/regress/sql/inherit.sql b/src/test/regress/sql/inherit.sql index 793c216376..a5e6ecdb86 100644 --- a/src/test/regress/sql/inherit.sql +++ b/src/test/regress/sql/inherit.sql @@ -334,6 +334,45 @@ DROP TABLE test_foreign_constraints_inh; DROP TABLE test_foreign_constraints; DROP TABLE test_primary_constraints; +-- Test that parent and child CHECK constraints can be created in either order +create table p1(f1 int); +create table p1_c1() inherits(p1); + +alter table p1 add constraint inh_check_constraint1 check (f1 > 0); +alter table p1_c1 add constraint inh_check_constraint1 check (f1 > 0); + +alter table p1_c1 add constraint inh_check_constraint2 check (f1 < 10); +alter table p1 add constraint inh_check_constraint2 check (f1 < 10); + +select conrelid::regclass::text as relname, conname, conislocal, coninhcount +from pg_constraint where conname like 'inh\_check\_constraint%' +order by 1, 2; + +drop table p1 cascade; + +-- Test that a valid child can have not-valid parent, but not vice versa +create table invalid_check_con(f1 int); +create table invalid_check_con_child() inherits(invalid_check_con); + +alter table invalid_check_con_child add constraint inh_check_constraint check(f1 > 0) not valid; +alter table invalid_check_con add constraint inh_check_constraint check(f1 > 0); -- fail +alter table invalid_check_con_child drop constraint inh_check_constraint; + +insert into invalid_check_con values(0); + +alter table invalid_check_con_child add constraint inh_check_constraint check(f1 > 0); +alter table invalid_check_con add constraint inh_check_constraint check(f1 > 0) not valid; + +insert into invalid_check_con values(0); -- fail +insert into invalid_check_con_child values(0); -- fail + +select conrelid::regclass::text as relname, conname, + convalidated, conislocal, coninhcount, connoinherit +from pg_constraint where conname like 'inh\_check\_constraint%' +order by 1, 2; + +-- We don't drop the invalid_check_con* tables, to test dump/reload with + -- -- Test parameterized append plans for inheritance trees --