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

Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)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)
{
...
}

The above contidion is always true according to my test.
I haven't figured out an anti-case.
Any thoughts?

[1]
https://www.postgresql.org/message-id/CAHewXNkuU2V7GfgFkwd265RJ99%2BBfJueNEZhrHSk39o3thqxNA%40mail.gmail.com

--
Thanks,
Tender Wang