On Thu, Nov 12, 2026 at 1:42 PM Amit Kapila <[email protected]> wrote: > > On Thu, Nov 12, 2026 at 11:29 AM Nisha Moond <[email protected]> wrote: > > > > On Wed, Nov 11, 2026 at 8:04 PM Zhijie Hou <[email protected]> wrote: > > > > > > I'm helping to merge the changes into 0001 and preparing patches for all > > > branches, hoping to close this item soon. > > > > > > > Thanks for the patches. The fix LGTM. I tested the patches on all > > respective branches and found no issues. > > > > Pushed. >
Here are the remaining patches rebased and updated as needed: v3-0001: Rebased only; no other changes. v3-0002: The code is unchanged. Its TAP test depended on the setup in the old v2-0001 test in 100_bugs.pl, which was not committed (the committed fix tests via test_decoding instead). I kept the parts it needs and trimmed the rest to what this patch fixes. The checks of what pgoutput sends are removed, as decoding is now covered by the committed test. v3-0003: Rebased only; no other changes. -- Thanks, Nisha
From 395bc25b124ace4dd186dc1448bacdbcd147e9fe Mon Sep 17 00:00:00 2001 From: nkey <[email protected]> Date: Thu, 13 Aug 2026 11:18:37 +0200 Subject: [PATCH v3 1/3] Fix crash inserting into a partition pending detach with an identity column MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ALTER TABLE ... DETACH PARTITION ... CONCURRENTLY, added by 71f4c8c6f74b, leaves the partition marked as detaching when its wait is interrupted -- by a lock timeout, a cancel, a disconnect. Only DETACH PARTITION ... FINALIZE clears that mark, and the partition cannot be attached back. In that state pg_class still says relispartition, while get_partition_ancestors() already reports nothing. getIdentitySequence() was not ready for it. The sequence behind a partition's identity column belongs to the root of its partition tree, so it asked for the last ancestor, which is an assertion failure, and a NULL pointer dereference without assertions. Any INSERT that has to compute the default of such a column gets there. The root is genuinely wanted here: the partition keeps its identity columns, and the sequences behind them keep belonging to the root until the detach is finalized, so climb to it regardless of the mark. Oversight in 509199587df7, which taught getIdentitySequence() to climb to the topmost partitioned table with get_partition_ancestors(), which by then could already report none. Author: Mikhail Nikalayeu <[email protected]> Reviewed-by: Álvaro Herrera <[email protected]> Reviewed-by: Nisha Moond <[email protected]> Discussion: https://postgr.es/m/CADzfLwWoFPT%2Ba73%3DA%3DbsNWRMZQ98NpBEMgE%3Dt1FS4O4_%3DQVLfA%40mail.gmail.com Backpatch-through: 17 --- src/backend/catalog/pg_depend.c | 19 +++++-- .../detach-partition-concurrently-3.out | 52 +++++++++++++++++++ .../detach-partition-concurrently-3.spec | 18 +++++++ 3 files changed, 86 insertions(+), 3 deletions(-) diff --git a/src/backend/catalog/pg_depend.c b/src/backend/catalog/pg_depend.c index 9a7a401aced..0cf8ef6c183 100644 --- a/src/backend/catalog/pg_depend.c +++ b/src/backend/catalog/pg_depend.c @@ -1157,15 +1157,28 @@ getIdentitySequence(Relation rel, AttrNumber attnum, bool missing_ok) */ if (RelationGetForm(rel)->relispartition) { - List *ancestors = get_partition_ancestors(relid); const char *attname = get_attname(relid, attnum, false); - relid = llast_oid(ancestors); + /* + * Climb to the root. get_partition_ancestors() is unusable here: it + * stops at a partition whose concurrent detach has been committed but + * not finalized, and so reports no ancestors at all for one. Such a + * partition keeps its identity columns, and the sequences behind them + * keep belonging to the root, until the detach is finalized. + * + * XXX A variant of get_partition_ancestors() taking even_if_detached + * would climb with pg_inherits opened once, but adding one would make + * this harder to back-patch. + */ + do + { + relid = get_partition_parent(relid, true); + } while (get_rel_relispartition(relid)); + attnum = get_attnum(relid, attname); if (attnum == InvalidAttrNumber) elog(ERROR, "cache lookup failed for attribute \"%s\" of relation %u", attname, relid); - list_free(ancestors); } seqlist = getOwnedSequences_internal(relid, attnum, DEPENDENCY_INTERNAL); diff --git a/src/test/isolation/expected/detach-partition-concurrently-3.out b/src/test/isolation/expected/detach-partition-concurrently-3.out index c1705d80aa1..8a9b2630d6b 100644 --- a/src/test/isolation/expected/detach-partition-concurrently-3.out +++ b/src/test/isolation/expected/detach-partition-concurrently-3.out @@ -131,6 +131,58 @@ step s1delpart: DELETE FROM d3_listp1; step s1exceptpart: CREATE PUBLICATION pub_d3 FOR ALL TABLES EXCEPT (TABLE d3_listp1); ERROR: cannot specify relation "public.d3_listp1" in the publication EXCEPT clause +starting permutation: s2snitch s1b s1sid s2detachid s1cancel s1c s1insertidpart +step s2snitch: INSERT INTO d3_pid SELECT pg_backend_pid(); +step s1b: BEGIN; +step s1sid: SELECT * FROM d3_idp; +a|b +-+- +(0 rows) + +step s2detachid: ALTER TABLE d3_idp DETACH PARTITION d3_idp1 CONCURRENTLY; <waiting ...> +step s1cancel: SELECT pg_cancel_backend(pid) FROM d3_pid; <waiting ...> +step s2detachid: <... completed> +ERROR: canceling statement due to user request +step s1cancel: <... completed> +pg_cancel_backend +----------------- +t +(1 row) + +step s1c: COMMIT; +step s1insertidpart: INSERT INTO d3_idp1 (a) VALUES (1) RETURNING b; +b +- +1 +(1 row) + + +starting permutation: s2snitch s1b s1sid s2detachid2 s1cancel s1c s1insertidleaf +step s2snitch: INSERT INTO d3_pid SELECT pg_backend_pid(); +step s1b: BEGIN; +step s1sid: SELECT * FROM d3_idp; +a|b +-+- +(0 rows) + +step s2detachid2: ALTER TABLE d3_idp DETACH PARTITION d3_idp2 CONCURRENTLY; <waiting ...> +step s1cancel: SELECT pg_cancel_backend(pid) FROM d3_pid; <waiting ...> +step s2detachid2: <... completed> +ERROR: canceling statement due to user request +step s1cancel: <... completed> +pg_cancel_backend +----------------- +t +(1 row) + +step s1c: COMMIT; +step s1insertidleaf: INSERT INTO d3_idp2_leaf (a) VALUES (2) RETURNING b; +b +- +1 +(1 row) + + 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 6d359756e31..18d9f8aa31f 100644 --- a/src/test/isolation/specs/detach-partition-concurrently-3.spec +++ b/src/test/isolation/specs/detach-partition-concurrently-3.spec @@ -14,10 +14,17 @@ setup CREATE TABLE d3_listp2 PARTITION OF d3_listp FOR VALUES IN (2); CREATE TABLE d3_pid (pid int); INSERT INTO d3_listp VALUES (1); + CREATE TABLE d3_idp (a int, b bigint GENERATED BY DEFAULT AS IDENTITY) + PARTITION BY LIST(a); + CREATE TABLE d3_idp1 PARTITION OF d3_idp FOR VALUES IN (1); + CREATE TABLE d3_idp2 PARTITION OF d3_idp FOR VALUES IN (2) PARTITION BY LIST(a); + CREATE TABLE d3_idp2_leaf (b bigint NOT NULL, a int); + ALTER TABLE d3_idp2 ATTACH PARTITION d3_idp2_leaf FOR VALUES IN (2); } teardown { 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; } session s1 @@ -34,6 +41,9 @@ step s1insertpart { INSERT INTO d3_listp1 VALUES (1); } step s1updpart { UPDATE d3_listp1 SET a = 1; } step s1delpart { DELETE FROM d3_listp1; } step s1exceptpart { CREATE PUBLICATION pub_d3 FOR ALL TABLES EXCEPT (TABLE d3_listp1); } +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 s1drop { DROP TABLE d3_listp; } step s1droppart { DROP TABLE d3_listp1; } step s1trunc { TRUNCATE TABLE d3_listp; } @@ -47,6 +57,8 @@ step s2begin { BEGIN; } step s2snitch { INSERT INTO d3_pid SELECT pg_backend_pid(); } step s2detach { ALTER TABLE d3_listp DETACH PARTITION d3_listp1 CONCURRENTLY; } step s2detach2 { ALTER TABLE d3_listp DETACH PARTITION d3_listp2 CONCURRENTLY; } +step s2detachid { ALTER TABLE d3_idp DETACH PARTITION d3_idp1 CONCURRENTLY; } +step s2detachid2 { ALTER TABLE d3_idp DETACH PARTITION d3_idp2 CONCURRENTLY; } step s2detachfinal { ALTER TABLE d3_listp DETACH PARTITION d3_listp1 FINALIZE; } step s2drop { DROP TABLE d3_listp1; } step s2commit { COMMIT; } @@ -64,6 +76,12 @@ permutation s2snitch s1b s1s s2detach s1cancel(s2detach) s1c s1insertpart # reports no ancestors, even though relispartition is still set, which # would otherwise trigger a topmost ancestor lookup crash. permutation s2snitch s1b s1s s2detach s1cancel(s2detach) s1c s1updpart s1delpart s1exceptpart +# The sequence behind an identity column lives in the topmost ancestor, and a +# partition pending detach has none. The climb to the root must also get past +# an intermediate partition pending detach, and map the column by name: the +# leaf has its columns in a different order than the root. +permutation s2snitch s1b s1sid s2detachid s1cancel(s2detachid) s1c s1insertidpart +permutation s2snitch s1b s1sid s2detachid2 s1cancel(s2detachid2) s1c s1insertidleaf # Test partition descriptor caching permutation s2snitch s1b s1s s2detach2 s1cancel(s2detach2) s1c s1brr s1insert s1s s1insert s1c -- 2.54.0 (Apple Git-157)
From a1dd07e756fca9a8b6d6add3006713fa63faf248 Mon Sep 17 00:00:00 2001 From: Nisha Moond <[email protected]> Date: Fri, 13 Nov 2026 11:24:37 +0530 Subject: [PATCH v3 2/3] Include partitions pending detach in publication table lists For FOR ALL TABLES publications with publish_via_partition_root enabled, pgoutput publishes a partition with no active ancestors separately, but the publication table lookup still excludes it based on relispartition. As a result, REFRESH PUBLICATION cannot register the table before FINALIZE, and the subscriber silently skips its changes until then. The same holds when the EXCEPT clause names the former root: it no longer covers the partition. Use the active ancestor list in both the full and targeted publication table lookups. Cover regular and partitioned tables pending detach, including publication membership with and without an EXCEPT clause, the initial sync of a new subscription, and applying new changes after refresh with copy_data disabled. The full lookup, GetAllPublicationRelations(), has been affected since v14. The targeted one, is_table_publishable_in_publication(), was only added in v19 by fd7a25af11e2, so older branches need just the first half. Author: Zsolt Parragi <[email protected]> Reported-by: shveta malik <[email protected]> Reviewed-by: shveta malik <[email protected]> Reviewed-by: Amit Kapila <[email protected]> Reviewed-by: Nisha Moond <[email protected]> Reviewed-by: Mikhail Nikalayeu <[email protected]> Discussion: https://postgr.es/m/CADzfLwWoFPT%2Ba73%3DA%3DbsNWRMZQ98NpBEMgE%3Dt1FS4O4_%3DQVLfA%40mail.gmail.com Backpatch-through: 14 --- src/backend/catalog/pg_publication.c | 40 ++++-- src/test/subscription/t/100_bugs.pl | 198 +++++++++++++++++++++++++++ 2 files changed, 230 insertions(+), 8 deletions(-) diff --git a/src/backend/catalog/pg_publication.c b/src/backend/catalog/pg_publication.c index f11ea8839ca..d9eda70ecf3 100644 --- a/src/backend/catalog/pg_publication.c +++ b/src/backend/catalog/pg_publication.c @@ -1077,6 +1077,25 @@ GetAllTablesPublications(void) return result; } +/* + * Returns true if the given partition still has ancestors. + * + * A partition whose concurrent detach has been committed but not finalized + * has relispartition set, but get_partition_ancestors() reports nothing for + * it. Publications treat such a partition as a standalone table, as after + * the detach is finalized, so relispartition alone must not be used to + * decide whether a relation is published via its root. + */ +static bool +partition_has_ancestors(Oid relid) +{ + List *ancestors = get_partition_ancestors(relid); + bool result = (ancestors != NIL); + + list_free(ancestors); + return result; +} + /* * Gets list of all relations published by FOR ALL TABLES/SEQUENCES * publication. @@ -1122,7 +1141,8 @@ GetAllPublicationRelations(Oid pubid, char relkind, bool pubviaroot) Oid relid = relForm->oid; if (is_publishable_class(relid, relForm) && - !(relForm->relispartition && pubviaroot) && + !(pubviaroot && relForm->relispartition && + partition_has_ancestors(relid)) && !list_member_oid(exceptlist, relid)) result = lappend_oid(result, relid); } @@ -1144,7 +1164,8 @@ GetAllPublicationRelations(Oid pubid, char relkind, bool pubviaroot) Oid relid = relForm->oid; if (is_publishable_class(relid, relForm) && - !relForm->relispartition && + !(relForm->relispartition && + partition_has_ancestors(relid)) && !list_member_oid(exceptlist, relid)) result = lappend_oid(result, relid); } @@ -1370,7 +1391,6 @@ GetPublicationByName(const char *pubname, bool missing_ok) static bool is_table_publishable_in_publication(Oid relid, Publication *pub) { - bool relispartition; List *ancestors = NIL; /* @@ -1380,9 +1400,13 @@ is_table_publishable_in_publication(Oid relid, Publication *pub) if (!pub->pubviaroot && get_rel_relkind(relid) == RELKIND_PARTITIONED_TABLE) return false; - relispartition = get_rel_relispartition(relid); - - if (relispartition) + /* + * A partition whose concurrent detach has been committed but not + * finalized reports no ancestors, even though relispartition is still + * set. Treat such a partition as a standalone table, as after the detach + * is finalized. + */ + if (get_rel_relispartition(relid)) ancestors = get_partition_ancestors(relid); if (pub->alltables) @@ -1391,7 +1415,7 @@ is_table_publishable_in_publication(Oid relid, Publication *pub) * ALL TABLES with pubviaroot includes only regular tables or top-most * partitioned tables -- never child partitions. */ - if (pub->pubviaroot && relispartition) + if (pub->pubviaroot && ancestors) return false; /* @@ -1428,7 +1452,7 @@ is_table_publishable_in_publication(Oid relid, Publication *pub) * If it's false, the partition is covered by its ancestor's presence in * the publication, it should be included (return true). */ - if (relispartition && + if (ancestors && OidIsValid(GetTopMostAncestorInPublication(pub->oid, ancestors, NULL))) return !pub->pubviaroot; diff --git a/src/test/subscription/t/100_bugs.pl b/src/test/subscription/t/100_bugs.pl index a39d6b0d100..f81e7fbb9d4 100644 --- a/src/test/subscription/t/100_bugs.pl +++ b/src/test/subscription/t/100_bugs.pl @@ -79,6 +79,204 @@ $node_publisher->stop('fast'); $node_subscriber->stop('fast'); +# Replicating changes to a partition whose concurrent detach never finished. + +# Between the two transactions of ALTER TABLE ... DETACH PARTITION ... +# CONCURRENTLY, pg_class still says relispartition while +# get_partition_ancestors() already reports nothing. The partition is then +# published as a standalone table, so the publication's table lists must +# include it too. The state outlives the command: only DETACH PARTITION ... +# FINALIZE clears it. + +$node_publisher->append_conf('postgresql.conf', + 'max_prepared_transactions = 1'); +$node_publisher->rotate_logfile(); +$node_publisher->start(); +$node_subscriber->rotate_logfile(); +$node_subscriber->start(); + +for my $partition_type ('regular', 'partitioned') +{ + note "pending detach of a $partition_type partition"; + my $partition_by = + $partition_type eq 'partitioned' ? 'PARTITION BY LIST (b)' : ''; + my $detach_ddl = qq[ + CREATE TABLE parted (a int, b int) PARTITION BY LIST (a); + CREATE TABLE part1 PARTITION OF parted FOR VALUES IN (1) $partition_by; + CREATE TABLE part2 PARTITION OF parted FOR VALUES IN (2); +]; + $detach_ddl .= q[ + CREATE TABLE part1_leaf PARTITION OF part1 DEFAULT; +] if $partition_type eq 'partitioned'; + $node_publisher->safe_psql('postgres', $detach_ddl); + $node_subscriber->safe_psql('postgres', $detach_ddl); + + $node_publisher->safe_psql( + 'postgres', q[ + CREATE PUBLICATION pub_detach FOR ALL TABLES + WITH (publish_via_partition_root = true); + CREATE PUBLICATION pub_detach_root FOR TABLE parted + WITH (publish_via_partition_root = true); + CREATE PUBLICATION pub_detach_except FOR ALL TABLES EXCEPT (TABLE parted) + WITH (publish_via_partition_root = true); +]); + $node_subscriber->safe_psql('postgres', + "CREATE SUBSCRIPTION sub_detach CONNECTION '$publisher_connstr' PUBLICATION pub_detach" + ); + $node_subscriber->wait_for_subscription_sync($node_publisher, + 'sub_detach'); + + is( $node_subscriber->safe_psql( + 'postgres', q[ + SELECT srrelid::regclass FROM pg_subscription_rel + WHERE srsubid = (SELECT oid FROM pg_subscription + WHERE subname = 'sub_detach')]), + 'parted', + 'only the root is registered in the subscription while attached'); + + $node_publisher->safe_psql('postgres', + 'INSERT INTO parted VALUES (1, 1), (2, 1)'); + $node_publisher->wait_for_catchup('sub_detach'); + + is( $node_subscriber->safe_psql( + 'postgres', 'SELECT * FROM parted ORDER BY a, b'), + "1|1\n2|1", + 'both partitions replicate while attached'); + + # Leave part1 pending detach: a prepared transaction holds the snapshot the + # detach waits for, so the wait ends in the lock timeout. + $node_publisher->safe_psql('postgres', + q[BEGIN; SELECT count(*) FROM parted; PREPARE TRANSACTION 'holder';]); + { + local $ENV{PGOPTIONS} = '-c lock_timeout=1s'; + $node_publisher->psql( + 'postgres', + 'ALTER TABLE parted DETACH PARTITION part1 CONCURRENTLY', + on_error_stop => 0); + } + $node_publisher->safe_psql('postgres', q[ROLLBACK PREPARED 'holder']); + + is( $node_publisher->safe_psql( + 'postgres', q[ + SELECT inhdetachpending FROM pg_inherits + WHERE inhrelid = 'part1'::regclass]), + 't', + 'the partition is marked as detaching'); + + is( $node_publisher->safe_psql( + 'postgres', q[ + SELECT relid::regclass::text + FROM pg_get_publication_tables('pub_detach') ORDER BY 1]), + "part1\nparted", + 'ALL TABLES includes the partition pending detach as a separate table' + ); + is( $node_publisher->safe_psql( + 'postgres', q[ + SELECT relid::regclass + FROM pg_get_publication_tables(ARRAY['pub_detach'], 'part1'::regclass)]), + 'part1', + 'targeted publication lookup includes the partition pending detach'); + is( $node_publisher->safe_psql( + 'postgres', q[ + SELECT count(*) + FROM pg_get_publication_tables(ARRAY['pub_detach_root'], 'part1'::regclass)] + ), + '0', + 'publishing only the former root does not include the partition pending detach' + ); + is( $node_publisher->safe_psql( + 'postgres', q[ + SELECT relid::regclass::text + FROM pg_get_publication_tables('pub_detach_except') ORDER BY 1]), + 'part1', + 'excluding the former root does not exclude the partition pending detach' + ); + is( $node_publisher->safe_psql( + 'postgres', q[ + SELECT relid::regclass + FROM pg_get_publication_tables(ARRAY['pub_detach_except'], 'part1'::regclass)] + ), + 'part1', + '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)' + ); + + # The part1 change is sent using part1's identity, but the subscriber skips + # it because only parted is registered in pg_subscription_rel, until the + # refresh below. The part2 change is applied via parted as before. + $node_publisher->safe_psql( + 'postgres', q[ + INSERT INTO part1 VALUES (1, 2); + INSERT INTO parted VALUES (2, 2); +]); + $node_publisher->wait_for_catchup('sub_detach'); + + is( $node_subscriber->safe_psql( + 'postgres', 'SELECT * FROM parted ORDER BY a, b'), + "1|1\n2|1\n2|2", + 'changes to the partition pending detach are skipped before refresh'); + + # Refresh must discover part1 even before FINALIZE. Do not copy existing + # rows, some of which were already replicated via parted before the detach. + $node_subscriber->safe_psql('postgres', + 'ALTER SUBSCRIPTION sub_detach REFRESH PUBLICATION WITH (copy_data = false)' + ); + is( $node_subscriber->safe_psql( + 'postgres', q[ + SELECT srrelid::regclass::text FROM pg_subscription_rel + WHERE srsubid = (SELECT oid FROM pg_subscription + WHERE subname = 'sub_detach') ORDER BY 1]), + "part1\nparted", + 'refresh registers the partition pending detach separately'); + $node_publisher->safe_psql('postgres', + 'INSERT INTO part1 VALUES (1, 3); INSERT INTO parted VALUES (2, 3)'); + $node_publisher->wait_for_catchup('sub_detach'); + is( $node_subscriber->safe_psql( + 'postgres', 'SELECT * FROM parted ORDER BY a, b'), + "1|1\n1|3\n2|1\n2|2\n2|3", + 'new changes to the partition pending detach are applied after refresh' + ); + + # The initial sync of a new subscription also sees part1 as a table of its + # own. Subscribe from another database, so that copying part1's rows does + # not clash with what sub_detach has already applied. + $node_subscriber->safe_psql('postgres', 'CREATE DATABASE detach_except'); + $node_subscriber->safe_psql('detach_except', $detach_ddl); + $node_subscriber->safe_psql('detach_except', + "CREATE SUBSCRIPTION sub_detach_except CONNECTION '$publisher_connstr' PUBLICATION pub_detach_except" + ); + $node_subscriber->wait_for_subscription_sync($node_publisher, + 'sub_detach_except', 'detach_except'); + is( $node_subscriber->safe_psql( + 'detach_except', + 'SELECT srrelid::regclass FROM pg_subscription_rel'), + 'part1', + 'a subscription excluding the former root registers the partition pending detach' + ); + is( $node_subscriber->safe_psql( + 'detach_except', 'SELECT * FROM parted ORDER BY a, b'), + "1|1\n1|2\n1|3", + 'initial sync copies the partition pending detach'); + $node_subscriber->safe_psql('detach_except', + 'DROP SUBSCRIPTION sub_detach_except'); + $node_subscriber->safe_psql('postgres', 'DROP DATABASE detach_except'); + + # Drop replication state and the tables, as the tests below re-use the nodes. + $node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION sub_detach"); + $node_publisher->safe_psql('postgres', + "DROP PUBLICATION pub_detach, pub_detach_root, pub_detach_except"); + $node_publisher->safe_psql('postgres', "DROP TABLE parted, part1"); + $node_subscriber->safe_psql('postgres', "DROP TABLE parted"); +} + +$node_publisher->stop('fast'); +$node_subscriber->stop('fast'); + + # Handling of temporary and unlogged tables with FOR ALL TABLES publications # If a FOR ALL TABLES publication exists, temporary and unlogged -- 2.54.0 (Apple Git-157)
From 0e07580c1533f4762ff3130464cc4218aeb54536 Mon Sep 17 00:00:00 2001 From: Nisha Moond <[email protected]> Date: Fri, 13 Nov 2026 11:30:44 +0530 Subject: [PATCH v3 3/3] 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 <[email protected]> Author: Mikhail Nikalayeu <[email protected]> Reported-by: shveta malik <[email protected]> Reviewed-by: Nisha Moond <[email protected]> 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 b6700929459..2c2006ff062 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -22453,6 +22453,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; <waiting ...> +step s1cancel: SELECT pg_cancel_backend(pid) FROM d3_pid; <waiting ...> +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; <waiting ...> +step s1cancel: SELECT pg_cancel_backend(pid) FROM d3_pid; <waiting ...> +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; <waiting ...> +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 f81e7fbb9d4..6e6995f2c18 100644 --- a/src/test/subscription/t/100_bugs.pl +++ b/src/test/subscription/t/100_bugs.pl @@ -200,14 +200,13 @@ 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)' - ); - # The part1 change is sent using part1's identity, but the subscriber skips # it because only parted is registered in pg_subscription_rel, until the # refresh below. The part2 change is applied via parted as before. + # + # 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.54.0 (Apple Git-157)
