Re: partitioned tables referenced by FKs - Mailing list pgsql-hackers

Hi Alvaro,

On 2019/03/22 6:54, Alvaro Herrera wrote:
> Here's v7;

Needs rebasing on top of 940311e4bb3.

0001:

+        Oid            objectClass = getObjectClass(thisobj);

I guess you meant to use ObjectClass, not Oid here.

Tested 0002 a bit more and found some problems.

create table p (a int primary key) 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);

create table q (a int) partition by list (a);
create table q1 partition of q for values in (1) partition by list (a);
create table q11 partition of q1 for values in (1);

alter table q add foreign key (a) references p;

create table p2 partition of p for values in (2);
ERROR:  index for 0 not found in partition p2

The problem appears to be that CloneFkReferenced() is stumbling across
inconsistent pg_constraint rows previously created by
addFkRecurseReferencing() when we did alter table q add foreign key:

select oid, conname, conrelid::regclass, confrelid::regclass, conindid,
conparentid from pg_constraint where conname like '%fkey%';
  oid  │  conname  │ conrelid │ confrelid │ conindid │ conparentid
───────┼───────────┼──────────┼───────────┼──────────┼─────────────
 60336 │ q_a_fkey  │ q        │ p         │    60315 │           0
 60337 │ q_a_fkey1 │ q        │ p1        │    60320 │       60336
 60338 │ q_a_fkey2 │ q        │ p11       │    60325 │       60337
 60341 │ q_a_fkey  │ q1       │ p         │        0 │       60336 <=
 60342 │ q_a_fkey  │ q11      │ p         │        0 │       60341 <=
(5 rows)

I think the last two rows contain invalid value of conindid, perhaps as a
result of a bug of addFkRecurseReferencing().  There's this code in it to
fetch the index partition of the referencing partition:

+            /*
+             * No luck finding a good constraint to reuse; create our own.
+             */
+            partIndexOid = index_get_partition(partition, indexOid);

That seems unnecessary.  Maybe, it's a remnant of the code copied from
addFkRecurseReferenced(), where using the partition index is necessary
(although without the elog for when we get an InvalidOid, which would've
caught this.)  The above index_get_partition() returns InvalidOid,
because, obviously, we don't require referencing side to have the index.
Attached a fix (addFkRecurseReferencing-fix.patch).

Once we apply the above fix, we run into another problem:

create table p2 partition of p for values in (2);

select oid, conname, conrelid::regclass, confrelid::regclass, conindid,
conparentid from pg_constraint where conname like '%fkey%';
  oid  │  conname   │ conrelid │ confrelid │ conindid │ conparentid
───────┼────────────┼──────────┼───────────┼──────────┼─────────────
 60336 │ q_a_fkey   │ q        │ p         │    60315 │           0
 60337 │ q_a_fkey1  │ q        │ p1        │    60320 │       60336
 60338 │ q_a_fkey2  │ q        │ p11       │    60325 │       60337
 60341 │ q_a_fkey   │ q1       │ p         │    60315 │       60336
 60342 │ q_a_fkey   │ q11      │ p         │    60315 │       60341
 60350 │ q_a_fkey3  │ q        │ p2        │    60348 │       60336 <=
 60353 │ q11_a_fkey │ q11      │ p2        │    60348 │       60342 <=
(7 rows)

There are 2 pg_constraint rows both targeting p2, whereas there should've
been only 1 (that is, one for q referencing p2).  However, if one adds the
foreign constraint on q *after* creating p2, there is only 1 row, which is
correct.

alter table q drop constraint q_a_fkey;
alter table q add foreign key (a) references p;

select oid, conname, conrelid::regclass, confrelid::regclass, conindid,
conparentid from pg_constraint where conname like '%fkey%';
  oid  │  conname  │ conrelid │ confrelid │ conindid │ conparentid
───────┼───────────┼──────────┼───────────┼──────────┼─────────────
 60356 │ q_a_fkey  │ q        │ p         │    60315 │           0
 60357 │ q_a_fkey1 │ q        │ p1        │    60320 │       60356
 60358 │ q_a_fkey2 │ q        │ p11       │    60325 │       60357
 60361 │ q_a_fkey3 │ q        │ p2        │    60348 │       60356 <=
 60364 │ q_a_fkey  │ q1       │ p         │    60315 │       60356
 60365 │ q_a_fkey  │ q11      │ p         │    60315 │       60364
(6 rows)

This appears to be a bug of CloneFkReferenced().  Specifically, the way it
builds the list of foreign key constraint to be cloned.  Attached a fix
(CloneFkReferenced-fix.patch).

> As I said before, I'm thinking of getting rid of the whole business of
> checking partitions on the referenced side of an FK at DROP time, and
> instead jut forbid the DROP completely if any FKs reference an ancestor
> of that partition.

Will that allow `DROP TABLE parted_pk CASCADE` to succeed even if
partitions still contain referenced data?  I suppose that's the example
you cited upthread as a bug that remains to be solved.

Thanks,
Amit