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
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index e6606e5e57a..592a12fa95b 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -1975,13 +1975,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
@@ -5918,49 +5924,55 @@ 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
*/
- foreach(ltab, *wqueue)
+ if (SessionReplicationRole != SESSION_REPLICATION_ROLE_REPLICA)
{
- AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
- Relation rel = NULL;
- ListCell *lcon;
-
- /* Relations without storage may be ignored here too */
- if (!RELKIND_HAS_STORAGE(tab->relkind))
- continue;
-
- foreach(lcon, tab->constraints)
+ foreach(ltab, *wqueue)
{
- NewConstraint *con = lfirst(lcon);
+ AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
+ Relation rel = NULL;
+ ListCell *lcon;
- if (con->contype == CONSTR_FOREIGN)
+ /* Relations without storage may be ignored here too */
+ if (!RELKIND_HAS_STORAGE(tab->relkind))
+ continue;
+
+ foreach(lcon, tab->constraints)
{
- Constraint *fkconstraint = (Constraint *) con->qual;
- Relation refrel;
+ NewConstraint *con = lfirst(lcon);
- if (rel == NULL)
+ if (con->contype == CONSTR_FOREIGN)
{
- /* Long since locked, no need for another */
- rel = table_open(tab->relid, NoLock);
- }
+ Constraint *fkconstraint = (Constraint *) con->qual;
+ Relation refrel;
- refrel = table_open(con->refrelid, RowShareLock);
+ if (rel == NULL)
+ {
+ /* Long since locked, no need for another */
+ rel = table_open(tab->relid, NoLock);
+ }
- validateForeignKeyConstraint(fkconstraint->conname, rel, refrel,
- con->refindid,
- con->conid);
+ refrel = table_open(con->refrelid, RowShareLock);
- /*
- * No need to mark the constraint row as validated, we did
- * that when we inserted the row earlier.
- */
+ validateForeignKeyConstraint(fkconstraint->conname, rel, refrel,
+ con->refindid,
+ con->conid);
- table_close(refrel, NoLock);
+ /*
+ * No need to mark the constraint row as validated, we did
+ * that when we inserted the row earlier.
+ */
+
+ table_close(refrel, NoLock);
+ }
}
- }
- if (rel)
- table_close(rel, NoLock);
+ if (rel)
+ table_close(rel, NoLock);
+ }
}
/* Finally, run any afterStmts that were queued up */
diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out
index 8cbad245217..b6d41e6df11 100644
--- a/src/test/regress/expected/foreign_key.out
+++ b/src/test/regress/expected/foreign_key.out
@@ -3073,6 +3073,41 @@ ALTER TABLE fk_r DROP CONSTRAINT fk_r_p_id_p_jd_fkey1;
ERROR: cannot drop inherited constraint "fk_r_p_id_p_jd_fkey1" 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 ea08fd5a6f6..b24277a901f 100644
--- a/src/test/regress/sql/foreign_key.sql
+++ b/src/test/regress/sql/foreign_key.sql
@@ -1151,6 +1151,10 @@ commit;
drop table pktable2, fktable2;
+--
+--
+--
+
--
-- Test keys that "look" different but compare as equal
--
@@ -2177,7 +2181,54 @@ 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_fkey1;
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;
RESET search_path;
+
