From: ocean_li_996 <ocean_li_996@163.com>
To: Álvaro Herrera <alvherre@kurilemu.de>
Cc: mohen.lhy@alibaba-inc.com <mohen.lhy@alibaba-inc.com>
Cc: pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
Cc: robertmhaas@gmail.com
Subject: Re:Re: BUG #19070: issue with DETACH PARTITION CONCURRENTLY on a hash partition table
Date: Sat, 11 Oct 2025 01:14:49 +0800 (CST)
Message-ID: <17a8f848.b7f6.199cf1e3b5e.Coremail.ocean_li_996@163.com> (raw)
In-Reply-To: <202510101057.2czghujtfz33@alvherre.pgsql>
References: <202510101057.2czghujtfz33@alvherre.pgsql>
Hi, Álvaro
At 2025-10-10 19:03:08, "Álvaro Herrera" <alvherre@kurilemu.de> wrote:
>On 2025-Oct-10, ocean_li_996 wrote:
>
>> Since there has been no update in this thread for a few days, I
>> reached out to the primary committer for this functionality.
>
>Yes, this is buggy, and has been reported previously (#18371).
Thanks for your feedback. It seems I might have overlooked something.
At first, I assumed this was expected behavior, and after searching
the pgsql-hackers archives I did not find any related threads. I also
realize I haven’t checked pgsql-bugs.
>I'll fix
>it before the November minors, probably by removing the step of creating
>the constraints during detach. I think for stable branches I'll do that
>only for hash partitioning, to avoid breaking workloads that might
>depend on those constraints, but for branch master I'll do it for all
>partitioning strategies.
Based on the discussion in #18371 and my own investigation, I think the
proposed fix is appropriate. I’m attaching patches for both the stable
and master branches, FYI.
--
regards
Haiyang Li
Attachments:
[application/octet-stream] 0001_master_fix_detach_partition_concurrently_issue.patch (5.5K, ../17a8f848.b7f6.199cf1e3b5e.Coremail.ocean_li_996@163.com/3-0001_master_fix_detach_partition_concurrently_issue.patch)
download | inline diff:diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.cindex b3ed69457fc..f78fca03182 100644--- a/src/backend/commands/tablecmds.c+++ b/src/backend/commands/tablecmds.c@@ -720,7 +720,6 @@ static void QueuePartitionConstraintValidation(List **wqueue, Relation scanrel,
List *partConstraint,
bool validate_default);
static void CloneRowTriggersToPartition(Relation parent, Relation partition);
-static void DetachAddConstraintIfNeeded(List **wqueue, Relation partRel);
static void DropClonedTriggersFromPartition(Oid partitionId);
static ObjectAddress ATExecDetachPartition(List **wqueue, AlteredTableInfo *tab,
Relation rel, RangeVar *name,
@@ -20869,12 +20868,6 @@ ATExecDetachPartition(List **wqueue, AlteredTableInfo *tab, Relation rel,
char *parentrelname;
char *partrelname;
- /*- * Add a new constraint to the partition being detached, which- * supplants the partition constraint (unless there is one already).- */- DetachAddConstraintIfNeeded(wqueue, partRel);-
/*
* We're almost done now; the only traces that remain are the
* pg_inherits tuple and the partition's relpartbounds. Before we can
@@ -21323,49 +21316,6 @@ ATExecDetachPartitionFinalize(Relation rel, RangeVar *name)
return address;
}
-/*- * DetachAddConstraintIfNeeded- * Subroutine for ATExecDetachPartition. Create a constraint that- * takes the place of the partition constraint, but avoid creating- * a dupe if a constraint already exists which implies the needed- * constraint.- */-static void-DetachAddConstraintIfNeeded(List **wqueue, Relation partRel)-{- List *constraintExpr;-- constraintExpr = RelationGetPartitionQual(partRel);- constraintExpr = (List *) eval_const_expressions(NULL, (Node *) constraintExpr);-- /*- * Avoid adding a new constraint if the needed constraint is implied by an- * existing constraint- */- if (!PartConstraintImpliedByRelConstraint(partRel, constraintExpr))- {- AlteredTableInfo *tab;- Constraint *n;-- tab = ATGetQueueEntry(wqueue, partRel);-- /* Add constraint on partition, equivalent to the partition constraint */- n = makeNode(Constraint);- n->contype = CONSTR_CHECK;- n->conname = NULL;- n->location = -1;- n->is_no_inherit = false;- n->raw_expr = NULL;- n->cooked_expr = nodeToString(make_ands_explicit(constraintExpr));- n->is_enforced = true;- n->initially_valid = true;- n->skip_validation = true;- /* It's a re-add, since it nominally already exists */- ATAddCheckNNConstraint(wqueue, tab, partRel, n,- true, false, true, ShareUpdateExclusiveLock);- }-}-
/*
* DropClonedTriggersFromPartition
* subroutine for ATExecDetachPartition to remove any triggers that were
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.outindex 476266e3f4b..e2bffc7be21 100644--- a/src/test/regress/expected/alter_table.out+++ b/src/test/regress/expected/alter_table.out@@ -4472,8 +4472,6 @@ Number of partitions: 0
Column | Type | Collation | Nullable | Default
--------+---------+-----------+----------+---------
a | integer | | |
-Check constraints:- "part_rp_a_check" CHECK (a IS NOT NULL AND a >= 0 AND a < 100)
CREATE TABLE part_rp100 PARTITION OF range_parted2 (CHECK (a>=123 AND a<133 AND a IS NOT NULL)) FOR VALUES FROM (100) to (200);
ALTER TABLE range_parted2 DETACH PARTITION part_rp100 CONCURRENTLY;
@@ -4487,6 +4485,30 @@ Check constraints:
"part_rp100_a_check" CHECK (a >= 123 AND a < 133 AND a IS NOT NULL)
DROP TABLE range_parted2;
+CREATE TABLE hash_parted2 (+ a int+) PARTITION BY HASH(a);+CREATE TABLE part_hp PARTITION OF hash_parted2 FOR VALUES WITH (MODULUS 2, REMAINDER 0);+ALTER TABLE hash_parted2 DETACH PARTITION part_hp CONCURRENTLY;+\d+ hash_parted2+ Partitioned table "public.hash_parted2"+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description +--------+---------+-----------+----------+---------+---------+--------------+-------------+ a | integer | | | | plain | | +Partition key: HASH (a)+Number of partitions: 0++-- constraint should not be created+\d part_hp+ Table "public.part_hp"+ Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+---------+ a | integer | | | ++DROP TABLE hash_parted2;+-- work fine+INSERT INTO part_hp VALUES (1);+DROP TABLE part_hp;
-- Check ALTER TABLE commands for partitioned tables and partitions
-- cannot add/drop column to/from *only* the parent
ALTER TABLE ONLY list_parted2 ADD COLUMN c int;
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sqlindex 5ce9d1e429f..06d4a8be60f 100644--- a/src/test/regress/sql/alter_table.sql+++ b/src/test/regress/sql/alter_table.sql@@ -2832,6 +2832,19 @@ ALTER TABLE range_parted2 DETACH PARTITION part_rp100 CONCURRENTLY;
\d part_rp100
DROP TABLE range_parted2;
+CREATE TABLE hash_parted2 (+ a int+) PARTITION BY HASH(a);+CREATE TABLE part_hp PARTITION OF hash_parted2 FOR VALUES WITH (MODULUS 2, REMAINDER 0);+ALTER TABLE hash_parted2 DETACH PARTITION part_hp CONCURRENTLY;+\d+ hash_parted2+-- constraint should not be created+\d part_hp+DROP TABLE hash_parted2;+-- work fine+INSERT INTO part_hp VALUES (1);+DROP TABLE part_hp;+
-- Check ALTER TABLE commands for partitioned tables and partitions
-- cannot add/drop column to/from *only* the parent
[application/octet-stream] 0001_stable_fix_detach_partition_concurrently_issue.patch (4.0K, ../17a8f848.b7f6.199cf1e3b5e.Coremail.ocean_li_996@163.com/4-0001_stable_fix_detach_partition_concurrently_issue.patch)
download | inline diff:diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.cindex b3ed69457fc..7612e9a3f86 100644--- a/src/backend/commands/tablecmds.c+++ b/src/backend/commands/tablecmds.c@@ -20797,13 +20797,14 @@ ATExecDetachPartition(List **wqueue, AlteredTableInfo *tab, Relation rel,
Relation partRel;
ObjectAddress address;
Oid defaultPartOid;
+ PartitionDesc partdesc;
/*
* We must lock the default partition, because detaching this partition
* will change its partition constraint.
*/
- defaultPartOid =- get_default_oid_from_partdesc(RelationGetPartitionDesc(rel, true));+ partdesc = RelationGetPartitionDesc(rel, true);+ defaultPartOid = get_default_oid_from_partdesc(partdesc);
if (OidIsValid(defaultPartOid))
{
/*
@@ -20872,8 +20873,19 @@ ATExecDetachPartition(List **wqueue, AlteredTableInfo *tab, Relation rel,
/*
* Add a new constraint to the partition being detached, which
* supplants the partition constraint (unless there is one already).
+ * + * In stable branches, we currently skip adding this constraint+ * for hash partitions. The reason is to avoid leaving behind+ * OID-dependent check constraints referencing the parent table's OID,+ * which could lead to unexpected errors(e.g. Could not open relation+ * with OID xxxx).+ * + * For other partitioning strategies, we keep the previous behavior+ * to prevent breaking existing workloads that might rely on the+ * absence of such constraints.
*/
- DetachAddConstraintIfNeeded(wqueue, partRel);+ if (partdesc->boundinfo->strategy != PARTITION_STRATEGY_HASH)+ DetachAddConstraintIfNeeded(wqueue, partRel);
/*
* We're almost done now; the only traces that remain are the
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.outindex 476266e3f4b..fd5b1b35bda 100644--- a/src/test/regress/expected/alter_table.out+++ b/src/test/regress/expected/alter_table.out@@ -4487,6 +4487,30 @@ Check constraints:
"part_rp100_a_check" CHECK (a >= 123 AND a < 133 AND a IS NOT NULL)
DROP TABLE range_parted2;
+CREATE TABLE hash_parted2 (+ a int+) PARTITION BY HASH(a);+CREATE TABLE part_hp PARTITION OF hash_parted2 FOR VALUES WITH (MODULUS 2, REMAINDER 0);+ALTER TABLE hash_parted2 DETACH PARTITION part_hp CONCURRENTLY;+\d+ hash_parted2+ Partitioned table "public.hash_parted2"+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description +--------+---------+-----------+----------+---------+---------+--------------+-------------+ a | integer | | | | plain | | +Partition key: HASH (a)+Number of partitions: 0++-- constraint should not be created+\d part_hp+ Table "public.part_hp"+ Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+---------+ a | integer | | | ++DROP TABLE hash_parted2;+-- work fine+INSERT INTO part_hp VALUES (1);+DROP TABLE part_hp;
-- Check ALTER TABLE commands for partitioned tables and partitions
-- cannot add/drop column to/from *only* the parent
ALTER TABLE ONLY list_parted2 ADD COLUMN c int;
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sqlindex 5ce9d1e429f..06d4a8be60f 100644--- a/src/test/regress/sql/alter_table.sql+++ b/src/test/regress/sql/alter_table.sql@@ -2832,6 +2832,19 @@ ALTER TABLE range_parted2 DETACH PARTITION part_rp100 CONCURRENTLY;
\d part_rp100
DROP TABLE range_parted2;
+CREATE TABLE hash_parted2 (+ a int+) PARTITION BY HASH(a);+CREATE TABLE part_hp PARTITION OF hash_parted2 FOR VALUES WITH (MODULUS 2, REMAINDER 0);+ALTER TABLE hash_parted2 DETACH PARTITION part_hp CONCURRENTLY;+\d+ hash_parted2+-- constraint should not be created+\d part_hp+DROP TABLE hash_parted2;+-- work fine+INSERT INTO part_hp VALUES (1);+DROP TABLE part_hp;+
-- Check ALTER TABLE commands for partitioned tables and partitions
-- cannot add/drop column to/from *only* the parent
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: ocean_li_996@163.com, alvherre@kurilemu.de, mohen.lhy@alibaba-inc.com, pgsql-bugs@lists.postgresql.org, robertmhaas@gmail.com
Subject: Re:Re: BUG #19070: issue with DETACH PARTITION CONCURRENTLY on a hash partition table
In-Reply-To: <17a8f848.b7f6.199cf1e3b5e.Coremail.ocean_li_996@163.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox