On 2019/01/18 7:54, Alvaro Herrera wrote:
> On 2019-Jan-09, Amit Langote wrote:
> 
>> 1. Foreign keys of partitions stop working correctly after being detached
>> from the parent table
> 
>> This happens because the action triggers defined on the PK relation (pk)
>> refers to p as the referencing relation.  On detaching p1 from p, p1's
>> data is no longer accessible to that trigger.
> 
> Ouch.
> 
>> To fix this problem, we need create action triggers on PK relation
>> that refer to p1 when it's detached (unless such triggers already
>> exist which might be true in some cases).  Attached patch 0001 shows
>> this approach.
> 
> Hmm, okay.  I'm not in love with the idea that such triggers might
> already exist -- seems unclean.  We should remove redundant action
> triggers when we attach a table as a partition, no?

OK, I agree.  I have updated the patch to make things work that way.  With
the patch:

create table pk (a int primary key);
create table p (a int references pk) partition by list (a);

-- this query shows the action triggers on the referenced rel ('pk'), name
-- of the constraint that the trigger is part of and the foreign key rel
-- ('p', etc.)

select tgrelid::regclass as pkrel, c.conname as fkconname,
tgconstrrelid::regclass as fkrel from pg_trigger t, pg_constraint c where
tgrelid = 'pk'::regclass and tgconstraint = c.oid;
 pkrel │ fkconname │ fkrel
───────┼───────────┼───────
 pk    │ p_a_fkey  │ p
 pk    │ p_a_fkey  │ p
(2 rows)

create table p1 (
   a int references pk,
   foreign key (a) references pk (a) ON UPDATE CASCADE ON DELETE CASCADE
DEFERRABLE,
   foreign key (a) references pk (a) MATCH FULL ON UPDATE CASCADE ON
DELETE CASCADE
) partition by list (a);

-- p1_a_fkey on 'p1' is equivalent to p_a_fkey on 'p', but they're not
-- attached yet

select tgrelid::regclass as pkrel, conname as fkconname,
tgconstrrelid::regclass as fkrel from pg_trigger t, pg_constraint c where
tgrelid = 'pk'::regclass and tgconstraint = c.oid;
 pkrel │ fkconname  │ fkrel
───────┼────────────┼───────
 pk    │ p_a_fkey   │ p
 pk    │ p_a_fkey   │ p
 pk    │ p1_a_fkey  │ p1
 pk    │ p1_a_fkey  │ p1
 pk    │ p1_a_fkey1 │ p1
 pk    │ p1_a_fkey1 │ p1
 pk    │ p1_a_fkey2 │ p1
 pk    │ p1_a_fkey2 │ p1
(8 rows)

create table p11 (like p1, foreign key (a) references pk);

-- again, p11_a_fkey, p1_a_fkey, and p_a_fkey are equivalent

select tgrelid::regclass as pkrel, conname as fkconname,
tgconstrrelid::regclass as fkrel from pg_trigger t, pg_constraint c where
tgrelid = 'pk'::regclass and tgconstraint = c.oid;
 pkrel │ fkconname  │ fkrel
───────┼────────────┼───────
 pk    │ p_a_fkey   │ p
 pk    │ p_a_fkey   │ p
 pk    │ p1_a_fkey  │ p1
 pk    │ p1_a_fkey  │ p1
 pk    │ p1_a_fkey1 │ p1
 pk    │ p1_a_fkey1 │ p1
 pk    │ p1_a_fkey2 │ p1
 pk    │ p1_a_fkey2 │ p1
 pk    │ p11_a_fkey │ p11
 pk    │ p11_a_fkey │ p11
(10 rows)


alter table p1 attach partition p11 for values in (1);

-- p1_a_fkey and p11_a_fkey merged, so triggers for the latter dropped

select tgrelid::regclass as pkrel, conname as fkconname,
tgconstrrelid::regclass as fkrel from pg_trigger t, pg_constraint c where
tgrelid = 'pk'::regclass and tgconstraint = c.oid;
 pkrel │ fkconname  │ fkrel
