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

From: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
To: Jehan-Guillaume de Rorthais <jgdr(at)dalibo(dot)com>
Cc: Tender Wang <tndrwang(at)gmail(dot)com>, Junwang Zhao <zhjwpku(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, Alexander Lakhin <exclusion(at)gmail(dot)com>, Guillaume Lelarge <guillaume(at)lelarge(dot)info>, Michael Paquier <michael(at)paquier(dot)xyz>
Subject: Re: [BUG] Fix DETACH with FK pointing to a partitioned table fails
Date: 2024-10-18 14:50:59
Message-ID: 202410181450.xchtzcq2uyjt@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2024-Sep-26, Jehan-Guillaume de Rorthais wrote:

> REL_14_STABLE backport doesn't seem trivial, so I'll wait for some
> feedback, review & decision before going further down in backpatching.

Hi, thanks for these patches. I have made some edits of my own. In the
end I decided that I quite liked the new structure of the
addFkConstraint() function and friends. I added a bunch of comments and
changed names somewhat. Also, I think the benefit of the refactoring is
more obvious when all patches are squashed together, so I did that.

For branch 14, I opted to make it delete the constraints on detach.
This isn't ideal but unless somebody wants to spend a lot more time on
this, it seems the best we can do. Leaving broken constraints around
seems worse. The patch for 14 doesn't apply to 13 sadly. I didn't have
time to verify why, but it seems there's some rather larger conflict in
one spot.

I hope to be able to get these pushed over the weekend. That would give
us a couple of weeks before the November releases (but my availability
in those two weeks might be spotty.)

I spent some time going through your test additions and ended up with
your functions in this form:

-- displays constraints in schema fkpart12
CREATE OR REPLACE FUNCTION
fkpart12_constraints(OUT conrel regclass, OUT conname name,
OUT confrelid regclass, OUT conparent text)
RETURNS SETOF record AS $$
WITH RECURSIVE arrh AS (
SELECT oid, conrelid, conname, confrelid, NULL::name AS conparent
FROM pg_constraint
WHERE connamespace = 'fkpart12'::regnamespace AND
contype = 'f' AND conparentid = 0
UNION ALL
SELECT c.oid, c.conrelid, c.conname, c.confrelid,
(pg_identify_object('pg_constraint'::regclass, arrh.oid, 0)).identity
FROM pg_constraint c
JOIN arrh ON c.conparentid = arrh.oid
) SELECT conrelid::regclass, conname, confrelid::regclass, conparent
FROM arrh
ORDER BY conrelid::regclass::text, conname;
$$
LANGUAGE SQL;

-- displays triggers in tables in schema fkpart12
CREATE OR REPLACE FUNCTION
fkpart12_triggers(OUT tablename regclass, OUT constr text,
OUT samefunc boolean, OUT parent text)
RETURNS SETOF record AS $$
WITH RECURSIVE arrh AS (
SELECT t.oid, t.tgrelid::regclass as tablename, tgname,
t.tgfoid::regproc as trigfn,
(pg_identify_object('pg_constraint'::regclass, c.oid, 0)).identity as constr,
NULL::bool as samefunc,
NULL::name AS parent
FROM pg_trigger t
LEFT JOIN pg_constraint c ON c.oid = t.tgconstraint
WHERE (SELECT relnamespace FROM pg_class WHERE oid = t.tgrelid) = 'fkpart12'::regnamespace
AND c.contype = 'f' AND t.tgparentid = 0
UNION ALL
SELECT t2.oid, t2.tgrelid::regclass as tablename, t2.tgname,
t2.tgfoid::regproc as trigfn,
(pg_identify_object('pg_constraint'::regclass, c2.oid, 0)).identity,
arrh.trigfn = t2.tgfoid as samefunc,
replace((pg_identify_object('pg_trigger'::regclass, t2.tgparentid, 0)).identity,
t2.tgparentid::text, 'TGOID')
FROM pg_trigger t2
LEFT JOIN pg_constraint c2 ON c2.oid = t2.tgconstraint
JOIN arrh ON t2.tgparentid = arrh.oid
) SELECT tablename, constr, samefunc, parent
FROM arrh
ORDER BY tablename::text, constr;
$$
LANGUAGE SQL;

However, in the end I think this is a very good technique to verify that
the fix works correctly, but it's excessive to include these results in
the tests forevermore. So I've left them out for now. Maybe we should
reconsider on the older branches?

--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"If you have nothing to say, maybe you need just the right tool to help you
not say it." (New York Times, about Microsoft PowerPoint)

Attachment Content-Type Size
v4_14-0001-Redo-foreign-key-maintenance-during-partition-.patch text/x-diff 18.1 KB
v4_15-0001-Redo-foreign-key-maintenance-during-partition-.patch text/x-diff 45.8 KB
v4_16-0001-Redo-foreign-key-maintenance-during-partition-.patch text/x-diff 45.7 KB
v4_17-0001-Redo-foreign-key-maintenance-during-partition-.patch text/x-diff 45.7 KB
v4_master-0001-Redo-foreign-key-maintenance-during-partit.patch text/x-diff 46.7 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Nathan Bossart 2024-10-18 15:22:51 Re: Doc: shared_memory_size_in_huge_pages with the "SHOW" command.
Previous Message Tom Lane 2024-10-18 14:22:32 Re: Incorrect comment on pg_shadow view