On Sun, 11 Jan 2026 at 14:29, Hannu Krosing <[email protected]> wrote:
> rebased to latest > > On Sun, Jul 6, 2025 at 4:48 PM Hannu Krosing <[email protected]> wrote: > >> And now it also passes tests. >> >> Still learning about the git way of generating PostgreSQL patches, >> that's why there are two separate ones >> >> On Sun, Jul 6, 2025 at 4:30 PM Hannu Krosing <[email protected]> wrote: >> > >> > Managed to send wrong patch earlier, this one actually compiles >> > >> > On Sun, Jul 6, 2025 at 1:48 PM Hannu Krosing <[email protected]> wrote: >> > > >> > > Here is a rebased patch >> > > >> > > this time I did not indent the part under >> > > if(SessionReplicationRole != SESSION_REPLICATION_ROLE_REPLICA) >> > > { >> > > ... (<did not add indent> >> > > } >> > > >> > > so it is immediately obviuos from the patch what is added. >> > > >> > > I can add the indent later, or just let pg_ident take care of this in >> due time >> > > >> > > On Sat, May 24, 2025 at 4:02 PM Hannu Krosing <[email protected]> >> wrote: >> > > > >> > > > I would also argue for treating this as a bug and back-porting to >> all >> > > > supported versions - a quick look at v13 seems to confirm that the >> > > > wrapped code has not changed at least since then. >> > > > >> > > > I don't think we can claim the current state is Working As Intended >> > > > unless someone stands up and says they really intended it to work >> this >> > > > way :) >> > > > >> > > > On Sat, May 24, 2025 at 3:47 PM Hannu Krosing <[email protected]> >> wrote: >> > > > > >> > > > > Hello Everybody, >> > > > > >> > > > > Currently setting `session_replication_role` to `replica` disables >> > > > > foreign key checks allowing, among other things, free table copy >> order >> > > > > and faster CDC apply in logical replication. >> > > > > >> > > > > But two other cases of foreign keys are still restricted or >> blocked >> > > > > even with this setting >> > > > > >> > > > > >> > > > > 1. Foreign Key checks during `ALTER TABLE <fkt> ADD FOREIGN KEY >> > > > > (<fkcol>) REFERENCES <pkt>(<pkcol>);` >> > > > > >> > > > > These can be really, Really, REALLY slow when the PK table is huge >> > > > > (hundreds of billions of rows). And here I mean >> taking-tens-of-days >> > > > > slow in extreme cases. >> > > > > >> > > > > And they are also completely unnecessary when the table data comes >> > > > > from a known good source. >> > > > > >> > > > > >> > > > > 2. `TRUNCATE <referenced table>` is blocked when there are any >> foreign >> > > > > keys referencing the <referenced table> >> > > > > >> > > > > But you still can mess up your database in exactly the same way by >> > > > > running `DELETE FROM <referenced table>`, just a little (or a lot) >> > > > > slower. >> > > > > >> > > > > >> > > > > The attached patch fixes this, allowing both cases to work when >> `SET >> > > > > session_replication_role=replica;` is in force: >> > > > > >> > > > > ### Test for `ALTER TABLE <fkt> ADD FOREIGN KEY (<fkcol>) >> REFERENCES >> > > > > <pkt>(<pkcol>);` >> > > > > >> > > > > CREATE TABLE pkt(id int PRIMARY KEY); >> > > > > CREATE TABLE fkt(fk int); >> > > > > INSERT INTO fkt VALUES(1); >> > > > > >> > > > > ALTER TABLE fkt ADD FOREIGN KEY (fk) REFERENCES pkt(id); -- fails >> > > > > >> > > > > SET session_replication_role=replica; >> > > > > ALTER TABLE fkt ADD FOREIGN KEY (fk) REFERENCES pkt(id); -- now >> succeeds >> > > > > >> > > > > ### Test for `TRUNCATE <referenced table>` >> > > > > >> > > > > CREATE TABLE pkt(id int PRIMARY KEY); >> > > > > CREATE TABLE fkt(fk int REFERENCES pkt(id)); >> > > > > >> > > > > TRUNCATE pkt; -- fails >> > > > > >> > > > > SET session_replication_role=replica; >> > > > > TRUNCATE pkt; -- now succeeds >> > > > > >> > > > > >> > > > > >> > > > > The patch just wraps two sections of code in >> > > > > >> > > > > if (SessionReplicationRole != SESSION_REPLICATION_ROLE_REPLICA) { >> > > > > ... >> > > > > } >> > > > > >> > > > > and the rest is added indentation for the wrapped. >> > > > > >> > > > > Plus I added tests, including the until now missing test for >> > > > > explicitly the foreign key checks (which can be argued to already >> be >> > > > > covered by generic trigger tests) >> > > > > >> > > > > >> > > > > I am not sure if we need to add anything to current >> documentation[*] >> > > > > which says just this about foreign keys and >> > > > > `session_replication_role=replica` >> > > > > >> > > > > > Since foreign keys are implemented as triggers, setting this >> parameter to replica also disables all foreign key checks, which can leave >> data in an inconsistent state if improperly used. >> > > > > >> > > > > [*] >> https://www.postgresql.org/docs/17/runtime-config-client.html#GUC-SESSION-REPLICATION-ROLE >> > > > > >> > > > > Any comments and suggestions are welcome >> > I looked into this patch and found a case which doesn't look right, CREATE TABLE pkt(id int PRIMARY KEY); CREATE TABLE fkt(fk int); INSERT INTO fkt VALUES(1); -- no matching pkt row SET session_replication_role=replica; ALTER TABLE fkt ADD FOREIGN KEY (fk) REFERENCES pkt(id); The Alter command goes through successfully. So, basically, fkt has one row which doesn't follow the constraint. -- Regards, Rafia Sabih CYBERTEC PostgreSQL International GmbH
