Re: [PATCH] Extending FK check skipping on replicas to ADD FK and TRUNCATE

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 <hannuk(at)google(dot)com> 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