From 92da65c2359db0bb6947a283a7996273d22a014a Mon Sep 17 00:00:00 2001 From: amit Date: Wed, 9 Jan 2019 14:01:47 +0900 Subject: [PATCH v1 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 | 57 ++++++++++++++++++++++++++++--------- 3 files changed, 72 insertions(+), 36 deletions(-) diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c index c88e923327..60e4ff5594 100644 --- a/src/backend/catalog/pg_constraint.c +++ b/src/backend/catalog/pg_constraint.c @@ -478,8 +478,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; @@ -500,8 +498,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) @@ -702,9 +698,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)); @@ -1156,8 +1149,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) @@ -1166,8 +1159,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)); @@ -1179,25 +1170,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 d263903622..d8430f6fe5 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -972,9 +972,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 a3eb39ffdd..8e2fb9b189 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\"", @@ -14688,6 +14710,13 @@ ATExecDetachPartition(Relation rel, RangeVar *name) elog(ERROR, "cache lookup failed for constraint %u", fk->conoid); conform = (Form_pg_constraint) GETSTRUCT(contup); + /* No need to touch the partition's own foreign keys. */ + if (conform->conislocal) + { + ReleaseSysCache(contup); + continue; + } + ConstraintSetParentConstraint(fk->conoid, InvalidOid); /* -- 2.11.0