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 >
From 5cf152e9ffb72072e75911da7a123c1f57abd4e7 Mon Sep 17 00:00:00 2001 From: Hannu Krosing <[email protected]> Date: Sun, 11 Jan 2026 01:04:30 +0100 Subject: [PATCH v5] rebase to current head --- src/backend/commands/tablecmds.c | 18 +++++++-- src/test/regress/expected/foreign_key.out | 35 +++++++++++++++++ src/test/regress/sql/foreign_key.sql | 46 +++++++++++++++++++++++ 3 files changed, 96 insertions(+), 3 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index f976c0e5c7e..513e6df36a9 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -2041,13 +2041,19 @@ ExecuteTruncateGuts(List *explicit_rels, * Check foreign key references. In CASCADE mode, this should be * unnecessary since we just pulled in all the references; but as a * cross-check, do it anyway if in an Assert-enabled build. + * + * Skip foreign key checks when `session_replication_role = replica` to + * match the behaviour of disabling FK triggers in the same situation */ + if (SessionReplicationRole != SESSION_REPLICATION_ROLE_REPLICA) + { #ifdef USE_ASSERT_CHECKING - heap_truncate_check_FKs(rels, false); -#else - if (behavior == DROP_RESTRICT) heap_truncate_check_FKs(rels, false); +#else + if (behavior == DROP_RESTRICT) + heap_truncate_check_FKs(rels, false); #endif + } /* * If we are asked to restart sequences, find all the sequences, lock them @@ -6079,7 +6085,12 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode, * theoretically possible that we have changed both relations of the * foreign key, and we'd better have finished both rewrites before we try * to read the tables. + * + * Skip the check when `session_replication_mode = replica` to save time + * and to match the FK trigger behaviour in the same situation */ + if (SessionReplicationRole != SESSION_REPLICATION_ROLE_REPLICA) + { foreach(ltab, *wqueue) { AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab); @@ -6124,6 +6135,7 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode, if (rel) table_close(rel, NoLock); } + } /* Finally, run any afterStmts that were queued up */ foreach(ltab, *wqueue) diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out index 9ae4dbf1b0a..eabea671967 100644 --- a/src/test/regress/expected/foreign_key.out +++ b/src/test/regress/expected/foreign_key.out @@ -3402,6 +3402,41 @@ ALTER TABLE fk_r DROP CONSTRAINT fk_r_p_id_p_jd_fkey_1; ERROR: cannot drop inherited constraint "fk_r_p_id_p_jd_fkey_1" of relation "fk_r" ALTER TABLE fk_r_2 DROP CONSTRAINT fk_r_p_id_p_jd_fkey; ERROR: cannot drop inherited constraint "fk_r_p_id_p_jd_fkey" of relation "fk_r_2" +-- tests for SET session_replication_role = replica; +RESET session_replication_role; +-- disabling FK checks +CREATE TABLE pkt(id int PRIMARY KEY); +CREATE TABLE fkt(fk int REFERENCES pkt(id)); +INSERT INTO fkt VALUES(1); -- should fail +ERROR: insert or update on table "fkt" violates foreign key constraint "fkt_fk_fkey" +DETAIL: Key (fk)=(1) is not present in table "pkt". +SET session_replication_role=replica; +INSERT INTO fkt VALUES(1); -- should succeed now +DROP TABLE fkt, pkt; +RESET session_replication_role; +-- skipping FK validation during ALTER TABLE ... ADD FOREIGN KEY +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); -- should fail +ERROR: insert or update on table "fkt" violates foreign key constraint "fkt_fk_fkey" +DETAIL: Key (fk)=(1) is not present in table "pkt". +SET session_replication_role=replica; +ALTER TABLE fkt ADD FOREIGN KEY (fk) REFERENCES pkt(id); -- should succeed now +DROP TABLE fkt, pkt; +RESET session_replication_role; +-- skipping FK existence checks during TRUNCATE +CREATE TABLE pkt(id int PRIMARY KEY); +CREATE TABLE fkt(fk int REFERENCES pkt(id)); +TRUNCATE pkt; -- should fail +ERROR: cannot truncate a table referenced in a foreign key constraint +DETAIL: Table "fkt" references "pkt". +HINT: Truncate table "fkt" at the same time, or use TRUNCATE ... CASCADE. +SET session_replication_role=replica; +TRUNCATE pkt; -- should succeed now +DROP TABLE fkt, pkt; +RESET session_replication_role; +-- end of tests for SET session_replication_role = replica; SET client_min_messages TO warning; DROP SCHEMA fkpart12 CASCADE; RESET client_min_messages; diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql index 3b8c95bf893..32486036304 100644 --- a/src/test/regress/sql/foreign_key.sql +++ b/src/test/regress/sql/foreign_key.sql @@ -2382,6 +2382,52 @@ ALTER TABLE fk_r_1 DROP CONSTRAINT fk_r_p_id_p_jd_fkey; ALTER TABLE fk_r DROP CONSTRAINT fk_r_p_id_p_jd_fkey_1; ALTER TABLE fk_r_2 DROP CONSTRAINT fk_r_p_id_p_jd_fkey; +-- tests for SET session_replication_role = replica; + +RESET session_replication_role; + +-- disabling FK checks + +CREATE TABLE pkt(id int PRIMARY KEY); +CREATE TABLE fkt(fk int REFERENCES pkt(id)); + +INSERT INTO fkt VALUES(1); -- should fail + +SET session_replication_role=replica; +INSERT INTO fkt VALUES(1); -- should succeed now + +DROP TABLE fkt, pkt; +RESET session_replication_role; + +-- skipping FK validation during ALTER TABLE ... ADD FOREIGN KEY + +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); -- should fail + +SET session_replication_role=replica; +ALTER TABLE fkt ADD FOREIGN KEY (fk) REFERENCES pkt(id); -- should succeed now + +DROP TABLE fkt, pkt; +RESET session_replication_role; + +-- skipping FK existence checks during TRUNCATE + +CREATE TABLE pkt(id int PRIMARY KEY); +CREATE TABLE fkt(fk int REFERENCES pkt(id)); + +TRUNCATE pkt; -- should fail + +SET session_replication_role=replica; +TRUNCATE pkt; -- should succeed now + +DROP TABLE fkt, pkt; +RESET session_replication_role; + +-- end of tests for SET session_replication_role = replica; + SET client_min_messages TO warning; DROP SCHEMA fkpart12 CASCADE; RESET client_min_messages; -- 2.43.0
