Subject: Re: [BUG] Fix DETACH with FK pointing to a partitioned table fails

From: Alvaro Herrera <[email-masked]>
Date: Wed, 6 Nov 2024 16:38:22 +0100

On 2024-Nov-05, Tom Lane wrote:

> I'm trying to write release notes for commits 53af9491a et al,
> and it seems to me that we need to explain how to get out of
> the mess that would be left behind by the old DETACH code.
> There's no hint about that in the commit message :-(

> Clearly, if you have now-inconsistent data, there's little
> help for that but to manually fix the inconsistencies.
> What I am worried about is how to get to a state where you
> have correct catalog entries for the constraint.
> 
> Will ALTER TABLE DROP CONSTRAINT on the now stand-alone table
> work to clean out the old catalog entries for the constraint?

Yes -- as far as I can tell, a DROP CONSTRAINT of the offending
constraint is successful and leaves no unwanted detritus.

Perhaps one more task for me is to figure out a way to get a list of all
the constraints that are broken because of this ... let me see if I can
figure that out.

> I'm worried that it will either fail, or go through but remove
> triggers on the referenced table that we still need for the
> original partitioned table.  If that doesn't work I think we had
> better create a recipe for manually removing the detritus.

As as far as I can see, it works and no triggers are spuriously removed.

> Once the old entries are gone it should be possible to do ALTER TABLE
> ADD CONSTRAINT (with an updated server), and that would validate
> your data.  It's the DROP CONSTRAINT part that worries me.

Yeah, that's correct: adding the constraint again after removing its
broken self detects that there are values violating the RI.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
Thou shalt check the array bounds of all strings (indeed, all arrays), for
surely where thou typest "foo" someone someday shall type
"supercalifragilisticexpialidocious" (5th Commandment for C programmers)

---

From: Alvaro Herrera <[email-masked]>
Date: Wed, 6 Nov 2024 18:53:07 +0100

> the constraints that are broken because of this ... let me see if I can

It's gotta be something like this,

SELECT conrelid::regclass AS "constrained table",
       conname as constraint, confrelid::regclass AS "references"
FROM pg_constraint
WHERE contype = 'f' and conparentid = 0 AND
   (SELECT count(*) FROM pg_constraint p2 WHERE conparentid = pg_constraint.oid) <>
   (SELECT count(*)
      FROM pg_inherits
     WHERE inhparent = pg_constraint.conrelid OR inhparent = pg_constraint.confrelid);

Essentially, top-level constraints should have as many children
constraint as direct partitions each partitioned table has.  Ideally
here you'd get an empty set, but you won't if the DETACH problem has
occurred.  I'll test this further later or maybe tomorrow as time
allows.

A quick test rig for this is:

create table pk (a int primary key) partition by list (a);
create table pk1 partition of pk for values in (1);
create table pk2367 partition of pk for values in (2, 3, 6, 7) partition by list (a);
create table pk67 partition of pk2367 for values in (6, 7) partition by list (a);
create table pk2 partition of pk2367 for values in (2);
create table pk3 partition of pk2367 for values in (3);
create table pk6 partition of pk67 for values in (6);
create table pk7 partition of pk67 for values in (7);
create table pk45 partition of pk for values in (4, 5) partition by list (a);
create table pk4 partition of pk45 for values in (4);
create table pk5 partition of pk45 for values in (5);

create table fk (a int references pk) partition by list (a);
create table fk1 partition of fk for values in (1);
create table fk2367 partition of fk for values in (2, 3, 6, 7) partition by list (a);
create table fk67 partition of fk2367 for values in (6, 7) partition by list (a);
create table fk2 partition of fk2367 for values in (2);
create table fk3 partition of fk2367 for values in (3);
create table fk6 partition of fk67 for values in (6);
create table fk7 partition of fk67 for values in (7);
create table fk45 partition of fk for values in (4, 5) partition by list (a);
create table fk4 partition of fk45 for values in (4);
create table fk5 partition of fk45 for values in (5);