───────┼────────────┼───────
 pk    │ p_a_fkey   │ p
 pk    │ p_a_fkey   │ p
 pk    │ p1_a_fkey  │ p1
 pk    │ p1_a_fkey  │ p1
 pk    │ p1_a_fkey1 │ p1
 pk    │ p1_a_fkey1 │ p1
 pk    │ p1_a_fkey2 │ p1
 pk    │ p1_a_fkey2 │ p1
(8 rows)

-- p_a_fkey and p1_a_fkey merged, so triggers for the latter dropped

alter table p attach partition p1 for values in (1);

select tgrelid::regclass as pkrel, conname as fkconname,
tgconstrrelid::regclass as fkrel from pg_trigger t, pg_constraint c where
tgrelid = 'pk'::regclass and tgconstraint = c.oid;
 pkrel │ fkconname  │ fkrel
───────┼────────────┼───────
 pk    │ p_a_fkey   │ p
 pk    │ p_a_fkey   │ p
 pk    │ p1_a_fkey1 │ p1
 pk    │ p1_a_fkey1 │ p1
 pk    │ p1_a_fkey2 │ p1
 pk    │ p1_a_fkey2 │ p1
(6 rows)


alter table p detach partition p1;

-- p1_a_fkey needs its own triggers again

select tgrelid::regclass as pkrel, conname as fkconname,
tgconstrrelid::regclass as fkrel from pg_trigger t, pg_constraint c where
tgrelid = 'pk'::regclass and tgconstraint = c.oid;
 pkrel │ fkconname  │ fkrel
───────┼────────────┼───────
 pk    │ p_a_fkey   │ p
 pk    │ p_a_fkey   │ p
 pk    │ p1_a_fkey1 │ p1
 pk    │ p1_a_fkey1 │ p1
 pk    │ p1_a_fkey2 │ p1
 pk    │ p1_a_fkey2 │ p1
 pk    │ p1_a_fkey  │ p1
 pk    │ p1_a_fkey  │ p1
(8 rows)

alter table p1 detach partition p11;

-- p11_a_fkey needs its own triggers again

select tgrelid::regclass as pkrel, conname as fkconname,
tgconstrrelid::regclass as fkrel from pg_trigger t, pg_constraint c where
tgrelid = 'pk'::regclass and tgconstraint = c.oid;
 pkrel │ fkconname  │ fkrel
───────┼────────────┼───────
 pk    │ p_a_fkey   │ p
 pk    │ p_a_fkey   │ p
 pk    │ p1_a_fkey1 │ p1
 pk    │ p1_a_fkey1 │ p1
 pk    │ p1_a_fkey2 │ p1
 pk    │ p1_a_fkey2 │ p1
 pk    │ p1_a_fkey  │ p1
 pk    │ p1_a_fkey  │ p1
 pk    │ p11_a_fkey │ p11
 pk    │ p11_a_fkey │ p11
 pk    │ p1_a_fkey1 │ p11
 pk    │ p1_a_fkey1 │ p11
 pk    │ p1_a_fkey2 │ p11
 pk    │ p1_a_fkey2 │ p11
(14 rows)

-- try again

alter table p1 attach partition p11 for values in (1);

select tgrelid::regclass as pkrel, conname as fkconname,
tgconstrrelid::regclass as fkrel from pg_trigger t, pg_constraint c where
tgrelid = 'pk'::regclass and tgconstraint = c.oid;
 pkrel │ fkconname  │ fkrel
───────┼────────────┼───────
 pk    │ p_a_fkey   │ p
 pk    │ p_a_fkey   │ p
 pk    │ p1_a_fkey1 │ p1
 pk    │ p1_a_fkey1 │ p1
 pk    │ p1_a_fkey2 │ p1
 pk    │ p1_a_fkey2 │ p1
 pk    │ p1_a_fkey  │ p1
 pk    │ p1_a_fkey  │ p1
(8 rows)


alter table p attach partition p1 for values in (1);

select tgrelid::regclass as pkrel, conname as fkconname,
tgconstrrelid::regclass as fkrel from pg_trigger t, pg_constraint c where
tgrelid = 'pk'::regclass and tgconstraint = c.oid;
 pkrel │ fkconname  │ fkrel
