Re: problems with foreign keys on partitioned tables

Hi,

On 2019/01/24 6:13, Alvaro Herrera wrote:
> On 2019-Jan-22, Amit Langote wrote:
>> Done. See the attached patches for HEAD and PG 11.
>
> I'm not quite sure I understand why the one in DefineIndex needs the
> deps but nothing else does. I fear that you added that one just to
> appease the existing test that breaks otherwise, and I worry that with
> that addition we're papering over some other, more fundamental bug.

Thinking more on this, my proposal to rip dependencies between parent and
child constraints altogether to resolve the bug I initially reported is
starting to sound a bit overambitious especially considering that we'd
need to back-patch it (the patch didn't even consider index constraints
properly, creating a divergence between the behaviors of inherited foreign
key constraints and inherited index constraints). We can pursue it if
only to avoid bloating the catalog for what can be achieved with little
bit of additional code in tablecmds.c, but maybe we should refrain from
doing it in reaction to this particular bug.

I've updated the patch that implements a much simpler fix for this
particular bug. Just to reiterate, the following illustrates the bug:

create table pk (a int primary key);
create table p (a int references pk) partition by list (a);
create table p1 partition of p for values in (1) partition by list (a);
create table p11 partition of p1 for values in (1);
alter table p detach partition p1;
alter table p1 drop constraint p_a_fkey;
ERROR: constraint "p_a_fkey" of relation "p11" does not exist

The error occurs because ATExecDropConstraint when recursively called on
p11 cannot find the constraint as the dependency mechanism already dropped
it. The new fix is to return from ATExecDropConstraint without recursing
if the constraint being dropped is index or foreign constraint.

A few hunks of the originally proposed patch are attached here as 0001,
especially the part which fixes ATAddForeignKeyConstraint to pass the
correct value of connoninherit to CreateConstraintEntry (which should be
false for partitioned tables). With that change, many tests start failing
because of the above bug. That patch also adds a test case like the one
above, but it fails along with others due to the bug. Patch 0002 is the
aforementioned simpler fix to make the errors (existing and the newly
added) go away.

These patches apply unchanged to the PG 11 branch.

Thanks,
Amit