From 61c03e3435eb10304361ca093be7687f56463b9f Mon Sep 17 00:00:00 2001 From: amit Date: Fri, 18 Jan 2019 14:20:13 +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 | 17 +++++++++++++ src/backend/commands/tablecmds.c | 50 ++++++++++++++++++++++++++----------- 3 files changed, 64 insertions(+), 36 deletions(-) diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c index 9df6540ac3..0048b3a436 100644 --- a/src/backend/catalog/pg_constraint.c +++ b/src/backend/catalog/pg_constraint.c @@ -483,8 +483,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; @@ -505,8 +503,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) @@ -749,9 +745,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)); @@ -1203,8 +1196,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) @@ -1213,8 +1206,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)); @@ -1226,25 +1217,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 fec5bc5dd6..1b497b5bd8 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -974,8 +974,25 @@ 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 (!IndexIsValid(cldidx->rd_index)) invalidate_parent = true; diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index d668a57ac2..d045b4eddb 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -7342,6 +7342,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 @@ -7680,6 +7683,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, @@ -7706,9 +7729,9 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, NULL, /* no check constraint */ NULL, NULL, - true, /* islocal */ - 0, /* inhcount */ - true, /* isnoinherit */ + conislocal, + coninhcount, + connoinherit, false); /* is_internal */ ObjectAddressSet(address, ConstraintRelationId, constrOid); @@ -7757,20 +7780,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); } @@ -9024,9 +9042,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