───────┼────────────┼───────
 pk    │ p_a_fkey   │ p
 pk    │ p_a_fkey   │ p
 pk    │ p1_a_fkey1 │ p1
 pk    │ p1_a_fkey1 │ p1
 pk    │ p1_a_fkey2 │ p1
 pk    │ p1_a_fkey2 │ p1
(6 rows)


By the way, I also noticed that there's duplicated code in
clone_fk_constraints() which 0001 gets rid of:

        datum = fastgetattr(tuple, Anum_pg_constraint_conpfeqop,
                            tupdesc, &isnull);
        if (isnull)
            elog(ERROR, "null conpfeqop");
        arr = DatumGetArrayTypeP(datum);
        nelem = ARR_DIMS(arr)[0];
        if (ARR_NDIM(arr) != 1 ||
            nelem < 1 ||
            nelem > INDEX_MAX_KEYS ||
            ARR_HASNULL(arr) ||
            ARR_ELEMTYPE(arr) != OIDOID)
            elog(ERROR, "conpfeqop is not a 1-D OID array");
        memcpy(conpfeqop, ARR_DATA_PTR(arr), nelem * sizeof(Oid));

-        datum = fastgetattr(tuple, Anum_pg_constraint_conpfeqop,
-                            tupdesc, &isnull);
-        if (isnull)
-            elog(ERROR, "null conpfeqop");
-        arr = DatumGetArrayTypeP(datum);
-        nelem = ARR_DIMS(arr)[0];
-        if (ARR_NDIM(arr) != 1 ||
-            nelem < 1 ||
-            nelem > INDEX_MAX_KEYS ||
-            ARR_HASNULL(arr) ||
-            ARR_ELEMTYPE(arr) != OIDOID)
-            elog(ERROR, "conpfeqop is not a 1-D OID array");
-        memcpy(conpfeqop, ARR_DATA_PTR(arr), nelem * sizeof(Oid));
-

I know you're working on a bug fix in the thread on pgsql-bugs which is
related to the patch 0002 here, but attaching it here anyway, because it
proposes to get rid of the needless dependencies which I didn't see
mentioned on the other thread.  Also, updated 0001 needed it to be rebased.

Like the last time, I've also attached the patches that can be applied
PG11 branch.

Thanks,
Amit
From 9c877b0cb1b8dbe7f6957b5b1256686f3690dee5 Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@gmail.com>
Date: Fri, 18 Jan 2019 14:13:11 +0900
Subject: [PATCH v2 1/2] Ensure PK-side action triggers for partitions after
 being detached

Detaching a partition from the parent whose foreign key(s) would've
been duplicated in the partition makes the foreign key(s) stop
working corretly.  That's because PK-side action trigger for the
foreign key would refer to the parent as the referencing relation
and after the partition is detached it's data is no longer accessible
via parent.  So while the check triggers that remain even after
being detached takes care of the one side of enforcing the foreign
key of the detached partition, lack of corresponding PK-side action
trigger to check detached partition's data means the other side
doesn't work.

To fix, add the action triggers on the PK relation that point to
the detached partition when detaching.
---
 src/backend/catalog/pg_constraint.c | 74 +++++++++++++++++++++++++++++--------
 src/backend/commands/tablecmds.c    | 42 +++++++++++++++++++--
 src/include/commands/tablecmds.h    |  3 +-
 3 files changed, 98 insertions(+), 21 deletions(-)

diff --git a/src/backend/catalog/pg_constraint.c 
b/src/backend/catalog/pg_constraint.c
index f4057a9f15..9df6540ac3 100644
--- a/src/backend/catalog/pg_constraint.c
+++ b/src/backend/catalog/pg_constraint.c
@@ -26,6 +26,7 @@
 #include "catalog/partition.h"
 #include "catalog/pg_constraint.h"
 #include "catalog/pg_operator.h"
+#include "catalog/pg_trigger.h"
 #include "catalog/pg_type.h"
 #include "commands/defrem.h"
 #include "commands/tablecmds.h"
