From 4d0692029e0afa2528e7c34d9e695b887d489c8e Mon Sep 17 00:00:00 2001 From: shveta malik Date: Sun, 13 Sep 2026 16:41:45 +0200 Subject: [PATCH v2 4/4] Invalidate a partition's relcache when its concurrent detach commits The first transaction of ALTER TABLE ... DETACH PARTITION ... CONCURRENTLY sets inhdetachpending on the partition's pg_inherits row and invalidates the parent, so that its partition descriptor no longer includes the partition. Nothing invalidated the partition itself, as its own pg_class row does not change until DETACH PARTITION ... FINALIZE. Its ancestors do change, though: get_partition_ancestors() stops at a partition pending detach. Anything derived from the ancestors and cached per relation was therefore stale from that commit until the finalize, or until an unrelated invalidation happened to arrive. pgoutput kept publishing the partition's changes via the former root, so a subscriber applied them to a table the publisher no longer routes them to; the relcache publication descriptor, which CheckCmdReplicaIdentity() consults, kept the former root's publications as well. Whether a session saw the old or the new state depended on whether it had touched the partition before the detach, which made the behavior nondeterministic. Invalidate the partition in the first transaction, and its descendants if it is itself partitioned, as the finalize step already does: their ancestor lists change as well. Lock the descendants with AccessShareLock only. They must be kept from being dropped while the invalidation is queued, as CacheInvalidateRelcacheByRelid() fails for a dropped relation, but a stronger lock would make the first transaction wait for, say, a VACUUM of a leaf. DetachPartitionFinalize() locks them with AccessExclusiveLock anyway. Author: shveta malik Author: Mikhail Nikalayeu Reported-by: shveta malik Reviewed-by: Nisha Moond Discussion: https://postgr.es/m/CADzfLwWoFPT%2Ba73%3DA%3DbsNWRMZQ98NpBEMgE%3Dt1FS4O4_%3DQVLfA%40mail.gmail.com Backpatch-through: 14 --- src/backend/commands/tablecmds.c | 25 ++++++++ .../detach-partition-concurrently-3.out | 64 +++++++++++++++++++ .../detach-partition-concurrently-3.spec | 23 +++++++ src/test/subscription/t/100_bugs.pl | 9 ++- 4 files changed, 116 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 0274d892f2e..0a6ad7c5fd7 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -21760,6 +21760,31 @@ ATExecDetachPartition(List **wqueue, AlteredTableInfo *tab, Relation rel, /* Invalidate relcache entries for the parent -- must be before close */ CacheInvalidateRelcache(rel); + /* + * Invalidate the partition too, and its descendants if it is itself + * partitioned. Nothing in their pg_class rows changes yet, but their + * ancestors do: get_partition_ancestors() stops at a partition + * pending detach, so anything derived from the ancestors, like the + * publication descriptor, is stale from this commit on. The + * descendants only need to be kept from being dropped until then, as + * invalidating a dropped relation fails. The weakest lock does that + * without making us wait for, say, a VACUUM of a leaf. + */ + CacheInvalidateRelcache(partRel); + if (partRel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) + { + List *children; + ListCell *cell; + + children = find_all_inheritors(RelationGetRelid(partRel), + AccessShareLock, NULL); + foreach(cell, children) + { + if (lfirst_oid(cell) != RelationGetRelid(partRel)) + CacheInvalidateRelcacheByRelid(lfirst_oid(cell)); + } + } + table_close(partRel, NoLock); table_close(rel, NoLock); tab->rel = NULL; diff --git a/src/test/isolation/expected/detach-partition-concurrently-3.out b/src/test/isolation/expected/detach-partition-concurrently-3.out index 8a9b2630d6b..952f407282f 100644 --- a/src/test/isolation/expected/detach-partition-concurrently-3.out +++ b/src/test/isolation/expected/detach-partition-concurrently-3.out @@ -183,6 +183,70 @@ b (1 row) +starting permutation: s2snitch s1pubid s1updidpart s1b s1sid s2detachid s1cancel s1c s1updidpart s1delidpart +step s2snitch: INSERT INTO d3_pid SELECT pg_backend_pid(); +step s1pubid: CREATE PUBLICATION pub_d3_id FOR TABLE d3_idp; +step s1updidpart: UPDATE d3_idp1 SET a = 1; +ERROR: cannot update table "d3_idp1" because it does not have a replica identity and publishes updates +step s1b: BEGIN; +step s1sid: SELECT * FROM d3_idp; +a|b +-+- +(0 rows) + +step s2detachid: ALTER TABLE d3_idp DETACH PARTITION d3_idp1 CONCURRENTLY; +step s1cancel: SELECT pg_cancel_backend(pid) FROM d3_pid; +step s2detachid: <... completed> +ERROR: canceling statement due to user request +step s1cancel: <... completed> +pg_cancel_backend +----------------- +t +(1 row) + +step s1c: COMMIT; +step s1updidpart: UPDATE d3_idp1 SET a = 1; +step s1delidpart: DELETE FROM d3_idp1; + +starting permutation: s2snitch s1pubid s1updidleaf s1b s1sid s2detachid2 s1cancel s1c s1updidleaf s1delidleaf +step s2snitch: INSERT INTO d3_pid SELECT pg_backend_pid(); +step s1pubid: CREATE PUBLICATION pub_d3_id FOR TABLE d3_idp; +step s1updidleaf: UPDATE d3_idp2_leaf SET a = 2; +ERROR: cannot update table "d3_idp2_leaf" because it does not have a replica identity and publishes updates +step s1b: BEGIN; +step s1sid: SELECT * FROM d3_idp; +a|b +-+- +(0 rows) + +step s2detachid2: ALTER TABLE d3_idp DETACH PARTITION d3_idp2 CONCURRENTLY; +step s1cancel: SELECT pg_cancel_backend(pid) FROM d3_pid; +step s2detachid2: <... completed> +ERROR: canceling statement due to user request +step s1cancel: <... completed> +pg_cancel_backend +----------------- +t +(1 row) + +step s1c: COMMIT; +step s1updidleaf: UPDATE d3_idp2_leaf SET a = 2; +step s1delidleaf: DELETE FROM d3_idp2_leaf; + +starting permutation: s1b s1lockidleaf s2detachid2 s1pendingid2 s1c +step s1b: BEGIN; +step s1lockidleaf: LOCK TABLE d3_idp2_leaf IN SHARE UPDATE EXCLUSIVE MODE; +step s2detachid2: ALTER TABLE d3_idp DETACH PARTITION d3_idp2 CONCURRENTLY; +step s1pendingid2: SELECT inhdetachpending FROM pg_inherits + WHERE inhrelid = 'd3_idp2'::regclass; +inhdetachpending +---------------- +t +(1 row) + +step s1c: COMMIT; +step s2detachid2: <... completed> + starting permutation: s2snitch s1b s1s s2detach2 s1cancel s1c s1brr s1insert s1s s1insert s1c step s2snitch: INSERT INTO d3_pid SELECT pg_backend_pid(); step s1b: BEGIN; diff --git a/src/test/isolation/specs/detach-partition-concurrently-3.spec b/src/test/isolation/specs/detach-partition-concurrently-3.spec index 18d9f8aa31f..195ddbff519 100644 --- a/src/test/isolation/specs/detach-partition-concurrently-3.spec +++ b/src/test/isolation/specs/detach-partition-concurrently-3.spec @@ -23,6 +23,7 @@ setup } teardown { + DROP PUBLICATION IF EXISTS pub_d3_id; DROP TABLE IF EXISTS d3_listp, d3_listp1, d3_listp2, d3_pid; DROP TABLE IF EXISTS d3_idp, d3_idp1, d3_idp2, d3_idp2_leaf; } @@ -44,6 +45,14 @@ step s1exceptpart { CREATE PUBLICATION pub_d3 FOR ALL TABLES EXCEPT (TABLE d3_li step s1sid { SELECT * FROM d3_idp; } step s1insertidpart { INSERT INTO d3_idp1 (a) VALUES (1) RETURNING b; } step s1insertidleaf { INSERT INTO d3_idp2_leaf (a) VALUES (2) RETURNING b; } +step s1pubid { CREATE PUBLICATION pub_d3_id FOR TABLE d3_idp; } +step s1updidpart { UPDATE d3_idp1 SET a = 1; } +step s1delidpart { DELETE FROM d3_idp1; } +step s1updidleaf { UPDATE d3_idp2_leaf SET a = 2; } +step s1delidleaf { DELETE FROM d3_idp2_leaf; } +step s1lockidleaf { LOCK TABLE d3_idp2_leaf IN SHARE UPDATE EXCLUSIVE MODE; } +step s1pendingid2 { SELECT inhdetachpending FROM pg_inherits + WHERE inhrelid = 'd3_idp2'::regclass; } step s1drop { DROP TABLE d3_listp; } step s1droppart { DROP TABLE d3_listp1; } step s1trunc { TRUNCATE TABLE d3_listp; } @@ -83,6 +92,20 @@ permutation s2snitch s1b s1s s2detach s1cancel(s2detach) s1c s1updpart s1delpart permutation s2snitch s1b s1sid s2detachid s1cancel(s2detachid) s1c s1insertidpart permutation s2snitch s1b s1sid s2detachid2 s1cancel(s2detachid2) s1c s1insertidleaf +# A session that built the publication descriptor of a partition before the +# detach must see it leave the former root's publications as soon as the first +# transaction commits: without a replica identity, UPDATE and DELETE fail +# while the partition is published via the root, and are allowed afterwards. +# The same holds for a partition of a partition pending detach. +permutation s2snitch s1pubid s1updidpart s1b s1sid s2detachid s1cancel(s2detachid) s1c s1updidpart s1delidpart +permutation s2snitch s1pubid s1updidleaf s1b s1sid s2detachid2 s1cancel(s2detachid2) s1c s1updidleaf s1delidleaf + +# Invalidating the descendants of a partitioned partition must not make the +# first transaction wait for a lock that does not conflict with dropping them, +# like the one VACUUM takes on a leaf. The first transaction commits, and only +# the finalize step waits. +permutation s1b s1lockidleaf s2detachid2 s1pendingid2 s1c + # Test partition descriptor caching permutation s2snitch s1b s1s s2detach2 s1cancel(s2detach2) s1c s1brr s1insert s1s s1insert s1c permutation s2snitch s1b s1s s2detach2 s1cancel(s2detach2) s1c s1brr s1s s1insert s1s s1c diff --git a/src/test/subscription/t/100_bugs.pl b/src/test/subscription/t/100_bugs.pl index 67cdb312f9d..db7ca032187 100644 --- a/src/test/subscription/t/100_bugs.pl +++ b/src/test/subscription/t/100_bugs.pl @@ -199,11 +199,6 @@ for my $partition_type ('regular', 'partitioned') 'targeted lookup does not exclude the partition pending detach either' ); - # Changes nothing; drops the walsender's cached mapping. - $node_publisher->safe_psql('postgres', - 'ALTER PUBLICATION pub_detach SET (publish_via_partition_root = true)' - ); - # Use another slot to check which changes pgoutput sends, independently of # whether the subscriber applies them. $node_publisher->safe_psql('postgres', @@ -213,6 +208,10 @@ for my $partition_type ('regular', 'partitioned') # This is what crashed. The part1 change is sent using part1's identity, but # the subscriber skips it because only parted is registered in # pg_subscription_rel. The following change shows decoding got past it. + # + # The walsender has cached part1 as published via parted since the insert + # before the detach; the detach must have invalidated that, or the change + # would still be sent via parted and applied to parted on the subscriber. $node_publisher->safe_psql( 'postgres', q[ INSERT INTO part1 VALUES (1, 2); -- 2.43.0