alter table fk detach partition fk2367;

Before the fix, you get

 constrained table │ constraint │ references 
───────────────────┼────────────┼────────────
 fk2367            │ fk_a_fkey  │ pk
(1 fila)

which means you need to
ALTER TABLE fk2367 DROP CONSTRAINT fk_a_fkey;

and then put it back.  Maybe it'd be better to have the query emit the
commands to drop and reconstruct the FK?

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/

---

From: Tom Lane <[email-masked]>
Date: Fri, 08 Nov 2024 10:42:59 -0500

>      WHERE inhparent = pg_constraint.conrelid OR inhparent = pg_constraint.confrelid);

Hmm ... interestingly, if I run this in HEAD's regression database,
I get

 constrained table |  constraint   | references  
-------------------+---------------+-------------
 clstr_tst         | clstr_tst_con | clstr_tst_s
(1 row)

Digging a bit deeper, the sub-select for conparentid finds no rows,
but the sub-select on pg_inherits finds

regression=# SELECT inhrelid::regclass, inhparent::regclass, inhseqno,inhdetachpending from pg_inherits WHERE inhparent = 'clstr_tst'::regclass or inhparent = 'clstr_tst_s'::regclass;
   inhrelid    | inhparent | inhseqno | inhdetachpending 
---------------+-----------+----------+------------------
 clstr_tst_inh | clstr_tst |        1 | f
(1 row)

So it looks like this query needs a guard to make it ignore
constraints on traditional-inheritance tables.

			regards, tom lane

---

From: Alvaro Herrera <[email-masked]>
Date: Sun, 10 Nov 2024 14:28:40 +0100

>  constrained table |  constraint   | references  

Eeek.

> constraints on traditional-inheritance tables.

Hmm, looks tricky, the only thing I found was to only consider rows in
pg_inherit if there's a corresponding one in pg_partitioned_table.  This
should do it.  I added the DROP/ADD commands.  I also added some
pg_catalog schema quals, though that may be kinda useless.  Anyway, this
reports empty in the regression database.

SELECT conrelid::pg_catalog.regclass AS "constrained table",
       conname AS constraint,
       confrelid::pg_catalog.regclass AS "references",
       pg_catalog.format('ALTER TABLE %s DROP CONSTRAINT %I;', conrelid::regclass, conname),
       pg_catalog.format('ALTER TABLE %s ADD CONSTRAINT %I %s;', conrelid::regclass, conname,
                         pg_catalog.pg_get_constraintdef(oid))
FROM pg_catalog.pg_constraint
WHERE contype = 'f' and conparentid = 0 AND
   (SELECT count(*) FROM pg_catalog.pg_constraint p2 WHERE conparentid = pg_constraint.oid) <>
   (SELECT count(*)
      FROM pg_catalog.pg_inherits
     WHERE EXISTS (SELECT 1 FROM pg_catalog.pg_partitioned_table WHERE partrelid = inhparent) AND
     inhparent = pg_constraint.conrelid OR inhparent = pg_constraint.confrelid)
;

I would have loved to be able to add the constraint as NOT VALID
followed by a separate VALIDATE command, because if there are any RI
violations, the constraint would now be in place to prevent future ones.
However,

=# ALTER TABLE fk2367 ADD CONSTRAINT fk_a_fkey FOREIGN KEY (a) REFERENCES pk(a) NOT VALID;
ERROR:  cannot add NOT VALID foreign key on partitioned table "fk2367" referencing relation "pk"
DETAIL:  This feature is not yet supported on partitioned tables.

So it looks like we should suggest to save the output of the query,
execute each DROP followed by each ADD, and if the latter fails, fix the
violations and retry the ADD.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"Java is clearly an example of money oriented programming"  (A. Stepanov)

---

From: Tom Lane <[email-masked]>
Date: Sun, 10 Nov 2024 10:46:48 -0500

> violations and retry the ADD.

Right.  I'll make it so -- thanks for doing the legwork on
creating the query!

			regards, tom lane