@@ -551,20 +552,6 @@ clone_fk_constraints(Relation pg_constraint, Relation 
parentRel,
                        elog(ERROR, "conpfeqop is not a 1-D OID array");
                memcpy(conpfeqop, ARR_DATA_PTR(arr), nelem * sizeof(Oid));
 
-               datum = fastgetattr(tuple, Anum_pg_constraint_conpfeqop,
-                                                       tupdesc, &isnull);
-               if (isnull)
-                       elog(ERROR, "null conpfeqop");
-               arr = DatumGetArrayTypeP(datum);
-               nelem = ARR_DIMS(arr)[0];
-               if (ARR_NDIM(arr) != 1 ||
-                       nelem < 1 ||
-                       nelem > INDEX_MAX_KEYS ||
-                       ARR_HASNULL(arr) ||
-                       ARR_ELEMTYPE(arr) != OIDOID)
-                       elog(ERROR, "conpfeqop is not a 1-D OID array");
-               memcpy(conpfeqop, ARR_DATA_PTR(arr), nelem * sizeof(Oid));
-
                datum = fastgetattr(tuple, Anum_pg_constraint_conppeqop,
                                                        tupdesc, &isnull);
                if (isnull)
@@ -607,6 +594,10 @@ clone_fk_constraints(Relation pg_constraint, Relation 
parentRel,
                        ForeignKeyCacheInfo *fk = 
lfirst_node(ForeignKeyCacheInfo, cell);
                        Form_pg_constraint partConstr;
                        HeapTuple       partcontup;
+                       Relation        trigrel;
+                       HeapTuple       trigtup;
+                       HeapScanDesc scan;
+                       ScanKeyData key[3];
 
                        attach_it = true;
 
@@ -658,7 +649,57 @@ clone_fk_constraints(Relation pg_constraint, Relation 
parentRel,
 
                        ReleaseSysCache(partcontup);
 
-                       /* looks good!  Attach this constraint */
+                       /*
+                        * looks good!  Attach this constraint.  Drop the 
action triggers
+                        * on the PK side that refer to this partition rel as 
part of
+                        * this constraint, because they will be wasteful after 
attaching
+                        * this constraint to the parent constraint.  After 
being attached,
+                        * action triggers corresponding to the parent 
constraint can
+                        * indirectly refer to this partition's data by 
referencing the
+                        * parent relation.
+                        */
+                       trigrel = heap_open(TriggerRelationId, AccessShareLock);
+                       ScanKeyInit(&key[0],
+                                               Anum_pg_trigger_tgrelid,
+                                               BTEqualStrategyNumber, F_OIDEQ,
+                                               
ObjectIdGetDatum(fk->confrelid));
+                       ScanKeyInit(&key[1],
+                                               Anum_pg_trigger_tgconstrrelid,
+                                               BTEqualStrategyNumber, F_OIDEQ,
+                                               ObjectIdGetDatum(fk->conrelid));
+                       ScanKeyInit(&key[2],
+                                               Anum_pg_trigger_tgconstraint,
+                                               BTEqualStrategyNumber, F_OIDEQ,
+                                               ObjectIdGetDatum(fk->conoid));
+                       scan = heap_beginscan_catalog(trigrel, 3, key);
+
+                       /*
+                        * There should be two tuples to be deleted 
corresponding to
+                        * the two action triggers that 
createForeignKeyActionTriggers
+                        * would have created.
+                        */
+                       trigtup = heap_getnext(scan, ForwardScanDirection);
+                       Assert(trigtup != NULL);
+                       deleteDependencyRecordsForClass(TriggerRelationId,
+                                                                               
        HeapTupleGetOid(trigtup),
+                                                                               
        ConstraintRelationId,
+                                                                               
        DEPENDENCY_INTERNAL);
+                       CatalogTupleDelete(trigrel, &trigtup->t_self);
+
+                       trigtup = heap_getnext(scan, ForwardScanDirection);
+                       Assert(trigtup != NULL);
+                       deleteDependencyRecordsForClass(TriggerRelationId,
+                                                                               
        HeapTupleGetOid(trigtup),
+                                                                               
        ConstraintRelationId,
+                                                                               
        DEPENDENCY_INTERNAL);
+                       CatalogTupleDelete(trigrel, &trigtup->t_self);
+
+                       trigtup = heap_getnext(scan, ForwardScanDirection);
+                       Assert(trigtup == NULL);
+
+                       heap_endscan(scan);
+                       heap_close(trigrel, AccessShareLock);
+
                        ConstraintSetParentConstraint(fk->conoid,
                                                                                
  HeapTupleGetOid(tuple));
                        CommandCounterIncrement();
@@ -720,7 +761,8 @@ clone_fk_constraints(Relation pg_constraint, Relation 
parentRel,
                fkconstraint->initdeferred = constrForm->condeferred;
 
                createForeignKeyTriggers(partRel, constrForm->confrelid, 
fkconstraint,
-                                                                constrOid, 
constrForm->conindid, false);
+                                                                constrOid, 
constrForm->conindid, false,
+                                                                true);
 
                if (cloned)
                {
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 71b2e3f134..d668a57ac2 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -7718,7 +7718,7 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo 
*tab, Relation rel,
         * though the constraints also exist below.
         */
        createForeignKeyTriggers(rel, RelationGetRelid(pkrel), fkconstraint,
-                                                        constrOid, indexOid, 
!recursing);
+                                                        constrOid, indexOid, 
!recursing, true);
 
        /*
         * Tell Phase 3 to check that the constraint is satisfied by existing
@@ -8827,7 +8827,8 @@ createForeignKeyCheckTriggers(Oid myRelOid, Oid refRelOid,
  */
 void
 createForeignKeyTriggers(Relation rel, Oid refRelOid, Constraint *fkconstraint,
-                                                Oid constraintOid, Oid 
indexOid, bool create_action)
+                                                Oid constraintOid, Oid 
indexOid, bool create_action,
+                                                bool create_check)
 {
        /*
         * For the referenced side, create action triggers, if requested.  (If 
the
@@ -8843,7 +8844,7 @@ createForeignKeyTriggers(Relation rel, Oid refRelOid, 
Constraint *fkconstraint,
         * For the referencing side, create the check triggers.  We only need
         * these on the partitions.
         */
-       if (rel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE)
+       if (rel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE && create_check)
                createForeignKeyCheckTriggers(RelationGetRelid(rel), refRelOid,
                                                                          
fkconstraint, constraintOid, indexOid);
 
@@ -14887,19 +14888,52 @@ ATExecDetachPartition(Relation rel, RangeVar *name)
        }
        heap_close(classRel, RowExclusiveLock);
 
-       /* Detach foreign keys */
+       /* Detach any foreign keys that are inherited */
        fks = copyObject(RelationGetFKeyList(partRel));
        foreach(cell, fks)
        {
                ForeignKeyCacheInfo *fk = lfirst(cell);
                HeapTuple       contup;
+               Form_pg_constraint conform;
+               Constraint *fkconstraint;
 
                contup = SearchSysCache1(CONSTROID, 
ObjectIdGetDatum(fk->conoid));
                if (!contup)
                        elog(ERROR, "cache lookup failed for constraint %u", 
fk->conoid);
+               conform = (Form_pg_constraint) GETSTRUCT(contup);
 
+               /* consider only the inherited foreign keys */
+               if (conform->contype != CONSTRAINT_FOREIGN ||
+                       !OidIsValid(conform->conparentid))
+               {
+                       ReleaseSysCache(contup);
+                       continue;
+               }
+
+               /* unset conparentid and adjust conislocal, coninhcount, etc. */
                ConstraintSetParentConstraint(fk->conoid, InvalidOid);
 
+               /*
+                * We'll need to make the action triggers on primary key 
relation that
+                * point to this relation as the FK relation.  We need to do 
this,
+                * because no PK-side triggers exist for an inherited FK 
constraint.
+                * Now that we've detached it from the parent, we'd our own.
+                */
+               fkconstraint = makeNode(Constraint);
+               fkconstraint->conname = pstrdup(NameStr(conform->conname));
+               fkconstraint->fk_upd_action = conform->confupdtype;
+               fkconstraint->fk_del_action = conform->confdeltype;
+               fkconstraint->deferrable = conform->condeferrable;
+               fkconstraint->initdeferred = conform->condeferred;
+
+               /*
+                * As we already got the check triggers, no need to recreate 
them,
+                * so pass false for create_check.
+                */
+               createForeignKeyTriggers(partRel, conform->confrelid, 
fkconstraint,
+                                                                fk->conoid, 
conform->conindid,
+                                                                true, false);
+
                ReleaseSysCache(contup);
        }
        list_free_deep(fks);
diff --git a/src/include/commands/tablecmds.h b/src/include/commands/tablecmds.h
index 138de84e83..dd985a80b6 100644
--- a/src/include/commands/tablecmds.h
+++ b/src/include/commands/tablecmds.h
@@ -77,7 +77,8 @@ extern void check_of_type(HeapTuple typetuple);
 
 extern void createForeignKeyTriggers(Relation rel, Oid refRelOid,
                                                 Constraint *fkconstraint, Oid 
constraintOid,
-                                                Oid indexOid, bool 
create_action);
+                                                Oid indexOid, bool 
create_action,
+                                                bool create_check);
 
 extern void register_on_commit_action(Oid relid, OnCommitAction action);
 extern void remove_on_commit_action(Oid relid);
-- 
2.11.0

From 61c03e3435eb10304361ca093be7687f56463b9f Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@gmail.com>
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

From d168bfa988bb8923fb138ceb4026ac005a7f580a Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@gmail.com>
Date: Wed, 9 Jan 2019 10:00:11 +0900
Subject: [PATCH v2 1/2] Ensure PK-side action triggers for partitions after
 being detached

Detaching a partition from the parent whose foreign key(s) would've
been duplicated in the partition makes the foreign key(s) stop
working corretly.  That's because PK-side action trigger for the
foreign key would refer to the parent as the referencing relation
and after the partition is detached it's data is no longer accessible
via parent.  So while the check triggers that remain even after
being detached takes care of the one side of enforcing the foreign
key of the detached partition, lack of corresponding PK-side action
trigger to check detached partition's data means the other side
doesn't work.

To fix, add the action triggers on the PK relation that point to
the detached partition when detaching.
---
 src/backend/catalog/pg_constraint.c | 74 +++++++++++++++++++++++++++++--------
 src/backend/commands/tablecmds.c    | 42 +++++++++++++++++++--
 src/include/commands/tablecmds.h    |  3 +-
 3 files changed, 98 insertions(+), 21 deletions(-)

diff --git a/src/backend/catalog/pg_constraint.c 
b/src/backend/catalog/pg_constraint.c
index 3c960c9423..3cac851447 100644
--- a/src/backend/catalog/pg_constraint.c
+++ b/src/backend/catalog/pg_constraint.c
@@ -27,6 +27,7 @@
 #include "catalog/partition.h"
 #include "catalog/pg_constraint.h"
 #include "catalog/pg_operator.h"
+#include "catalog/pg_trigger.h"
 #include "catalog/pg_type.h"
 #include "commands/defrem.h"
 #include "commands/tablecmds.h"
@@ -547,20 +548,6 @@ clone_fk_constraints(Relation pg_constraint, Relation 
parentRel,
                        elog(ERROR, "conpfeqop is not a 1-D OID array");
                memcpy(conpfeqop, ARR_DATA_PTR(arr), nelem * sizeof(Oid));
 
-               datum = fastgetattr(tuple, Anum_pg_constraint_conpfeqop,
-                                                       tupdesc, &isnull);
-               if (isnull)
-                       elog(ERROR, "null conpfeqop");
-               arr = DatumGetArrayTypeP(datum);
-               nelem = ARR_DIMS(arr)[0];
-               if (ARR_NDIM(arr) != 1 ||
-                       nelem < 1 ||
-                       nelem > INDEX_MAX_KEYS ||
-                       ARR_HASNULL(arr) ||
-                       ARR_ELEMTYPE(arr) != OIDOID)
-                       elog(ERROR, "conpfeqop is not a 1-D OID array");
-               memcpy(conpfeqop, ARR_DATA_PTR(arr), nelem * sizeof(Oid));
-
                datum = fastgetattr(tuple, Anum_pg_constraint_conppeqop,
                                                        tupdesc, &isnull);
                if (isnull)
@@ -603,6 +590,11 @@ clone_fk_constraints(Relation pg_constraint, Relation 
parentRel,
                        ForeignKeyCacheInfo *fk = 
lfirst_node(ForeignKeyCacheInfo, cell);
                        Form_pg_constraint partConstr;
                        HeapTuple       partcontup;
+                       Relation        trigrel;
+                       HeapTuple       trigtup;
+                       Form_pg_trigger trigform;
+                       HeapScanDesc scan;
+                       ScanKeyData key[3];
 
                        attach_it = true;
 
@@ -654,7 +646,56 @@ clone_fk_constraints(Relation pg_constraint, Relation 
parentRel,
 
                        ReleaseSysCache(partcontup);
 
-                       /* looks good!  Attach this constraint */
+                       /*
+                        * looks good!  Attach this constraint.  Drop the 
action triggers
+                        * on the PK side that refer to this partition rel as 
part of
+                        * this constraint, because they will be wasteful after 
attaching
+                        * this constraint to the parent constraint.  After 
being attached,
+                        * action triggers corresponding to the parent 
constraint can
+                        * indirectly refer to this partition's data by 
referencing the
+                        * parent relation.
+                        */
+                       trigrel = heap_open(TriggerRelationId, AccessShareLock);
+                       ScanKeyInit(&key[0],
+                                               Anum_pg_trigger_tgrelid,
+                                               BTEqualStrategyNumber, F_OIDEQ,
+                                               
ObjectIdGetDatum(fk->confrelid));
+                       ScanKeyInit(&key[1],
+                                               Anum_pg_trigger_tgconstrrelid,
+                                               BTEqualStrategyNumber, F_OIDEQ,
+                                               ObjectIdGetDatum(fk->conrelid));
+                       ScanKeyInit(&key[2],
+                                               Anum_pg_trigger_tgconstraint,
+                                               BTEqualStrategyNumber, F_OIDEQ,
+                                               ObjectIdGetDatum(fk->conoid));
+                       scan = heap_beginscan_catalog(trigrel, 3, key);
+
+                       /*
+                        * There should be two tuples to be deleted 
corresponding to
+                        * the two action triggers that 
createForeignKeyActionTriggers
+                        * would have created.
+                        */
+                       trigtup = heap_getnext(scan, ForwardScanDirection);
+                       Assert(trigtup != NULL);
+                       trigform = (Form_pg_trigger) GETSTRUCT(trigtup);
+                       deleteDependencyRecordsForClass(TriggerRelationId, 
trigform->oid,
+                                                                               
        ConstraintRelationId,
+                                                                               
        DEPENDENCY_INTERNAL);
+                       CatalogTupleDelete(trigrel, &trigtup->t_self);
+
+                       trigtup = heap_getnext(scan, ForwardScanDirection);
+                       Assert(trigtup != NULL);
+                       trigform = (Form_pg_trigger) GETSTRUCT(trigtup);
+                       deleteDependencyRecordsForClass(TriggerRelationId, 
trigform->oid,
+                                                                               
        ConstraintRelationId,
+                                                                               
        DEPENDENCY_INTERNAL);
+                       CatalogTupleDelete(trigrel, &trigtup->t_self);
+
+                       trigtup = heap_getnext(scan, ForwardScanDirection);
+                       Assert(trigtup == NULL);
+                       heap_endscan(scan);
+                       heap_close(trigrel, AccessShareLock);
+
                        ConstraintSetParentConstraint(fk->conoid, 
constrForm->oid);
                        CommandCounterIncrement();
                        attach_it = true;
@@ -714,7 +755,8 @@ clone_fk_constraints(Relation pg_constraint, Relation 
parentRel,
                fkconstraint->initdeferred = constrForm->condeferred;
 
                createForeignKeyTriggers(partRel, constrForm->confrelid, 
fkconstraint,
-                                                                constrOid, 
constrForm->conindid, false);
+                                                                constrOid, 
constrForm->conindid, false,
+                                                                true);
 
                if (cloned)
                {
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index d2781cbf19..80b46d3139 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -7623,7 +7623,7 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo 
*tab, Relation rel,
         * though the constraints also exist below.
         */
        createForeignKeyTriggers(rel, RelationGetRelid(pkrel), fkconstraint,
-                                                        constrOid, indexOid, 
!recursing);
+                                                        constrOid, indexOid, 
!recursing, true);
 
        /*
         * Tell Phase 3 to check that the constraint is satisfied by existing
@@ -8728,7 +8728,8 @@ createForeignKeyCheckTriggers(Oid myRelOid, Oid refRelOid,
  */
 void
 createForeignKeyTriggers(Relation rel, Oid refRelOid, Constraint *fkconstraint,
-                                                Oid constraintOid, Oid 
indexOid, bool create_action)
+                                                Oid constraintOid, Oid 
indexOid, bool create_action,
+                                                bool create_check)
 {
        /*
         * For the referenced side, create action triggers, if requested.  (If 
the
@@ -8744,7 +8745,7 @@ createForeignKeyTriggers(Relation rel, Oid refRelOid, 
Constraint *fkconstraint,
         * For the referencing side, create the check triggers.  We only need
         * these on the partitions.
         */
-       if (rel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE)
+       if (rel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE && create_check)
                createForeignKeyCheckTriggers(RelationGetRelid(rel), refRelOid,
                                                                          
fkconstraint, constraintOid, indexOid);
 
@@ -14749,19 +14750,52 @@ ATExecDetachPartition(Relation rel, RangeVar *name)
        }
        heap_close(classRel, RowExclusiveLock);
 
-       /* Detach foreign keys */
+       /* Detach any foreign keys that are inherited */
        fks = copyObject(RelationGetFKeyList(partRel));
        foreach(cell, fks)
        {
                ForeignKeyCacheInfo *fk = lfirst(cell);
                HeapTuple       contup;
+               Form_pg_constraint conform;
+               Constraint *fkconstraint;
 
                contup = SearchSysCache1(CONSTROID, 
ObjectIdGetDatum(fk->conoid));
                if (!contup)
                        elog(ERROR, "cache lookup failed for constraint %u", 
fk->conoid);
+               conform = (Form_pg_constraint) GETSTRUCT(contup);
 
+               /* consider only the inherited foreign keys */
+               if (conform->contype != CONSTRAINT_FOREIGN ||
+                       !OidIsValid(conform->conparentid))
+               {
+                       ReleaseSysCache(contup);
+                       continue;
+               }
+
+               /* unset conparentid and adjust conislocal, coninhcount, etc. */
                ConstraintSetParentConstraint(fk->conoid, InvalidOid);
 
+               /*
+                * We'll need to make the action triggers on primary key 
relation that
+                * point to this relation as the FK relation.  We need to do 
this,
+                * because no PK-side triggers exist for an inherited FK 
constraint.
+                * Now that we've detached it from the parent, we'd our own.
+                */
+               fkconstraint = makeNode(Constraint);
+               fkconstraint->conname = pstrdup(NameStr(conform->conname));
+               fkconstraint->fk_upd_action = conform->confupdtype;
+               fkconstraint->fk_del_action = conform->confdeltype;
+               fkconstraint->deferrable = conform->condeferrable;
+               fkconstraint->initdeferred = conform->condeferred;
+
+               /*
+                * As we already got the check triggers, no need to recreate 
them,
+                * so pass false for create_check.
+                */
+               createForeignKeyTriggers(partRel, conform->confrelid, 
fkconstraint,
+                                                                fk->conoid, 
conform->conindid,
+                                                                true, false);
+
                ReleaseSysCache(contup);
        }
        list_free_deep(fks);
diff --git a/src/include/commands/tablecmds.h b/src/include/commands/tablecmds.h
index ec3bb90b01..6bebc68425 100644
--- a/src/include/commands/tablecmds.h
+++ b/src/include/commands/tablecmds.h
@@ -78,7 +78,8 @@ extern void check_of_type(HeapTuple typetuple);
 
 extern void createForeignKeyTriggers(Relation rel, Oid refRelOid,
                                                 Constraint *fkconstraint, Oid 
constraintOid,
-                                                Oid indexOid, bool 
create_action);
+                                                Oid indexOid, bool 
create_action,
+                                                bool create_check);
 
 extern void register_on_commit_action(Oid relid, OnCommitAction action);
 extern void remove_on_commit_action(Oid relid);
-- 
2.11.0

From 3c2ec8fb45df5b807cb00fa9d2c742451372bda2 Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@gmail.com>
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

Reply via email to