This is a bug indeed. I tried your patch, but it ends up in a seg fault.

I also see this was raised in another thread [0].

It can be reproduced in a slightly simplified case, using only a
single level partition.

"""
CREATE TABLE bar(id int PRIMARY KEY) PARTITION BY RANGE(id);
CREATE TABLE bar_p0 PARTITION OF bar FOR VALUES FROM (0) TO (100);

CREATE TABLE foo(id int) PARTITION BY RANGE(id);
CREATE TABLE foo_p0 PARTITION OF foo FOR VALUES FROM (0) TO (100);

ALTER TABLE foo_p0 ADD CONSTRAINT child_fk_con FOREIGN KEY (id) REFERENCES bar;

ALTER TABLE foo DETACH PARTITION foo_p0 ;
"""

Here is what I I found.

In DetachPartitionFinalize, after the child is detached from
the parent, the FK's insert and update triggers are then
removed from pg_depend with TriggerSetParentTrigger

        /*
         * The constraint on this table must be marked no longer a child of
         * the parent's constraint, as do its check triggers.
         */
        ConstraintSetParentConstraint(fk->conoid, InvalidOid, InvalidOid);

        /*
         * Also, look up the partition's "check" triggers corresponding to the
         * constraint being detached and detach them from the parent triggers.
         */
        GetForeignKeyCheckTriggers(trigrel,
                                   fk->conoid, fk->confrelid, fk->conrelid,
                                   &insertTriggerOid, &updateTriggerOid);
        Assert(OidIsValid(insertTriggerOid));
        TriggerSetParentTrigger(trigrel, insertTriggerOid, InvalidOid,
                                RelationGetRelid(partRel));
        Assert(OidIsValid(updateTriggerOid));
        TriggerSetParentTrigger(trigrel, updateTriggerOid, InvalidOid,
                                RelationGetRelid(partRel));

Specifically, the dependency types DEPENDENCY_PARTITION_PRI and
DEPENDENCY_PARTITION_SEC are removed from pg_depend.

        deleteDependencyRecordsForClass(TriggerRelationId, childTrigId,
                                        TriggerRelationId,
                                        DEPENDENCY_PARTITION_PRI);
        deleteDependencyRecordsForClass(TriggerRelationId, childTrigId,
                                        RelationRelationId,
                                        DEPENDENCY_PARTITION_SEC);


In the repro case, an FK on a partition with a reference
to a partition parent table does not create a new dependency.

postgres=# SELECT count(*) total, deptype FROM pg_depend WHERE deptype
in ('P', 'S') group by deptype;
 total | deptype
-------+---------
     2 | P
     2 | S
(2 rows)

postgres=# ALTER TABLE foo_p0 ADD CONSTRAINT child_fk_con FOREIGN KEY
(id) REFERENCES bar;
ALTER TABLE
postgres=#
postgres=# SELECT count(*) total, deptype FROM pg_depend WHERE deptype
in ('P', 'S') group by deptype;
 total | deptype
-------+---------
     2 | P
     2 | S
(2 rows)

We also see that the FK riggers created are associated with the parent
constraint rather than the child constraint, i.e. tgconstraint = 17387

postgres=# ALTER TABLE foo_p0 ADD CONSTRAINT child_fk_con FOREIGN KEY
(id) REFERENCES bar;
ALTER TABLE
postgres=#
postgres=# SELECT oid, tgrelid::regclass relname, tgparentid, 'insert'
trigger_type, tgconstraint FROM pg_trigger WHERE tgfoid in (1644,
1645)
postgres-# and tgtype & (1 << 2) > 0
postgres-# union all
postgres-# SELECT oid, tgrelid::regclass relname, tgparentid, 'update'
trigger_type, tgconstraint FROM pg_trigger WHERE tgfoid in (1644,
1645)
postgres-# and tgtype & (1 << 4) > 0;
  oid  | relname | tgparentid | trigger_type | tgconstraint
-------+---------+------------+--------------+--------------
 17393 | foo_p0  |          0 | insert       |        17387
 17394 | foo_p0  |          0 | update       |        17387
(2 rows)

postgres=# ALTER TABLE foo DETACH PARTITION foo_p0 ;
ERROR:  could not find ON INSERT check triggers of foreign key constraint 17390
postgres=#
postgres=# select oid, conparentid, conrelid::regclass,
confrelid::regclass from pg_constraint where oid = 17390;
  oid  | conparentid | conrelid | confrelid
-------+-------------+----------+-----------
 17390 |       17387 | foo_p0   | bar_p0
(1 row)

postgres=# select oid, conparentid, conrelid::regclass,
confrelid::regclass from pg_constraint where oid = 17387;
  oid  | conparentid | conrelid | confrelid
-------+-------------+----------+-----------
 17387 |           0 | foo_p0   | bar
(1 row)


This is not the case when the constraint is created on
the parent table,
i.e. ALTER TABLE foo ADD CONSTRAINT child_fk_con FOREIGN KEY (id) REFERENCES bar

In this case we also see a dependency.

postgres=# SELECT count(*) total, deptype FROM pg_depend WHERE deptype
in ('P', 'S') group by deptype;
 total | deptype
-------+---------
     2 | P
     2 | S
(2 rows)

postgres=# ALTER TABLE foo ADD CONSTRAINT child_fk_con FOREIGN KEY
(id) REFERENCES bar;
ALTER TABLE
postgres=# SELECT count(*) total, deptype FROM pg_depend WHERE deptype
in ('P', 'S') group by deptype;
 total | deptype
-------+---------
     3 | P
     3 | S
(2 rows)

Also, the constraint relname in the parent constraint is that of the
parent table and the child constraint is that of the child table.

postgres=*# ALTER TABLE foo DETACH PARTITION foo_p0 ;
ALTER TABLE
postgres=*# ROLLBACK;
ROLLBACK
                                                      ^
postgres=#  select oid, conparentid, conrelid::regclass,
confrelid::regclass from pg_constraint where oid = 17455;
  oid  | conparentid | conrelid | confrelid
-------+-------------+----------+-----------
 17455 |       17447 | foo_p0   | bar
(1 row)

postgres=# select oid, conparentid, conrelid::regclass,
confrelid::regclass from pg_constraint where oid = 17447;
  oid  | conparentid | conrelid | confrelid
-------+-------------+----------+-----------
 17447 |           0 | foo      | bar
(1 row

If the relation on the parent and child constraint match, that
tells us we don't have inheritance.
So, I am thinking we should add another condition for checking
if a foreign key is inherited by checking if the parent constraint
relation is different from the child constraint relation.

I am attaching an unpolished patch ( we need test coverage as well ) that
implements  the above. All tests pass with this patch.

Regards,

Sami Imseih
Amazon Web Services (AWS)

[0] 
https://www.postgresql.org/message-id/flat/CAHewXNm5rtfQZNv2uWkiHZVJeicFFa4x7p0%3Dy-x2vAM0vorgNQ%40mail.gmail.com#2ffeca3b63339da32fab04516f066bf6

Attachment: Fix-DETACH-PARTITION-with-foreign-key-referencing.patch
Description: Binary data

Reply via email to