From 432c4551990d0da1c77b6b9523296b0a2a0a5119 Mon Sep 17 00:00:00 2001 From: amit Date: Tue, 22 Jan 2019 13:20:54 +0900 Subject: [PATCH] Do not track foreign key inheritance by dependencies Inheritance information maintained in pg_constraint is enough to prevent a child constraint to be dropped and for them to be dropped when the parent constraint is dropped. So, do not create dependencies between the parent foreign key constraint and its children. Also, fix ATAddForeignKeyConstraint to set connoniherit on the parent constraint correctly. --- src/backend/catalog/pg_constraint.c | 27 +++++++++++++-------------- src/backend/commands/indexcmds.c | 22 ++++++++++++++++++++-- src/backend/commands/tablecmds.c | 24 ++++++++++++------------ src/test/regress/expected/foreign_key.out | 17 +++++++++++++++-- src/test/regress/sql/foreign_key.sql | 14 +++++++++++++- 5 files changed, 73 insertions(+), 31 deletions(-) diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c index 446b54b9ff..855d57c65a 100644 --- a/src/backend/catalog/pg_constraint.c +++ b/src/backend/catalog/pg_constraint.c @@ -762,8 +762,8 @@ AlterConstraintNamespaces(Oid ownerId, Oid oldNspId, * ConstraintSetParentConstraint * Set a partition's constraint as child of its parent table's * - * This updates the constraint's pg_constraint row to show it as inherited, and - * add a dependency to the parent so that it cannot be removed on its own. + * This updates the constraint's pg_constraint row to change its inheritance + * properties. */ void ConstraintSetParentConstraint(Oid childConstrId, Oid parentConstrId) @@ -772,8 +772,6 @@ ConstraintSetParentConstraint(Oid childConstrId, Oid parentConstrId) Form_pg_constraint constrForm; HeapTuple tuple, newtup; - ObjectAddress depender; - ObjectAddress referenced; constrRel = table_open(ConstraintRelationId, RowExclusiveLock); tuple = SearchSysCache1(CONSTROID, ObjectIdGetDatum(childConstrId)); @@ -785,25 +783,26 @@ 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); - - ObjectAddressSet(referenced, ConstraintRelationId, parentConstrId); - ObjectAddressSet(depender, ConstraintRelationId, childConstrId); - - recordDependencyOn(&depender, &referenced, DEPENDENCY_INTERNAL_AUTO); } 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, - ConstraintRelationId, - DEPENDENCY_INTERNAL_AUTO); CatalogTupleUpdate(constrRel, &tuple->t_self, newtup); } diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 3edc94308e..6c8aa5d149 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -972,8 +972,26 @@ DefineIndex(Oid relationId, /* Attach index to parent and we're done. */ IndexSetParentIndex(cldidx, indexRelationId); if (createdConstraintId != InvalidOid) - ConstraintSetParentConstraint(cldConstrOid, - createdConstraintId); + { + ObjectAddress depender; + ObjectAddress referenced; + + ConstraintSetParentConstraint(cldConstrOid, + createdConstraintId); + + /* + * Need to set an explicit dependency in this + * case unlike other types of constraints where + * the child constraint gets dropped due to + * inheritance recursion. + */ + ObjectAddressSet(referenced, ConstraintRelationId, + createdConstraintId); + ObjectAddressSet(depender, ConstraintRelationId, + cldConstrOid); + recordDependencyOn(&depender, &referenced, + DEPENDENCY_INTERNAL_AUTO); + } if (!cldidx->rd_index->indisvalid) invalidate_parent = true; diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index fbd2d101c1..0040ac63d2 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, false); /* is_internal */ ObjectAddressSet(address, ConstraintRelationId, constrOid); @@ -7812,8 +7815,6 @@ CloneFkReferencing(Relation pg_constraint, Relation parentRel, Constraint *fkconstraint; bool attach_it; Oid constrOid; - ObjectAddress parentAddr, - childAddr; ListCell *cell; int i; @@ -7830,8 +7831,6 @@ CloneFkReferencing(Relation pg_constraint, Relation parentRel, continue; } - ObjectAddressSet(parentAddr, ConstraintRelationId, parentConstrOid); - DeconstructFkConstraintRow(tuple, &numfks, conkey, confkey, conpfeqop, conppeqop, conffeqop); for (i = 0; i < numfks; i++) @@ -7986,9 +7985,6 @@ CloneFkReferencing(Relation pg_constraint, Relation parentRel, 1, false, true); subclone = lappend_oid(subclone, constrOid); - ObjectAddressSet(childAddr, ConstraintRelationId, constrOid); - recordDependencyOn(&childAddr, &parentAddr, DEPENDENCY_INTERNAL_AUTO); - fkconstraint = makeNode(Constraint); /* for now this is all we need */ fkconstraint->conname = pstrdup(NameStr(constrForm->conname)); @@ -9284,11 +9280,15 @@ ATExecDropConstraint(Relation rel, const char *constrName, con = (Form_pg_constraint) GETSTRUCT(copy_tuple); - /* Right now only CHECK constraints can be inherited */ - if (con->contype != CONSTRAINT_CHECK) - elog(ERROR, "inherited constraint is not a CHECK constraint"); + /* + * Only CHECK constraints and foreign key constraints can be + * inherited. + */ + if (con->contype != CONSTRAINT_CHECK && + con->contype != CONSTRAINT_FOREIGN) + elog(ERROR, "inherited constraint is not a CHECK constraint or a foreign key constraint"); - if (con->coninhcount <= 0) /* shouldn't happen */ + if (con->coninhcount == 0) /* shouldn't happen */ elog(ERROR, "relation %u has non-inherited constraint \"%s\"", childrelid, constrName); diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out index 76040a7601..ba83c9d9bb 100644 --- a/src/test/regress/expected/foreign_key.out +++ b/src/test/regress/expected/foreign_key.out @@ -1945,7 +1945,20 @@ 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 p_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 partition of fkpart2.fk_part_1 for values in (1); +alter table fkpart2.fk_part_1 drop constraint p_fkey; -- should fail +ERROR: cannot drop inherited constraint "p_fkey" of relation "fk_part_1" +alter table fkpart2.fk_part_1_1 drop constraint p_fkey; -- should fail +ERROR: cannot drop inherited constraint "p_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 p_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..966162d045 100644 --- a/src/test/regress/sql/foreign_key.sql +++ b/src/test/regress/sql/foreign_key.sql @@ -1392,6 +1392,18 @@ 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 p_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 partition of fkpart2.fk_part_1 for values in (1); +alter table fkpart2.fk_part_1 drop constraint p_fkey; -- should fail +alter table fkpart2.fk_part_1_1 drop constraint p_fkey; -- should fail +alter table fkpart2.fk_part detach partition fkpart2.fk_part_1; +alter table fkpart2.fk_part_1 drop constraint p_fkey; + \set VERBOSITY terse \\ -- suppress cascade details -drop schema fkpart0, fkpart1 cascade; +drop schema fkpart0, fkpart1, fkpart2 cascade; \set VERBOSITY default -- 2.11.0