Thread

[BUG] FK broken after DETACHing referencing part

Jehan-Guillaume de Rorthais <jgdr@dalibo.com> — 2023-04-20T12:43:44Z

Hi,

Considering two partitionned tables with a FK between them:

  DROP TABLE IF EXISTS p, c, c_1 CASCADE;

  ----------------------------------
  -- Parent table + partition + data
  CREATE TABLE p (
    id bigint PRIMARY KEY
  )
  PARTITION BY list (id);

  CREATE TABLE p_1 PARTITION OF p FOR VALUES IN (1);

  INSERT INTO p VALUES (1);

  ------------------------------------
  -- Child table + partition + data
  CREATE TABLE c (
    id        bigint PRIMARY KEY,
    p_id bigint NOT NULL,
    FOREIGN KEY (p_id) REFERENCES p (id)
  )
  PARTITION BY list (id);

  CREATE TABLE c_1 PARTITION OF c FOR VALUES IN (1);

  INSERT INTO c VALUES (1,1);

After DETACHing the "c_1" partition, current implementation make sure it
keeps the FK herited from its previous top table "c":

  ALTER TABLE c DETACH PARTITION c_1;
  \d c_1
  -- outputs:
  -- [...]
  -- Foreign-key constraints:
  --   "c_p_id_fkey" FOREIGN KEY (p_id) REFERENCES p(id)

However, because the referenced side is partionned, this FK is half backed, with
only the referencing (insert/update on c_1) side enforced, but not the
referenced side (update/delete on p):

  INSERT INTO c_1 VALUES (2,2); -- fails as EXPECTED
  -- ERROR:  insert or update on table "child_1" violates foreign key [...]

  DELETE FROM p; -- should actually fail
  -- DELETE 1

  SELECT * FROM c_1;
  --  id | parent_id 
  -- ----+-----------
  --   1 |         1
  -- (1 row)

  SELECT * FROM p;
  --  id 
  -- ----
  -- (0 rows)

When detaching "c_1", current implementation adds two triggers to enforce
UPDATE/DELETE on "p" are restricted if "c_1" keeps referencing the
related rows... But it forgets to add them on partitions of "p_1", where the
triggers should actually fire.

To make it clear, the FK c_1 -> p constraint and triggers after DETACHING c_1
are:

  SELECT c.oid AS conid, c.conname, c.conparentid AS conparent,
         r2.relname AS pkrel,
         t.tgrelid::regclass AS tgrel,
         p.proname
  FROM pg_constraint c 
  JOIN pg_class r ON c.conrelid = r.oid
  JOIN pg_class r2 ON c.confrelid = r2.oid
  JOIN pg_trigger t ON t.tgconstraint = c.oid
  JOIN pg_proc p ON p.oid = t.tgfoid
  WHERE r.relname = 'c_1' AND r2.relname LIKE 'p%'
  ORDER BY r.relname, c.conname, t.tgrelid::regclass::text, p.proname;

  --  conid |   conname   | conparent | pkrel | tgrel |       proname        
  -- -------+-------------+-----------+-------+-------+----------------------
  --  18454 | c_p_id_fkey |         0 | p     | c_1   | RI_FKey_check_ins
  --  18454 | c_p_id_fkey |         0 | p     | c_1   | RI_FKey_check_upd
  --  18454 | c_p_id_fkey |         0 | p     | p     | RI_FKey_noaction_del
  --  18454 | c_p_id_fkey |         0 | p     | p     | RI_FKey_noaction_upd

Where they should be:

  --  conid |   conname    | conparent | pkrel | tgrel |       proname        
  -- -------+--------------+-----------+-------+-------+----------------------
  --  18454 | c_p_id_fkey  |         0 | p     | c_1   | RI_FKey_check_ins
  --  18454 | c_p_id_fkey  |         0 | p     | c_1   | RI_FKey_check_upd
  --  18454 | c_p_id_fkey  |         0 | p     | p     | RI_FKey_noaction_del
  --  18454 | c_p_id_fkey  |         0 | p     | p     | RI_FKey_noaction_upd
  --  NEW!! | c_p_id_fkey1 |     18454 | p_1   | p_1   | RI_FKey_noaction_del
  --  NEW!! | c_p_id_fkey1 |     18454 | p_1   | p_1   | RI_FKey_noaction_upd

I poked around DetachPartitionFinalize() to try to find a way to fix this, but
it looks like it would duplicate a bunch of code from other code path (eg.
from CloneFkReferenced).

Instead of tweaking existing FK, keeping old constraint name (wouldn't
"c_1_p_id_fkey" be better after detach?) and duplicating some code around, what
about cleaning up the FK constraints from the detached table and
recreating a cleaner one using the known code path ATAddForeignKeyConstraint() ?

Thanks for reading me down to here!

++