On Fri, Sep 11, 2026 at 10:41 AM shveta malik <[email protected]> wrote: > > On Thu, Sep 10, 2026 at 7:40 PM Nisha Moond <[email protected]> wrote: > > > > On Thu, Sep 10, 2026 at 5:37 PM shveta malik <[email protected]> wrote: > > > > ~~~ > > > > > > > > Given that we are treating a detach-pending partition as an individual > > > > table for publication decisions, the changes in patch-003 (pg19 > > > > regression) look correct to me. The changes in relcache.c correctly > > > > treat the detach-pending partition as an individual table. > > > > > > > > > > I haven't had a chance to reveiw 003 yet as 001 itself was > > > problematic. Will review it tomorrow. > > > > > > > Here is a rebased version of 003 that applies independently on both > > HEAD and pg19. This should make reviewing patch-003 separately easier. > > > > Okay, the fix looks good. I verified that it fixes the crash on HEAD. > But it would be good to change the fix slightly to be consistent with > 001. See the attached changes for reference. >
Thanks for sharing the patch. It looks good to me. I’ve updated the attached patch accordingly. One additional improvement: The error message when trying to add a detach-pending partition to the EXCEPT clause was misleading: ERROR: cannot specify relation "public.child" in the publication EXCEPT clause DETAIL: This operation is not supported for individual partitions. The relation is no longer a partition in this state; it is in detach-pending. I’ve updated the error message to make this clearer: ERROR: cannot specify relation "public.child" in the publication EXCEPT clause DETAIL: This operation is not supported for partitions with an incomplete detach. HINT: Use ALTER TABLE ... DETACH PARTITION ... FINALIZE to complete the pending detach operation. -- Thanks, Nisha
From 358667a451873bb323902df393304b06e5382418 Mon Sep 17 00:00:00 2001 From: Nisha Moond <[email protected]> Date: Fri, 11 Sep 2026 11:08:36 +0530 Subject: [PATCH v3_003] 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. Treat a partition reporting no ancestors as the standalone table it has effectively become -- which is also what it becomes once the detach completes, and what is_table_publishable_in_publication() already does for the same clause. The relation is then published by FOR ALL TABLES publications, but cannot be added to an EXCEPT clause until the detach is completed. Report that, with a hint to run DETACH PARTITION ... FINALIZE. Oversight in fd366065e06a, which added the exclusion. Author: shveta malik <[email protected]> Author: Mikhail Nikalayeu <[email protected]> Reviewed-by: XXX Discussion: XXX Backpatch-through: 19, where it was introduced --- src/backend/catalog/pg_publication.c | 9 +++ src/backend/utils/cache/relcache.c | 17 ++++- .../detach-partition-concurrently-3.out | 67 +++++++++++++++++++ .../detach-partition-concurrently-3.spec | 13 ++++ 4 files changed, 103 insertions(+), 3 deletions(-) diff --git a/src/backend/catalog/pg_publication.c b/src/backend/catalog/pg_publication.c index 12af7d15536..26c86f667b9 100644 --- a/src/backend/catalog/pg_publication.c +++ b/src/backend/catalog/pg_publication.c @@ -73,10 +73,19 @@ check_publication_add_relation(PublicationRelInfo *pri) /* If in EXCEPT clause, must be root partitioned table */ if (pri->except && targetrel->rd_rel->relispartition) + { + if (PartitionHasPendingDetach(RelationGetRelid(targetrel))) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg(errormsg, relname), + errdetail("This operation is not supported for partitions with an incomplete detach."), + errhint("Use ALTER TABLE ... DETACH PARTITION ... FINALIZE to complete the pending detach operation."))); + ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg(errormsg, relname), errdetail("This operation is not supported for individual partitions."))); + } /* Must be a regular or partitioned table */ if (RelationGetForm(targetrel)->relkind != RELKIND_RELATION && diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index f475d703977..d002db08f57 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -5810,6 +5810,7 @@ RelationBuildPublicationDesc(Relation relation, PublicationDesc *pubdesc) Oid schemaid; List *ancestors = NIL; Oid relid = RelationGetRelid(relation); + bool am_partition = relation->rd_rel->relispartition; /* * If not publishable, it publishes no actions. (pgoutput_change() will @@ -5846,12 +5847,22 @@ RelationBuildPublicationDesc(Relation relation, PublicationDesc *pubdesc) schemaid = RelationGetNamespace(relation); puboids = list_concat_unique_oid(puboids, GetSchemaPublications(schemaid)); - if (relation->rd_rel->relispartition) + /* + * 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); + } + + if (am_partition) { Oid last_ancestor_relid; - /* Add publications that the ancestors are in too. */ - ancestors = get_partition_ancestors(relid); last_ancestor_relid = llast_oid(ancestors); 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 f23f46ad89b..19a9aeb062f 100644 --- a/src/test/isolation/expected/detach-partition-concurrently-3.out +++ b/src/test/isolation/expected/detach-partition-concurrently-3.out @@ -106,6 +106,73 @@ t step s1c: COMMIT; step s1insertpart: INSERT INTO d3_listp1 VALUES (1); +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 s2detach s1cancel s1c s1exceptpart +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 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 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..567cc27a3d8 100644 --- a/src/test/isolation/specs/detach-partition-concurrently-3.spec +++ b/src/test/isolation/specs/detach-partition-concurrently-3.spec @@ -31,6 +31,9 @@ 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 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 s1drop { DROP TABLE d3_listp; } step s1droppart { DROP TABLE d3_listp1; } step s1trunc { TRUNCATE TABLE d3_listp; } @@ -55,6 +58,16 @@ 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 +# 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 + +# Such a partition is published as a standalone table, but it cannot be named +# in an EXCEPT clause until the detach is completed; check that the error says +# so rather than blaming individual partitions. +permutation s2snitch s1b s1s s2detach s1cancel(s2detach) s1c s1exceptpart + # 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.50.1 (Apple Git-155)
