Re: [HACKERS] Conflicting constraint being merged

2016-10-13 Thread Tom Lane
Amit Langote  writes:
> Currently, if child table has a non-inherited constraint and a constraint
> with the same name is added to the parent, it will fail with an error as
> illustrated below:
> ...
> If we had allowed it to be merged, any children of child itself won't
> inherit that constraint (because on child it's marked NO INHERIT), which
> would not be good.

Right.  Merging must happen only for inheritable constraints.

> However, it is still possible for a child to override/hide the parent's
> constraint as follows:

Hmm, I thought I'd checked for this when I was fooling with constraint
merging a few days ago.  I must have checked one of the other cases and
missed this path.  Thanks for catching it!

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


[HACKERS] Conflicting constraint being merged

2016-10-12 Thread Amit Langote
Currently, if child table has a non-inherited constraint and a constraint
with the same name is added to the parent, it will fail with an error as
illustrated below:

create table parent (a int);
CREATE TABLE
create table child (constraint check_a check (a > 0) no inherit) inherits
(parent);
CREATE TABLE
alter table parent add constraint check_a check (a > 0);
ERROR:  constraint "check_a" conflicts with non-inherited constraint on
relation "child"

Or if parent with a inheritable constraint is added to inheritance parents
of child with non-inherited constraint of the same name:

create table parent (a int, constraint check_a check (a > 0));
CREATE TABLE
create table child (a int, constraint check_a check (a > 0) no inherit)
CREATE TABLE
alter table child inherit parent;
ERROR:  constraint "check_a" conflicts with non-inherited constraint on
child table "child"

If we had allowed it to be merged, any children of child itself won't
inherit that constraint (because on child it's marked NO INHERIT), which
would not be good.


However, it is still possible for a child to override/hide the parent's
constraint as follows:

create table parent (a int, constraint check_a check (a > 0));
CREATE TABLE
create table child (constraint check_a check (a > 0) no inherit) inherits
(parent);
CREATE TABLE

Note that child's "no inherit" constraint check_a has been successfully
created.  If we create its child table, the same won't be inherited.

create table child_child (a int) inherits (child);
CREATE TABLE

At this point:

\d parent
Table "public.parent"
 Column |  Type   | Modifiers
+-+---
 a  | integer |
Check constraints:
"check_a" CHECK (a > 0)
Number of child tables: 1 (Use \d+ to list them.)

\d child
 Table "public.child"
 Column |  Type   | Modifiers
+-+---
 a  | integer |
Check constraints:
"check_a" CHECK (a > 0) NO INHERIT
Inherits: parent
Number of child tables: 1 (Use \d+ to list them.)

\d child_child
  Table "public.child_child"
 Column |  Type   | Modifiers
+-+---
 a  | integer |
Inherits: child

Inserting 0 into child_child is fine, whereas it's not fine in parent or
child.

insert into child_child values (0);
INSERT 0 1

Selecting from parent will now return some rows violating one of its valid
constraints viz. check (a > 0).

select * from parent;
 a
---
 0
(1 row)


I am afraid this is an oversight/bug.  Attached tries to fix the same - In
MergeWithExistingConstraint(), we check if the new "no inherit" constraint
being added to a relation matches one of its existing constraints that is
*inherited*.  So the following will now happen:

create table parent (a int, constraint check_a check (a > 0));
CREATE TABLE
create table child (constraint check_a check (a > 0) no inherit) inherits
(parent);
ERROR:  constraint "check_a" conflicts with inherited constraint on
relation "child"

Thoughts?

Thanks,
Amit
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index ea06a57..abdd872 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -2463,6 +2463,17 @@ MergeWithExistingConstraint(Relation rel, char *ccname, Node *expr,
 ccname, RelationGetRelationName(rel;
 
 			/*
+			 * Cannot merge a "no inherit" constraint with the inherited child
+			 * constraint.  That's because the inherited constraints must
+			 * propagate to the lower-level children.
+			 */
+			if (con->coninhcount > 0 && is_no_inherit)
+ereport(ERROR,
+		(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+		 errmsg("constraint \"%s\" conflicts with inherited constraint on relation \"%s\"",
+ccname, RelationGetRelationName(rel;
+
+			/*
 			 * If the child constraint is "not valid" then cannot merge with a
 			 * valid parent constraint
 			 */
diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out
index 9d374fe..df7cba6 100644
--- a/src/test/regress/expected/inherit.out
+++ b/src/test/regress/expected/inherit.out
@@ -645,6 +645,9 @@ Check constraints:
 "p2chk" CHECK (ff1 > 10)
 Inherits: p1
 
+-- Test that child does not override inheritable constraints of the parent
+create table c2 (constraint p2chk check (ff1 > 10) no inherit) inherits (p1);	--fails
+ERROR:  constraint "p2chk" conflicts with inherited constraint on relation "c2"
 drop table p1 cascade;
 NOTICE:  drop cascades to table c1
 -- Tests for casting between the rowtypes of parent and child
diff --git a/src/test/regress/sql/inherit.sql b/src/test/regress/sql/inherit.sql
index 6b1df75..f45aab1 100644
--- a/src/test/regress/sql/inherit.sql
+++ b/src/test/regress/sql/inherit.sql
@@ -157,6 +157,9 @@ create table c1 () inherits (p1);
 \d p1
 \d c1
 
+-- Test that child does not override inheritable constraints of the parent
+create table c2 (constraint p2chk check (ff1 > 10) no inherit) inherits (p1);	--fails
+
 drop table p1 cascade;
 
 -- Tests for casting between the rowtypes of