Alvaro Herrera <alvhe...@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) { ... } 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