Re: [BUG] Fix DETACH with FK pointing to a partitioned table fails - Mailing list pgsql-hackers
From | Tender Wang |
---|---|
Subject | Re: [BUG] Fix DETACH with FK pointing to a partitioned table fails |
Date | |
Msg-id | CAHewXN=pPz-8_qxUJuD5jhm3evXBeta1LgzW4PHArJ-iYz7qHQ@mail.gmail.com Whole thread Raw |
In response to | [BUG] Fix DETACH with FK pointing to a partitioned table fails (Jehan-Guillaume de Rorthais <jgdr@dalibo.com>) |
List | pgsql-hackers |
Alvaro Herrera <alvherre@alvh.no-ip.org> 于2024年10月18日周五 22:52写道:
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?
Hi,
I looked at the patch on master. I gave a little comment in [1]
I reconsider the codes. I suspect that we don't need the below if statement anymore.
/*
* If the referenced side is partitioned (which we know because our
* parent's constraint points to a different relation than ours) then
* we must, in addition to the above, create pg_constraint rows that
* point to each partition, each with its own action triggers.
*/
if (parentConForm->conrelid != conform->conrelid)
* If the referenced side is partitioned (which we know because our
* parent's constraint points to a different relation than ours) then
* we must, in addition to the above, create pg_constraint rows that
* point to each partition, each with its own action triggers.
*/
if (parentConForm->conrelid != conform->conrelid)
{
...
}
The above contidion is always true according to my test.
I haven't figured out an anti-case.
Any thoughts?
Thanks,
Tender Wang
pgsql-hackers by date: