From 3c2ec8fb45df5b807cb00fa9d2c742451372bda2 Mon Sep 17 00:00:00 2001 From: amit Date: Wed, 9 Jan 2019 14:01:47 +0900 Subject: [PATCH v2 2/2] 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 inheritance information correctly. --- src/backend/catalog/pg_constraint.c | 33 ++++++++---------------- src/backend/commands/indexcmds.c | 18 +++++++++++++ src/backend/commands/tablecmds.c | 50 ++++++++++++++++++++++++++----------- 3 files changed, 65 insertions(+), 36 deletions(-) diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c index 3cac851447..3571416c3d 100644 --- a/src/backend/catalog/pg_constraint.c +++ b/src/backend/catalog/pg_constraint.c @@ -479,8 +479,6 @@ clone_fk_constraints(Relation pg_constraint, Relation parentRel, Constraint *fkconstraint; bool attach_it; Oid constrOid; - ObjectAddress parentAddr, - childAddr; int nelem; ListCell *cell; int i; @@ -501,8 +499,6 @@ clone_fk_constraints(Relation pg_constraint, Relation parentRel, continue; } - ObjectAddressSet(parentAddr, ConstraintRelationId, parentConstrOid); - datum = fastgetattr(tuple, Anum_pg_constraint_conkey, tupdesc, &isnull); if (isnull) @@ -743,9 +739,6 @@ clone_fk_constraints(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)); @@ -1197,8 +1190,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) @@ -1207,8 +1200,6 @@ ConstraintSetParentConstraint(Oid childConstrId, Oid parentConstrId) Form_pg_constraint constrForm; HeapTuple tuple, newtup; - ObjectAddress depender; - ObjectAddress referenced; constrRel = heap_open(ConstraintRelationId, RowExclusiveLock); tuple = SearchSysCache1(CONSTROID, ObjectIdGetDatum(childConstrId)); @@ -1220,25 +1211,23 @@ 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 1959e8a82e..91e0b968ac 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -973,9 +973,27 @@ DefineIndex(Oid relationId, /* Attach index to parent and we're done. */ IndexSetParentIndex(cldidx, indexRelationId); if (createdConstraintId != InvalidOid) + { + 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 80b46d3139..96aef0699c 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -7248,6 +7248,9 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, bool old_check_ok; ObjectAddress address; ListCell *old_pfeqop_item = list_head(fkconstraint->old_conpfeqop); + bool conislocal = true; + int coninhcount = 0; + bool connoinherit = true; /* * Grab ShareRowExclusiveLock on the pk table, so that someone doesn't @@ -7586,6 +7589,26 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, /* * Record the FK constraint in pg_constraint. */ + + /* + * Set the inheritance related properties correctly if it's being + * recursively added for a partition. + */ + if (OidIsValid(parentConstr)) + { + /* Foreign keys are inherited only for partitioning. */ + Assert(rel->rd_rel->relispartition); + conislocal = false; + /* Partitions can have only one parent. */ + coninhcount = 1; + /* Make sure that the constraint can be further inherited. */ + connoinherit = false; + } + + /* If partitioned table, constraint must be marked as inheritable. */ + if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) + connoinherit = false; + constrOid = CreateConstraintEntry(fkconstraint->conname, RelationGetNamespace(rel), CONSTRAINT_FOREIGN, @@ -7611,9 +7634,9 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, NULL, /* no exclusion constraint */ NULL, /* no check constraint */ NULL, - true, /* islocal */ - 0, /* inhcount */ - true, /* isnoinherit */ + conislocal, + coninhcount, + connoinherit, false); /* is_internal */ ObjectAddressSet(address, ConstraintRelationId, constrOid); @@ -7662,20 +7685,15 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, Oid partitionId = partdesc->oids[i]; Relation partition = heap_open(partitionId, lockmode); AlteredTableInfo *childtab; - ObjectAddress childAddr; CheckTableNotInUse(partition, "ALTER TABLE"); /* Find or create work queue entry for this table */ childtab = ATGetQueueEntry(wqueue, partition); - childAddr = - ATAddForeignKeyConstraint(wqueue, childtab, partition, - fkconstraint, constrOid, - recurse, true, lockmode); - - /* Record this constraint as dependent on the parent one */ - recordDependencyOn(&childAddr, &address, DEPENDENCY_INTERNAL_AUTO); + ATAddForeignKeyConstraint(wqueue, childtab, partition, + fkconstraint, constrOid, + recurse, true, lockmode); heap_close(partition, NoLock); } @@ -8925,9 +8943,13 @@ 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"); + /* + * Right now 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 */ elog(ERROR, "relation %u has non-inherited constraint \"%s\"", -- 2.11.0