Hi Surya, Thanks for testing each patch on its own. v4 is attached.
1. With only 0001 the index stays invalid after FINALIZE, which is safe. A plain DETACH on master does the same, so I would fix that separately. I don't think it blocks 0001 and 0002. 2. v4-0003 now lets ALTER TABLE on the parent reach p1, and the spec covers it. pg_publication_tables leaves p1 out again, like pg_partition_tree(). This also fixes your DROP COLUMN case. 3. For the stable branches I would rather not refuse DDL. A table already hit by this needs DDL on the parent to repair it, and that would be refused too. But I am happy to go with what Michael and Alvaro prefer. Thanks, Shihao On Mon, Nov 09, 2026 07:51 PM, surya poondla <[email protected]> wrote: > Hi Shihao, > > Thanks for the findings and the patch. > I tested all three v3 patches together, and 0001 by itself as this could > be backported. > make check and the isolation tests pass in both cases. > > With only 0001, if p1's index isn't attached yet, the parent index stays > invalid even after FINALIZE: > > create table t (a int) partition by list (a); > create table p1 partition of t for values in (1); > create table p2 partition of t for values in (2); > create unique index ti on only t (a); > create unique index p1_a on p1 (a); > create unique index p2_a on p2 (a); > -- p1 pending detach > alter index ti attach partition p2_a; > alter index ti attach partition p1_a; > ERROR: cannot attach index "p1_a" as a partition of index "ti" > alter table t detach partition p1 finalize; -- ti still invalid > > ATExecAttachPartitionIdx() still skips p1, and FINALIZE doesn't > revalidate. > Re-attaching p2_a fixes it, but nobody would guess that. > Maybe FINALIZE should revalidate the parent's indexes? > Michael, this affects pushing 0001/0002 alone. > > As you noted, without 0003 FINALIZE fails after RENAME or ADD COLUMN > on t, so that stays broken on the back branches. DROP COLUMN is worse as > FINALIZE succeeds but p1 keeps the column as inherited: > > create table t (a int, b int) partition by list (a); > create table p1 partition of t for values in (1); > create table p2 partition of t for values in (2); > -- p1 pending detach > alter table t drop column b; > alter table t detach partition p1 finalize; > alter table p1 drop column b; > ERROR: cannot drop inherited column "b" > create table t9 (a int, b int) partition by list (a); > alter table t9 attach partition p1 for values in (1); > TRAP: failed Assert("child_att->attinhcount == 1") > > Alvaro's idea of rejecting DDL while a detach is pending looks like what > the back branches need. > > With 0003, SET DEFAULT, SET STATISTICS and ALTER COLUMN TYPE on t now > fail with "cannot alter partition "p1" with an incomplete detach", while > ADD COLUMN etc. go through to p1. If that's intended, the spec > should cover it. Also pg_publication_tables now lists p1 but > pg_partition_tree() doesn't. > > Regards, > Surya Poondla >
From d6df83ed6fe5f7966bbfa08862569fe8cd5ce60e Mon Sep 17 00:00:00 2001 From: shihao zhong <[email protected]> Date: Tue, 3 Nov 2026 21:22:14 -0500 Subject: [PATCH 1/3] Count partitions pending detach when validating a partitioned index Otherwise the parent index could be marked valid while another partition had no index, letting duplicate keys in. Backpatch-through: 15 --- src/backend/commands/tablecmds.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index aa89c9a4637..45171356ad1 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -23259,8 +23259,11 @@ validatePartitionedIndex(Relation partedIdx, Relation partedTbl) /* * If we found as many inherited indexes as the partitioned table has * partitions, we're good; update pg_index to set indisvalid. + * + * Partitions pending detach must be counted too, because their indexes + * remain attached to this index until the detach is finalized. */ - if (tuples == RelationGetPartitionDesc(partedTbl, true)->nparts) + if (tuples == RelationGetPartitionDesc(partedTbl, false)->nparts) { Relation idxRel; HeapTuple indTup; -- 2.37.1 (Apple Git-137.1)
From 1cad0e414c013f193869ed832dc14739c142167a Mon Sep 17 00:00:00 2001 From: shihao zhong <[email protected]> Date: Tue, 3 Nov 2026 21:22:14 -0500 Subject: [PATCH 2/3] Test partitioned index validity while a partition is pending detach Backpatch-through: 15 --- .../detach-partition-concurrently-3.out | 41 +++++++++++++++++++ .../detach-partition-concurrently-3.spec | 14 ++++++- 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/src/test/isolation/expected/detach-partition-concurrently-3.out b/src/test/isolation/expected/detach-partition-concurrently-3.out index c1705d80aa1..485068dbd81 100644 --- a/src/test/isolation/expected/detach-partition-concurrently-3.out +++ b/src/test/isolation/expected/detach-partition-concurrently-3.out @@ -529,3 +529,44 @@ step s2detachfinal: ALTER TABLE d3_listp DETACH PARTITION d3_listp1 FINALIZE; step s1insertpart: INSERT INTO d3_listp1 VALUES (1); <waiting ...> step s2commit: COMMIT; step s1insertpart: <... completed> + +starting permutation: s2snitch s1idx s1b s1s s2detach s1cancel s1c s1attach2 s1valid s1attach3 s1valid +step s2snitch: INSERT INTO d3_pid SELECT pg_backend_pid(); +step s1idx: CREATE TABLE d3_listp3 PARTITION OF d3_listp FOR VALUES IN (3); + CREATE UNIQUE INDEX d3_idx ON ONLY d3_listp (a); + CREATE UNIQUE INDEX d3_listp1_a ON d3_listp1 (a); + CREATE UNIQUE INDEX d3_listp2_a ON d3_listp2 (a); + ALTER INDEX d3_idx ATTACH PARTITION d3_listp1_a; +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 s1attach2: ALTER INDEX d3_idx ATTACH PARTITION d3_listp2_a; +step s1valid: SELECT indisvalid FROM pg_index WHERE indexrelid = 'd3_idx'::regclass; +indisvalid +---------- +f +(1 row) + +step s1attach3: CREATE UNIQUE INDEX d3_listp3_a ON d3_listp3 (a); + ALTER INDEX d3_idx ATTACH PARTITION d3_listp3_a; +step s1valid: SELECT indisvalid FROM pg_index WHERE indexrelid = 'd3_idx'::regclass; +indisvalid +---------- +t +(1 row) + diff --git a/src/test/isolation/specs/detach-partition-concurrently-3.spec b/src/test/isolation/specs/detach-partition-concurrently-3.spec index 6d359756e31..7b7b6fee1d6 100644 --- a/src/test/isolation/specs/detach-partition-concurrently-3.spec +++ b/src/test/isolation/specs/detach-partition-concurrently-3.spec @@ -17,7 +17,7 @@ setup } teardown { - DROP TABLE IF EXISTS d3_listp, d3_listp1, d3_listp2, d3_pid; + DROP TABLE IF EXISTS d3_listp, d3_listp1, d3_listp2, d3_listp3, d3_pid; } session s1 @@ -41,6 +41,15 @@ step s1list { SELECT relname FROM pg_catalog.pg_class WHERE relname LIKE 'd3_listp%' ORDER BY 1; } step s1describe { SELECT 'd3_listp' AS root, * FROM pg_partition_tree('d3_listp') UNION ALL SELECT 'd3_listp1', * FROM pg_partition_tree('d3_listp1'); } +step s1idx { CREATE TABLE d3_listp3 PARTITION OF d3_listp FOR VALUES IN (3); + CREATE UNIQUE INDEX d3_idx ON ONLY d3_listp (a); + CREATE UNIQUE INDEX d3_listp1_a ON d3_listp1 (a); + CREATE UNIQUE INDEX d3_listp2_a ON d3_listp2 (a); + ALTER INDEX d3_idx ATTACH PARTITION d3_listp1_a; } +step s1attach2 { ALTER INDEX d3_idx ATTACH PARTITION d3_listp2_a; } +step s1attach3 { CREATE UNIQUE INDEX d3_listp3_a ON d3_listp3 (a); + ALTER INDEX d3_idx ATTACH PARTITION d3_listp3_a; } +step s1valid { SELECT indisvalid FROM pg_index WHERE indexrelid = 'd3_idx'::regclass; } session s2 step s2begin { BEGIN; } @@ -94,3 +103,6 @@ permutation s2snitch s1b s1s s2detach s1cancel(s2detach) s1c s1b s1spart s2detac permutation s2snitch s1b s1s s2detach s1cancel(s2detach) s1c s2begin s2detachfinal s2commit permutation s2snitch s1b s1s s2detach s1cancel(s2detach) s1c s2begin s2detachfinal s1spart s2commit permutation s2snitch s1b s1s s2detach s1cancel(s2detach) s1c s2begin s2detachfinal s1insertpart s2commit + +# The index of a partition pending detach must not count toward validity. +permutation s2snitch s1idx s1b s1s s2detach s1cancel(s2detach) s1c s1attach2 s1valid s1attach3 s1valid -- 2.37.1 (Apple Git-137.1)
From d955f7c59318057d4e37f61b6ed646571ec02a30 Mon Sep 17 00:00:00 2001 From: shihao zhong <[email protected]> Date: Sat, 7 Nov 2026 22:08:46 -0500 Subject: [PATCH 3/3] Let DDL see partitions pending detach Only queries and publications now omit such partitions, so DDL on the parent reaches them and FINALIZE works. --- src/backend/catalog/heap.c | 2 +- src/backend/catalog/pg_inherits.c | 26 ++++++++--- src/backend/catalog/pg_publication.c | 8 +++- src/backend/commands/analyze.c | 3 +- src/backend/commands/copyto.c | 3 +- src/backend/commands/indexcmds.c | 4 +- src/backend/commands/tablecmds.c | 27 +++++++----- src/backend/commands/trigger.c | 9 ++-- src/backend/partitioning/partbounds.c | 6 +-- src/backend/partitioning/partdesc.c | 15 ++++++- src/backend/replication/logical/worker.c | 6 +-- src/backend/utils/adt/partitionfuncs.c | 3 +- src/include/catalog/pg_inherits.h | 2 + src/include/partitioning/partdesc.h | 2 +- .../detach-partition-concurrently-3.out | 44 +++++++++++++++++++ .../detach-partition-concurrently-3.spec | 14 ++++++ 16 files changed, 134 insertions(+), 40 deletions(-) diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index a2c76b22fb0..2fb13a8e2a0 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -4455,7 +4455,7 @@ StorePartitionBound(Relation rel, Relation parent, PartitionBoundSpec *bound) * removed. */ defaultPartOid = - get_default_oid_from_partdesc(RelationGetPartitionDesc(parent, true)); + get_default_oid_from_partdesc(RelationGetPartitionDesc(parent)); if (OidIsValid(defaultPartOid)) CacheInvalidateRelcacheByRelid(defaultPartOid); diff --git a/src/backend/catalog/pg_inherits.c b/src/backend/catalog/pg_inherits.c index 23fb7886cfb..51c5c3f0e3e 100644 --- a/src/backend/catalog/pg_inherits.c +++ b/src/backend/catalog/pg_inherits.c @@ -52,13 +52,13 @@ typedef struct SeenRelsEntry * then no locks are acquired, but caller must beware of race conditions * against possible DROPs of child relations. * - * Partitions marked as being detached are omitted; see - * find_inheritance_children_extended for details. + * Partitions marked as being detached are included. Only queries omit + * them; see find_inheritance_children_extended. */ List * find_inheritance_children(Oid parentrelId, LOCKMODE lockmode) { - return find_inheritance_children_extended(parentrelId, true, lockmode, + return find_inheritance_children_extended(parentrelId, false, lockmode, NULL, NULL); } @@ -249,11 +249,22 @@ find_inheritance_children_extended(Oid parentrelId, bool omit_detached, * then no locks are acquired, but caller must beware of race conditions * against possible DROPs of child relations. * - * NB - No current callers of this routine are interested in children being - * concurrently detached, so there's no provision to include them. + * Partitions pending detach are included, as in find_inheritance_children. */ List * find_all_inheritors(Oid parentrelId, LOCKMODE lockmode, List **numparents) +{ + return find_all_inheritors_extended(parentrelId, false, lockmode, + numparents); +} + +/* + * find_all_inheritors_extended - + * As find_all_inheritors, optionally omitting partitions pending detach. + */ +List * +find_all_inheritors_extended(Oid parentrelId, bool omit_detached, + LOCKMODE lockmode, List **numparents) { /* hash table for O(1) rel_oid -> rel_numparents cell lookup */ HTAB *seen_rels; @@ -290,7 +301,10 @@ find_all_inheritors(Oid parentrelId, LOCKMODE lockmode, List **numparents) ListCell *lc; /* Get the direct children of this rel */ - currentchildren = find_inheritance_children(currentrel, lockmode); + currentchildren = find_inheritance_children_extended(currentrel, + omit_detached, + lockmode, + NULL, NULL); /* * Add to the queue only those children not already seen. This avoids diff --git a/src/backend/catalog/pg_publication.c b/src/backend/catalog/pg_publication.c index f11ea8839ca..9f732cc6dcb 100644 --- a/src/backend/catalog/pg_publication.c +++ b/src/backend/catalog/pg_publication.c @@ -434,8 +434,12 @@ GetPubPartitionOptionRelations(List *result, PublicationPartOpt pub_partopt, if (get_rel_relkind(relid) == RELKIND_PARTITIONED_TABLE && pub_partopt != PUBLICATION_PART_ROOT) { - List *all_parts = find_all_inheritors(relid, NoLock, - NULL); + /* + * A partition pending detach is not published through its parent, so + * leave it out here. + */ + List *all_parts = find_all_inheritors_extended(relid, true, + NoLock, NULL); if (pub_partopt == PUBLICATION_PART_ALL) result = list_concat(result, all_parts); diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c index 518d7526255..d01e12a9159 100644 --- a/src/backend/commands/analyze.c +++ b/src/backend/commands/analyze.c @@ -1497,7 +1497,8 @@ acquire_inherited_sample_rows(Relation onerel, int elevel, * the children. */ tableOIDs = - find_all_inheritors(RelationGetRelid(onerel), AccessShareLock, NULL); + find_all_inheritors_extended(RelationGetRelid(onerel), true, + AccessShareLock, NULL); /* * Check that there's at least one descendant, else fail. This could diff --git a/src/backend/commands/copyto.c b/src/backend/commands/copyto.c index 9ca06ed3e18..cf0fc69c300 100644 --- a/src/backend/commands/copyto.c +++ b/src/backend/commands/copyto.c @@ -860,7 +860,8 @@ BeginCopyTo(ParseState *pstate, * Collect OIDs of relation containing data, so that later * DoCopyTo can copy the data from them. */ - children = find_all_inheritors(RelationGetRelid(rel), AccessShareLock, NULL); + children = find_all_inheritors_extended(RelationGetRelid(rel), true, + AccessShareLock, NULL); foreach_oid(child, children) { diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 892abff4db5..572b487c94d 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -1267,7 +1267,7 @@ DefineIndex(ParseState *pstate, */ if (partitioned && stmt->relation && !stmt->relation->inh) { - PartitionDesc pd = RelationGetPartitionDesc(rel, true); + PartitionDesc pd = RelationGetPartitionDesc(rel); if (pd->nparts != 0) flags |= INDEX_CREATE_INVALID; @@ -1337,7 +1337,7 @@ DefineIndex(ParseState *pstate, * * If we're called internally (no stmt->relation), recurse always. */ - partdesc = RelationGetPartitionDesc(rel, true); + partdesc = RelationGetPartitionDesc(rel); if ((!stmt->relation || stmt->relation->inh) && partdesc->nparts > 0) { int nparts = partdesc->nparts; diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 45171356ad1..9f07c05e583 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -1195,8 +1195,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId, * lock the partition so as to avoid a deadlock. */ defaultPartOid = - get_default_oid_from_partdesc(RelationGetPartitionDesc(parent, - true)); + get_default_oid_from_partdesc(RelationGetPartitionDesc(parent)); if (OidIsValid(defaultPartOid)) defaultRel = table_open(defaultPartOid, AccessExclusiveLock); @@ -1949,7 +1948,8 @@ ExecuteTruncate(TruncateStmt *stmt) ListCell *child; List *children; - children = find_all_inheritors(myrelid, lockmode, NULL); + children = find_all_inheritors_extended(myrelid, true, lockmode, + NULL); foreach(child, children) { @@ -4986,9 +4986,12 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd, /* * Disallow any ALTER TABLE other than ALTER TABLE DETACH FINALIZE on - * partitions that are pending detach. + * partitions that are pending detach. A command recursing from the + * parent must still reach such a partition, since it remains a partition + * until the detach is finalized. */ - if (rel->rd_rel->relispartition && + if (!recursing && + rel->rd_rel->relispartition && cmd->subtype != AT_DetachPartitionFinalize && PartitionHasPendingDetach(RelationGetRelid(rel))) ereport(ERROR, @@ -11643,7 +11646,7 @@ addFkRecurseReferenced(Constraint *fkconstraint, Relation rel, */ if (pkrel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) { - PartitionDesc pd = RelationGetPartitionDesc(pkrel, true); + PartitionDesc pd = RelationGetPartitionDesc(pkrel); for (int i = 0; i < pd->nparts; i++) { @@ -11814,7 +11817,7 @@ addFkRecurseReferencing(List **wqueue, Constraint *fkconstraint, Relation rel, } else if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) { - PartitionDesc pd = RelationGetPartitionDesc(rel, true); + PartitionDesc pd = RelationGetPartitionDesc(rel); Relation trigrel; /* @@ -21615,7 +21618,7 @@ QueuePartitionConstraintValidation(List **wqueue, Relation scanrel, } else if (scanrel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) { - PartitionDesc partdesc = RelationGetPartitionDesc(scanrel, true); + PartitionDesc partdesc = RelationGetPartitionDesc(scanrel); int i; for (i = 0; i < partdesc->nparts; i++) @@ -21676,7 +21679,7 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd, * new partition will change its partition constraint. */ defaultPartOid = - get_default_oid_from_partdesc(RelationGetPartitionDesc(rel, true)); + get_default_oid_from_partdesc(RelationGetPartitionDesc(rel)); if (OidIsValid(defaultPartOid)) LockRelationOid(defaultPartOid, AccessExclusiveLock); @@ -22361,7 +22364,7 @@ ATExecDetachPartition(List **wqueue, AlteredTableInfo *tab, Relation rel, * will change its partition constraint. */ defaultPartOid = - get_default_oid_from_partdesc(RelationGetPartitionDesc(rel, true)); + get_default_oid_from_partdesc(RelationGetPartitionDesc(rel)); if (OidIsValid(defaultPartOid)) { /* @@ -23089,7 +23092,7 @@ ATExecAttachPartitionIdx(List **wqueue, Relation parentIdx, RangeVar *name) RelationGetRelationName(partIdx)))); /* Make sure it indexes a partition of the other index's table */ - partDesc = RelationGetPartitionDesc(parentTbl, true); + partDesc = RelationGetPartitionDesc(parentTbl); found = false; for (i = 0; i < partDesc->nparts; i++) { @@ -23263,7 +23266,7 @@ validatePartitionedIndex(Relation partedIdx, Relation partedTbl) * Partitions pending detach must be counted too, because their indexes * remain attached to this index until the detach is finalized. */ - if (tuples == RelationGetPartitionDesc(partedTbl, false)->nparts) + if (tuples == RelationGetPartitionDesc(partedTbl)->nparts) { Relation idxRel; HeapTuple indTup; diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index 838eae31660..01beb028ab9 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -1171,7 +1171,7 @@ CreateTriggerFiringOn(const CreateTrigStmt *stmt, const char *queryString, */ if (partition_recurse) { - PartitionDesc partdesc = RelationGetPartitionDesc(rel, true); + PartitionDesc partdesc = RelationGetPartitionDesc(rel); int i; MemoryContext oldcxt, perChildCxt; @@ -1580,7 +1580,7 @@ renametrig(RenameStmt *stmt) /* ... and if it is partitioned, recurse to its partitions */ if (targetrel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) { - PartitionDesc partdesc = RelationGetPartitionDesc(targetrel, true); + PartitionDesc partdesc = RelationGetPartitionDesc(targetrel); for (int i = 0; i < partdesc->nparts; i++) { @@ -1726,8 +1726,7 @@ renametrig_partition(Relation tgrel, Oid partitionId, Oid parentTriggerOid, /* And if this relation is partitioned, recurse to its partitions */ if (partitionRel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) { - PartitionDesc partdesc = RelationGetPartitionDesc(partitionRel, - true); + PartitionDesc partdesc = RelationGetPartitionDesc(partitionRel); for (int i = 0; i < partdesc->nparts; i++) { @@ -1849,7 +1848,7 @@ EnableDisableTrigger(Relation rel, const char *tgname, Oid tgparent, rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE && (TRIGGER_FOR_ROW(oldtrig->tgtype))) { - PartitionDesc partdesc = RelationGetPartitionDesc(rel, true); + PartitionDesc partdesc = RelationGetPartitionDesc(rel); int i; for (i = 0; i < partdesc->nparts; i++) diff --git a/src/backend/partitioning/partbounds.c b/src/backend/partitioning/partbounds.c index f618f59f7a8..ff3f5988032 100644 --- a/src/backend/partitioning/partbounds.c +++ b/src/backend/partitioning/partbounds.c @@ -2885,7 +2885,7 @@ check_new_partition_bound(char *relname, Relation parent, PartitionBoundSpec *spec, ParseState *pstate) { PartitionKey key = RelationGetPartitionKey(parent); - PartitionDesc partdesc = RelationGetPartitionDesc(parent, false); + PartitionDesc partdesc = RelationGetPartitionDesc(parent); PartitionBoundInfo boundinfo = partdesc->boundinfo; int with = -1; bool overlap = false; @@ -4088,7 +4088,7 @@ get_qual_for_list(Relation parent, PartitionBoundSpec *spec) { int i; int ndatums = 0; - PartitionDesc pdesc = RelationGetPartitionDesc(parent, false); + PartitionDesc pdesc = RelationGetPartitionDesc(parent); PartitionBoundInfo boundinfo = pdesc->boundinfo; if (boundinfo) @@ -4288,7 +4288,7 @@ get_qual_for_range(Relation parent, PartitionBoundSpec *spec, if (spec->is_default) { List *or_expr_args = NIL; - PartitionDesc pdesc = RelationGetPartitionDesc(parent, false); + PartitionDesc pdesc = RelationGetPartitionDesc(parent); Oid *inhoids = pdesc->oids; int nparts = pdesc->nparts, k; diff --git a/src/backend/partitioning/partdesc.c b/src/backend/partitioning/partdesc.c index 4399fe9a3a0..64f45f1549a 100644 --- a/src/backend/partitioning/partdesc.c +++ b/src/backend/partitioning/partdesc.c @@ -46,6 +46,7 @@ typedef struct PartitionDirectoryEntry PartitionDesc pd; } PartitionDirectoryEntry; +static PartitionDesc get_partition_desc(Relation rel, bool omit_detached); static PartitionDesc RelationBuildPartitionDesc(Relation rel, bool omit_detached); @@ -68,7 +69,17 @@ static PartitionDesc RelationBuildPartitionDesc(Relation rel, * that the data doesn't become stale. */ PartitionDesc -RelationGetPartitionDesc(Relation rel, bool omit_detached) +RelationGetPartitionDesc(Relation rel) +{ + return get_partition_desc(rel, false); +} + +/* + * get_partition_desc -- workhorse for RelationGetPartitionDesc and + * PartitionDirectoryLookup. Only queries omit partitions pending detach. + */ +static PartitionDesc +get_partition_desc(Relation rel, bool omit_detached) { Assert(rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE); @@ -467,7 +478,7 @@ PartitionDirectoryLookup(PartitionDirectory pdir, Relation rel) */ RelationIncrementReferenceCount(rel); pde->rel = rel; - pde->pd = RelationGetPartitionDesc(rel, pdir->omit_detached); + pde->pd = get_partition_desc(rel, pdir->omit_detached); Assert(pde->pd != NULL); } return pde->pd; diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c index 95044620e68..ddbd5335b03 100644 --- a/src/backend/replication/logical/worker.c +++ b/src/backend/replication/logical/worker.c @@ -3769,9 +3769,9 @@ apply_handle_truncate(StringInfo s) if (rel->localrel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) { ListCell *child; - List *children = find_all_inheritors(rel->localreloid, - lockmode, - NULL); + List *children = find_all_inheritors_extended(rel->localreloid, + true, lockmode, + NULL); foreach(child, children) { diff --git a/src/backend/utils/adt/partitionfuncs.c b/src/backend/utils/adt/partitionfuncs.c index e9db027aa2e..5ce62388db0 100644 --- a/src/backend/utils/adt/partitionfuncs.c +++ b/src/backend/utils/adt/partitionfuncs.c @@ -85,7 +85,8 @@ pg_partition_tree(PG_FUNCTION_ARGS) * Find all members of inheritance set. We only need AccessShareLock * on the children for the partition information lookup. */ - partitions = find_all_inheritors(rootrelid, AccessShareLock, NULL); + partitions = find_all_inheritors_extended(rootrelid, true, + AccessShareLock, NULL); if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE) elog(ERROR, "return type must be a row type"); diff --git a/src/include/catalog/pg_inherits.h b/src/include/catalog/pg_inherits.h index cc874abaabb..694d39a1f22 100644 --- a/src/include/catalog/pg_inherits.h +++ b/src/include/catalog/pg_inherits.h @@ -58,6 +58,8 @@ extern List *find_inheritance_children_extended(Oid parentrelId, bool omit_detac extern List *find_all_inheritors(Oid parentrelId, LOCKMODE lockmode, List **numparents); +extern List *find_all_inheritors_extended(Oid parentrelId, bool omit_detached, + LOCKMODE lockmode, List **numparents); extern bool has_subclass(Oid relationId); extern bool has_superclass(Oid relationId); extern bool typeInheritsFrom(Oid subclassTypeId, Oid superclassTypeId); diff --git a/src/include/partitioning/partdesc.h b/src/include/partitioning/partdesc.h index f8698e87455..0f2ff8f663d 100644 --- a/src/include/partitioning/partdesc.h +++ b/src/include/partitioning/partdesc.h @@ -58,7 +58,7 @@ typedef struct PartitionDescData } PartitionDescData; -extern PartitionDesc RelationGetPartitionDesc(Relation rel, bool omit_detached); +extern PartitionDesc RelationGetPartitionDesc(Relation rel); extern PartitionDirectory CreatePartitionDirectory(MemoryContext mcxt, bool omit_detached); extern PartitionDesc PartitionDirectoryLookup(PartitionDirectory, Relation); diff --git a/src/test/isolation/expected/detach-partition-concurrently-3.out b/src/test/isolation/expected/detach-partition-concurrently-3.out index 485068dbd81..78385da7da0 100644 --- a/src/test/isolation/expected/detach-partition-concurrently-3.out +++ b/src/test/isolation/expected/detach-partition-concurrently-3.out @@ -570,3 +570,47 @@ indisvalid t (1 row) + +starting permutation: s2snitch s1b s1s s2detach s1cancel s1c s1addck s1setdef s1alter s1pubtables s2detachfinal s1p1ddl +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 s1addck: ALTER TABLE d3_listp ADD CHECK (a > 0); +step s1setdef: ALTER TABLE d3_listp ALTER a SET DEFAULT 1; +step s1alter: ALTER TABLE d3_listp1 ALTER a DROP NOT NULL; +ERROR: cannot alter partition "d3_listp1" with an incomplete detach +step s1pubtables: CREATE PUBLICATION pub_d3t FOR TABLE d3_listp; + SELECT tablename FROM pg_publication_tables + WHERE pubname = 'pub_d3t' ORDER BY 1; + DROP PUBLICATION pub_d3t; +tablename +--------- +d3_listp2 +(1 row) + +step s2detachfinal: ALTER TABLE d3_listp DETACH PARTITION d3_listp1 FINALIZE; +step s1p1ddl: SELECT (SELECT pg_get_expr(adbin, adrelid) FROM pg_attrdef + WHERE adrelid = 'd3_listp1'::regclass) AS def, + (SELECT count(*) FROM pg_constraint + WHERE conrelid = 'd3_listp1'::regclass AND contype = 'c') AS checks; +def|checks +---+------ + 1| 1 +(1 row) + diff --git a/src/test/isolation/specs/detach-partition-concurrently-3.spec b/src/test/isolation/specs/detach-partition-concurrently-3.spec index 7b7b6fee1d6..86efa5954b8 100644 --- a/src/test/isolation/specs/detach-partition-concurrently-3.spec +++ b/src/test/isolation/specs/detach-partition-concurrently-3.spec @@ -50,6 +50,16 @@ step s1attach2 { ALTER INDEX d3_idx ATTACH PARTITION d3_listp2_a; } step s1attach3 { CREATE UNIQUE INDEX d3_listp3_a ON d3_listp3 (a); ALTER INDEX d3_idx ATTACH PARTITION d3_listp3_a; } step s1valid { SELECT indisvalid FROM pg_index WHERE indexrelid = 'd3_idx'::regclass; } +step s1addck { ALTER TABLE d3_listp ADD CHECK (a > 0); } +step s1setdef { ALTER TABLE d3_listp ALTER a SET DEFAULT 1; } +step s1pubtables { CREATE PUBLICATION pub_d3t FOR TABLE d3_listp; + SELECT tablename FROM pg_publication_tables + WHERE pubname = 'pub_d3t' ORDER BY 1; + DROP PUBLICATION pub_d3t; } +step s1p1ddl { SELECT (SELECT pg_get_expr(adbin, adrelid) FROM pg_attrdef + WHERE adrelid = 'd3_listp1'::regclass) AS def, + (SELECT count(*) FROM pg_constraint + WHERE conrelid = 'd3_listp1'::regclass AND contype = 'c') AS checks; } session s2 step s2begin { BEGIN; } @@ -106,3 +116,7 @@ permutation s2snitch s1b s1s s2detach s1cancel(s2detach) s1c s2begin s2detachfin # The index of a partition pending detach must not count toward validity. permutation s2snitch s1idx s1b s1s s2detach s1cancel(s2detach) s1c s1attach2 s1valid s1attach3 s1valid + +# DDL on the parent must reach a partition pending detach, while DDL on +# the partition itself is still refused. Publications do not list it. +permutation s2snitch s1b s1s s2detach s1cancel(s2detach) s1c s1addck s1setdef s1alter s1pubtables s2detachfinal s1p1ddl -- 2.37.1 (Apple Git-137.1)
