Managed to send wrong patch earlier, this one actually compiles
On Sun, Jul 6, 2025 at 1:48 PM Hannu Krosing <hannuk(at)google(dot)com> 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 <hannuk(at)google(dot)com> 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 <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