From 8eb5ce74e8656b517ad5a4e960b70de0ff3bedd5 Mon Sep 17 00:00:00 2001 From: amit Date: Thu, 24 Jan 2019 21:29:17 +0900 Subject: [PATCH 1/2] Fix ATAddForeignKeyConstraint to use correct value of connoinherit While at it, add some Asserts to ConstraintSetParentConstraint to assert the correct value of coninhcount. Many tests fail, but the next patch should take care of them. --- src/backend/catalog/pg_constraint.c | 11 +++++++++-- src/backend/commands/tablecmds.c | 5 ++++- src/test/regress/expected/foreign_key.out | 18 ++++++++++++++++-- src/test/regress/sql/foreign_key.sql | 15 ++++++++++++++- 4 files changed, 43 insertions(+), 6 deletions(-) diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c index 446b54b9ff..d2b8489b6c 100644 --- a/src/backend/catalog/pg_constraint.c +++ b/src/backend/catalog/pg_constraint.c @@ -785,6 +785,12 @@ ConstraintSetParentConstraint(Oid childConstrId, Oid parentConstrId) { constrForm->conislocal = false; constrForm->coninhcount++; + /* + * An inherited foreign key constraint can never have more than one + * parent, because inheriting foreign keys is only allowed for + * partitioning where multiple inheritance is disallowed. + */ + Assert(constrForm->coninhcount == 1); constrForm->conparentid = parentConstrId; CatalogTupleUpdate(constrRel, &tuple->t_self, newtup); @@ -797,8 +803,9 @@ ConstraintSetParentConstraint(Oid childConstrId, Oid parentConstrId) else { constrForm->coninhcount--; - if (constrForm->coninhcount <= 0) - constrForm->conislocal = true; + /* See the above comment. */ + Assert(constrForm->coninhcount == 0); + constrForm->conislocal = true; constrForm->conparentid = InvalidOid; deleteDependencyRecordsForClass(ConstraintRelationId, childConstrId, diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 738c178107..fea4d99735 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -7251,6 +7251,9 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, bool old_check_ok; ObjectAddress address; ListCell *old_pfeqop_item = list_head(fkconstraint->old_conpfeqop); + /* Partitioned table's foreign key must always be inheritable. */ + bool connoinherit = (rel->rd_rel->relkind != + RELKIND_PARTITIONED_TABLE); /* * Grab ShareRowExclusiveLock on the pk table, so that someone doesn't @@ -7616,7 +7619,7 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, NULL, true, /* islocal */ 0, /* inhcount */ - true, /* isnoinherit */ + connoinherit, /* isnoinherit */ false); /* is_internal */ ObjectAddressSet(address, ConstraintRelationId, constrOid); diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out index 76040a7601..b50bafef9e 100644 --- a/src/test/regress/expected/foreign_key.out +++ b/src/test/regress/expected/foreign_key.out @@ -1945,7 +1945,21 @@ DETAIL: Key (a)=(2) is not present in table "pkey". delete from fkpart1.pkey where a = 1; ERROR: update or delete on table "pkey" violates foreign key constraint "fk_part_a_fkey" on table "fk_part_1" DETAIL: Key (a)=(1) is still referenced from table "fk_part_1". +-- verify that attaching and detaching partitions manipulates the inheritance +-- properties of their FK constraints correctly +create schema fkpart2; +create table fkpart2.pkey (a int primary key); +create table fkpart2.fk_part (a int, constraint fkey foreign key (a) references fkpart2.pkey) partition by list (a); +create table fkpart2.fk_part_1 partition of fkpart2.fk_part for values in (1) partition by list (a); +create table fkpart2.fk_part_1_1 (a int, constraint my_fkey foreign key (a) references fkpart2.pkey); +alter table fkpart2.fk_part_1 attach partition fkpart2.fk_part_1_1 for values in (1); +alter table fkpart2.fk_part_1 drop constraint fkey; -- should fail +ERROR: cannot drop inherited constraint "fkey" of relation "fk_part_1" +alter table fkpart2.fk_part_1_1 drop constraint my_fkey; -- should fail +ERROR: cannot drop inherited constraint "my_fkey" of relation "fk_part_1_1" +alter table fkpart2.fk_part detach partition fkpart2.fk_part_1; +alter table fkpart2.fk_part_1 drop constraint fkey; \set VERBOSITY terse \\ -- suppress cascade details -drop schema fkpart0, fkpart1 cascade; -NOTICE: drop cascades to 5 other objects +drop schema fkpart0, fkpart1, fkpart2 cascade; +NOTICE: drop cascades to 8 other objects \set VERBOSITY default diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql index 9ed1166c66..c02251eaa8 100644 --- a/src/test/regress/sql/foreign_key.sql +++ b/src/test/regress/sql/foreign_key.sql @@ -1392,6 +1392,19 @@ create table fkpart1.fk_part_1_2 partition of fkpart1.fk_part_1 for values in (2 insert into fkpart1.fk_part_1 values (2); -- should fail delete from fkpart1.pkey where a = 1; +-- verify that attaching and detaching partitions manipulates the inheritance +-- properties of their FK constraints correctly +create schema fkpart2; +create table fkpart2.pkey (a int primary key); +create table fkpart2.fk_part (a int, constraint fkey foreign key (a) references fkpart2.pkey) partition by list (a); +create table fkpart2.fk_part_1 partition of fkpart2.fk_part for values in (1) partition by list (a); +create table fkpart2.fk_part_1_1 (a int, constraint my_fkey foreign key (a) references fkpart2.pkey); +alter table fkpart2.fk_part_1 attach partition fkpart2.fk_part_1_1 for values in (1); +alter table fkpart2.fk_part_1 drop constraint fkey; -- should fail +alter table fkpart2.fk_part_1_1 drop constraint my_fkey; -- should fail +alter table fkpart2.fk_part detach partition fkpart2.fk_part_1; +alter table fkpart2.fk_part_1 drop constraint fkey; + \set VERBOSITY terse \\ -- suppress cascade details -drop schema fkpart0, fkpart1 cascade; +drop schema fkpart0, fkpart1, fkpart2 cascade; \set VERBOSITY default -- 2.11.0