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

Started by Hannu Krosing11 months ago6 messageshackers
Jump to latest
#1Hannu Krosing
hannu@tm.ee

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

Attachments:

v1-0001-Skip-FK-Validations-when-Role-is-Replica.patchtext/x-patch; charset=US-ASCII; name=v1-0001-Skip-FK-Validations-when-Role-is-Replica.patchDownload+131-33
#2Hannu Krosing
hannu@tm.ee
In reply to: Hannu Krosing (#1)
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 :)

Show quoted text

On Sat, May 24, 2025 at 3:47 PM Hannu Krosing <hannuk@google.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

#3Hannu Krosing
hannu@tm.ee
In reply to: Hannu Krosing (#2)
Re: [PATCH] Extending FK check skipping on replicas to ADD FK and TRUNCATE

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

Show quoted text

On Sat, May 24, 2025 at 4:02 PM Hannu Krosing <hannuk@google.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@google.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

Attachments:

v2-0001-Skip-FK-Validations-when-Role-is-Replica.patchtext/x-patch; charset=US-ASCII; name=v2-0001-Skip-FK-Validations-when-Role-is-Replica.patchDownload+101-4
#4Hannu Krosing
hannu@tm.ee
In reply to: Hannu Krosing (#3)
Re: [PATCH] Extending FK check skipping on replicas to ADD FK and TRUNCATE

Managed to send wrong patch earlier, this one actually compiles

Show quoted text

On Sun, Jul 6, 2025 at 1:48 PM Hannu Krosing <hannuk@google.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@google.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@google.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

Attachments:

v3-0001-Skip-FK-Validations-when-Role-is-Replica.patchtext/x-patch; charset=US-ASCII; name=v3-0001-Skip-FK-Validations-when-Role-is-Replica.patchDownload+101-4
#5Hannu Krosing
hannu@tm.ee
In reply to: Hannu Krosing (#4)
Re: [PATCH] Extending FK check skipping on replicas to ADD FK and TRUNCATE

And now it also passes tests.

Still learning about the git way of generating PostgreSQL patches,
that's why there are two separate ones

Show quoted text

On Sun, Jul 6, 2025 at 4:30 PM Hannu Krosing <hannuk@google.com> wrote:

Managed to send wrong patch earlier, this one actually compiles

On Sun, Jul 6, 2025 at 1:48 PM Hannu Krosing <hannuk@google.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@google.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@google.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

Attachments:

v4-0002-removed-extra-comment-lines-from-test.patchtext/x-patch; charset=US-ASCII; name=v4-0002-removed-extra-comment-lines-from-test.patchDownload+0-2
v4-0001-rebase-now-avtually-compiles.patchtext/x-patch; charset=US-ASCII; name=v4-0001-rebase-now-avtually-compiles.patchDownload+101-4
#6Hannu Krosing
hannu@tm.ee
In reply to: Hannu Krosing (#5)
Re: [PATCH] Extending FK check skipping on replicas to ADD FK and TRUNCATE

rebased to latest

On Sun, Jul 6, 2025 at 4:48 PM Hannu Krosing <hannuk@google.com> wrote:

Show quoted text

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 <hannuk@google.com> wrote:

Managed to send wrong patch earlier, this one actually compiles

On Sun, Jul 6, 2025 at 1:48 PM Hannu Krosing <hannuk@google.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@google.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@google.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

Attachments:

v5-0001-rebase-to-current-head.patchtext/x-patch; charset=US-ASCII; name=v5-0001-rebase-to-current-head.patchDownload+96-4