| From: | Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com> |
|---|---|
| To: | Fujii Masao <masao(dot)fujii(at)gmail(dot)com> |
| Cc: | pgsql-hackers(at)lists(dot)postgresql(dot)org, Aleksander Alekseev <aleksander(at)tigerdata(dot)com> |
| Subject: | Re: Prevent foreign key references to unlogged partitions |
| Date: | 2026-11-11 23:42:40 |
| Message-ID: | CAN4CZFMGVZf4XVpbRf2CpoXF7_qcVpGAXAd+TmOr8DcOakKS9w@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> Even with the v2 patch, the following steps seem to allow a permanent table
> to reference an unlogged partition. Is this an issue?
Eh, I think it is, but it is not specific to partitioned tables. This
also works, and it shouldn't:
CREATE UNLOGGED TABLE upk (i int PRIMARY KEY);
CREATE UNLOGGED TABLE fk (x int);
ALTER TABLE fk SET LOGGED, ADD FOREIGN KEY (x) REFERENCES upk;
ATPrepChangePersistence checks the existing foreign keys in the prep
phase, the new constraint is added later while the table is still
unlogged, and the rewrite makes it permanent afterwards.
0003 moves the foreign key check right before the end of phase 3.
Similarly to the above example, we also had a (mostly theoretical)
issue with USING expressions and table_rewrite event triggers.
USING expressions are fixed by 0003, but table_rewrite triggers
aren't: if a table_rewrite trigger changes the persistence the rewrite
later reverts that change, and we can again end up with a mismatch in
a foreign key.
0004 fixes this by moving when we fire those triggers. (this also
affected access methods and tablespace changes)
> AFAIR, earlier versions allowed unlogged partitioned tables to have permanent
> partitions. In those versions, would the check added by this patch still work
> as expected in that case?
No, it wouldn't, 17 and earlier need additional checks on both the
referencing and referenced side, and 0002 is significantly more
complex there. I also attached a version for that, with more detailed
test coverage, that should prevent any combination where users try
create a foreign key between logged and unlogged tables.
I kept 0001 for now part of the patchset: if the patch in the other
thread gets committed, I'll drop it, the later patches do not depend
on it.
And one more question: these changes could cause pg_upgrade failures
for affected databases - but it's also a corner case, so I am not sure
how much handling it needs. Should we add detection this for
pg_upgrade --check, or at least mention it in the release notes?
e2bab2d7920 already broke upgrading without an added check, as this is
a valid 17 schema:
CREATE UNLOGGED TABLE upk (a int PRIMARY KEY);
CREATE UNLOGGED TABLE ufk (x int REFERENCES upk) PARTITION BY RANGE (x);
CREATE UNLOGGED TABLE ufk1 PARTITION OF ufk FOR VALUES FROM (0) TO (100);
and it fails the upgrade to 18, but pg_upgrade --check doesn't report it.
| Attachment | Content-Type | Size |
|---|---|---|
| nocfbot-v3-REL_17-0001-Check-partition-persistence-on-both-sides-of-fore.patch | text/x-patch | 18.6 KB |
| nocfbot-v3-REL_17-0002-Check-foreign-keys-after-adding-them-in-ALTER-TAB.patch | text/x-patch | 16.2 KB |
| nocfbot-v3-REL_17-0003-Don-t-undo-table_rewrite-event-trigger-changes-in.patch | text/x-patch | 5.2 KB |
| v3-0001-pgbench-don-t-disable-unlogged-tables-when-partit.patch | text/x-patch | 3.1 KB |
| v3-0002-Disallow-foreign-keys-referencing-unlogged-partit.patch | text/x-patch | 8.3 KB |
| v3-0003-Check-foreign-keys-after-adding-them-in-ALTER-TAB.patch | text/x-patch | 15.8 KB |
| v3-0004-Don-t-undo-table_rewrite-event-trigger-changes-in.patch | text/x-patch | 5.2 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Melanie Plageman | 2026-11-11 23:51:04 | Re: [BUG / PATCH] PG19: VACUUM misestimates reltuples (often 0) after on-access VM setting |
| Previous Message | Michael Paquier | 2026-11-11 23:34:10 | Re: Fix resource leaks in pg_receivewal on LZ4 close failures |