Hello, everyone! (Added CC to authors of patches that introduced the issues described below, sorry if it is duplicate for you).
I was working on stress-suite and discovered a few crashes caused mainly by the same issue. DETACH PARTITION ... CONCURRENTLY marks the partition and commits, and only then waits. If that wait is interrupted the mark remains. Only PARTITION ... FINALIZE clears it. Reaching this needs no race, and the state outlives the command that made it. In that state pg_class still says relispartition, while get_partition_ancestors() already reports nothing. Three places read the former as meaning the latter is not empty and ask for its last element, which causes an assert() or NPE: - get_rel_sync_entry() - getIdentitySequence() - RelationBuildPublicationDesc() The last one is a v19 regression, so I think it may be added to the open items list. Patches are attached, one for each place; each includes a test that reproduces the crash. Best regards, Mikhail.
From a605ddca2e05d1df2e21702556c73085d5dc4eee Mon Sep 17 00:00:00 2001 From: nkey <[email protected]> Date: Thu, 13 Aug 2026 11:11:52 +0200 Subject: [PATCH v1 1/3] Fix crash decoding a change to a partition pending detach 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. get_rel_sync_entry() was not ready for it: it read relispartition as meaning that the ancestor list is not empty, and asked for its last element, which is an assertion failure, and a NULL pointer dereference without assertions. Decoding any change to such a partition for a FOR ALL TABLES publication gets there, and as the change is never confirmed, a subscription turns that into a crash loop. Fetch the ancestors once, up front, and narrow am_partition to a partition that still has one. It is then published in its own right, which is how it is published once the detach completes. The crash became reachable in v14, where 71f4c8c6f74b introduced the state, and fd366065e06a widened it from a publication with publish_via_partition_root to any FOR ALL TABLES one. Author: Mikhail Nikalayeu <[email protected]> Reviewed-by: XXX Discussion: XXX Backpatch-through: 14 --- src/backend/replication/pgoutput/pgoutput.c | 17 +++- src/test/subscription/t/100_bugs.pl | 90 +++++++++++++++++++++ 2 files changed, 105 insertions(+), 2 deletions(-) diff --git a/src/backend/replication/pgoutput/pgoutput.c b/src/backend/replication/pgoutput/pgoutput.c index 0afdb1432ca..181c7f2fd6a 100644 --- a/src/backend/replication/pgoutput/pgoutput.c +++ b/src/backend/replication/pgoutput/pgoutput.c @@ -2104,6 +2104,20 @@ get_rel_sync_entry(PGOutputData *data, Relation relation) bool am_partition = get_rel_relispartition(relid); char relkind = get_rel_relkind(relid); List *rel_publications = NIL; + List *ancestors = NIL; + + /* + * A partition whose concurrent detach has been committed but not + * finalized reports no ancestors, even though relispartition is + * still set. It is handled below like the standalone table it has + * effectively become, which is also how it is handled once the detach + * completes. + */ + if (am_partition) + { + ancestors = get_partition_ancestors(relid); + am_partition = (ancestors != NIL); + } /* Reload publications if needed before use. */ if (!publications_valid) @@ -2211,7 +2225,6 @@ get_rel_sync_entry(PGOutputData *data, Relation relation) if (am_partition) { - List *ancestors = get_partition_ancestors(relid); Oid last_ancestor_relid = llast_oid(ancestors); /* @@ -2264,7 +2277,6 @@ get_rel_sync_entry(PGOutputData *data, Relation relation) { Oid ancestor; int level; - List *ancestors = get_partition_ancestors(relid); ancestor = GetTopMostAncestorInPublication(pub->oid, ancestors, @@ -2362,6 +2374,7 @@ get_rel_sync_entry(PGOutputData *data, Relation relation) list_free(pubids); list_free(schemaPubids); list_free(rel_publications); + list_free(ancestors); entry->replicate_valid = true; } diff --git a/src/test/subscription/t/100_bugs.pl b/src/test/subscription/t/100_bugs.pl index 335efd86bca..736f997b425 100644 --- a/src/test/subscription/t/100_bugs.pl +++ b/src/test/subscription/t/100_bugs.pl @@ -79,6 +79,96 @@ $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, and get_rel_sync_entry() +# crashed on that; for a subscription that is a crash loop. 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(); + +my $detach_ddl = q[ + CREATE TABLE parted (a int, b int) PARTITION BY LIST (a); + CREATE TABLE part1 PARTITION OF parted FOR VALUES IN (1); + CREATE TABLE part2 PARTITION OF parted FOR VALUES IN (2); +]; +$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); +]); +$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'); + +$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'); + +# Changes nothing; drops the walsender's cached mapping. +$node_publisher->safe_psql('postgres', + 'ALTER PUBLICATION pub_detach SET (publish_via_partition_root = true)'); + +# This is what crashed. The part1 change is not replicated -- the publication +# lists no partitions -- so the change after it is what shows decoding got by. +$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", + 'replication got past the partition pending detach'); + +# 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"); +$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.43.0
From a318d3726c6f2e7a6c03dd9c2b70f5c5d3526f1e Mon Sep 17 00:00:00 2001 From: nkey <[email protected]> Date: Thu, 13 Aug 2026 11:18:37 +0200 Subject: [PATCH v1 2/3] Fix crash inserting into a partition pending detach with an identity column 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: XXX Discussion: XXX Backpatch-through: 17 --- src/backend/catalog/pg_depend.c | 19 +++++++++++--- .../detach-partition-concurrently-3.out | 26 +++++++++++++++++++ .../detach-partition-concurrently-3.spec | 11 ++++++++ 3 files changed, 53 insertions(+), 3 deletions(-) diff --git a/src/backend/catalog/pg_depend.c b/src/backend/catalog/pg_depend.c index 9a7a401aced..b89cbc03bbe 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 in a single scan of pg_inherits, 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 f23f46ad89b..a4a78eb5bdf 100644 --- a/src/test/isolation/expected/detach-partition-concurrently-3.out +++ b/src/test/isolation/expected/detach-partition-concurrently-3.out @@ -106,6 +106,32 @@ t step s1c: COMMIT; step s1insertpart: INSERT INTO d3_listp1 VALUES (1); +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 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 31aa3080daf..29bf352e470 100644 --- a/src/test/isolation/specs/detach-partition-concurrently-3.spec +++ b/src/test/isolation/specs/detach-partition-concurrently-3.spec @@ -14,10 +14,14 @@ 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); } teardown { DROP TABLE IF EXISTS d3_listp, d3_listp1, d3_listp2, d3_pid; + DROP TABLE IF EXISTS d3_idp, d3_idp1; } session s1 @@ -31,6 +35,8 @@ step s1c { COMMIT; } step s1alter { ALTER TABLE d3_listp1 ALTER a DROP NOT NULL; } step s1insert { INSERT INTO d3_listp VALUES (1); } step s1insertpart { INSERT INTO d3_listp1 VALUES (1); } +step s1sid { SELECT * FROM d3_idp; } +step s1insertidpart { INSERT INTO d3_idp1 (a) VALUES (1) RETURNING b; } step s1drop { DROP TABLE d3_listp; } step s1droppart { DROP TABLE d3_listp1; } step s1trunc { TRUNCATE TABLE d3_listp; } @@ -44,6 +50,7 @@ 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 s2detachfinal { ALTER TABLE d3_listp DETACH PARTITION d3_listp1 FINALIZE; } step s2drop { DROP TABLE d3_listp1; } step s2commit { COMMIT; } @@ -55,6 +62,10 @@ permutation s2snitch s1b s1s s2detach s1cancel(s2detach) s1insert s1c permutation s2snitch s1brr s1s s2detach s1cancel(s2detach) s1insert s1c s1spart permutation s2snitch s1b s1s s2detach s1cancel(s2detach) s1c s1insertpart +# The sequence behind an identity column lives in the topmost ancestor, and a +# partition pending detach has none. +permutation s2snitch s1b s1sid s2detachid s1cancel(s2detachid) s1c s1insertidpart + # 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 -- 2.43.0
From 2c769c4c11a8306cc0dd1f79d156d032aaeb9fbe Mon Sep 17 00:00:00 2001 From: nkey <[email protected]> Date: Thu, 13 Aug 2026 11:22:01 +0200 Subject: [PATCH v1 3/3] Fix crash on UPDATE or DELETE of a partition pending detach 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. RelationBuildPublicationDesc() was not ready for it. It read relispartition as meaning that the ancestor list is not empty, and asked for its last element to evaluate the EXCEPT clause there, which is an assertion failure, and a NULL pointer dereference without assertions. CheckCmdReplicaIdentity() calls it for every UPDATE and DELETE of a publishable relation, so no publication has to exist for this: a plain UPDATE takes the cluster down. Evaluate the exclusion on the relation itself when there are no ancestors, as is_table_publishable_in_publication() already does for the same clause. Oversight in fd366065e06a, which added the exclusion. Author: Mikhail Nikalayeu <[email protected]> Reviewed-by: XXX Discussion: XXX Backpatch-through: 19, where it was introduced --- src/backend/utils/cache/relcache.c | 9 +++- .../detach-partition-concurrently-3.out | 44 +++++++++++++++++++ .../detach-partition-concurrently-3.spec | 7 +++ 3 files changed, 59 insertions(+), 1 deletion(-) diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index 9abbaeab4a9..d03e5e8b79f 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -5855,7 +5855,14 @@ RelationBuildPublicationDesc(Relation relation, PublicationDesc *pubdesc) /* Add publications that the ancestors are in too. */ ancestors = get_partition_ancestors(relid); - last_ancestor_relid = llast_oid(ancestors); + + /* + * A partition whose concurrent detach has been committed but not + * finalized reports no ancestors, even though relispartition is still + * set. Fall back to the partition itself, which is what it will be + * once the detach completes. + */ + last_ancestor_relid = ancestors != NIL ? llast_oid(ancestors) : relid; foreach(lc, ancestors) { diff --git a/src/test/isolation/expected/detach-partition-concurrently-3.out b/src/test/isolation/expected/detach-partition-concurrently-3.out index a4a78eb5bdf..1f2be58b791 100644 --- a/src/test/isolation/expected/detach-partition-concurrently-3.out +++ b/src/test/isolation/expected/detach-partition-concurrently-3.out @@ -132,6 +132,50 @@ b (1 row) +starting permutation: s2snitch s1b s1s s2detach s1cancel s1c s1updpart +step s2snitch: INSERT INTO d3_pid SELECT pg_backend_pid(); +step s1b: BEGIN; +step s1s: SELECT * FROM d3_listp; +a +- +1 +(1 row) + +step s2detach: ALTER TABLE d3_listp DETACH PARTITION d3_listp1 CONCURRENTLY; <waiting ...> +step s1cancel: SELECT pg_cancel_backend(pid) FROM d3_pid; <waiting ...> +step s2detach: <... completed> +ERROR: canceling statement due to user request +step s1cancel: <... completed> +pg_cancel_backend +----------------- +t +(1 row) + +step s1c: COMMIT; +step s1updpart: UPDATE d3_listp1 SET a = 1; + +starting permutation: s2snitch s1b s1s s2detach s1cancel s1c s1delpart +step s2snitch: INSERT INTO d3_pid SELECT pg_backend_pid(); +step s1b: BEGIN; +step s1s: SELECT * FROM d3_listp; +a +- +1 +(1 row) + +step s2detach: ALTER TABLE d3_listp DETACH PARTITION d3_listp1 CONCURRENTLY; <waiting ...> +step s1cancel: SELECT pg_cancel_backend(pid) FROM d3_pid; <waiting ...> +step s2detach: <... completed> +ERROR: canceling statement due to user request +step s1cancel: <... completed> +pg_cancel_backend +----------------- +t +(1 row) + +step s1c: COMMIT; +step s1delpart: DELETE FROM d3_listp1; + 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 29bf352e470..ab64bc81ce8 100644 --- a/src/test/isolation/specs/detach-partition-concurrently-3.spec +++ b/src/test/isolation/specs/detach-partition-concurrently-3.spec @@ -37,6 +37,8 @@ step s1insert { INSERT INTO d3_listp VALUES (1); } step s1insertpart { INSERT INTO d3_listp1 VALUES (1); } step s1sid { SELECT * FROM d3_idp; } step s1insertidpart { INSERT INTO d3_idp1 (a) VALUES (1) RETURNING b; } +step s1updpart { UPDATE d3_listp1 SET a = 1; } +step s1delpart { DELETE FROM d3_listp1; } step s1drop { DROP TABLE d3_listp; } step s1droppart { DROP TABLE d3_listp1; } step s1trunc { TRUNCATE TABLE d3_listp; } @@ -66,6 +68,11 @@ permutation s2snitch s1b s1s s2detach s1cancel(s2detach) s1c s1insertpart # partition pending detach has none. permutation s2snitch s1b s1sid s2detachid s1cancel(s2detachid) s1c s1insertidpart +# Deciding whether the relation is published made the same assumption, and +# every UPDATE and DELETE of a publishable relation goes through it. +permutation s2snitch s1b s1s s2detach s1cancel(s2detach) s1c s1updpart +permutation s2snitch s1b s1s s2detach s1cancel(s2detach) s1c s1delpart + # 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 -- 2.43.0
