From: feichanghong <feichanghong@qq.com>
To: feichanghong <feichanghong@qq.com>
To: pgsql-bugs <pgsql-bugs@lists.postgresql.org>
To: alvherre <alvherre@alvh.no-ip.org>
Subject: Re:BUG #18371: There are wrong constraint residues when detach hash partiton concurrently
Date: Thu, 29 Feb 2024 14:21:58 +0800
Message-ID: <tencent_4DA59E77900C87CEBA0F0213000E64040D07@qq.com> (raw)
In-Reply-To: <18371-7fef49f63de13f02@postgresql.org>
References: <18371-7fef49f63de13f02@postgresql.org>
> This can work normally on range partitions. However, the constraint on hash
> partitions uses satisfies_hash_partition with the OID of the parent table, and
> the newly created constraint does not take effect. For example, in the following
> case, although there is a t_p1_a_check constraint on t_p1, it is still possible
> to perform an insert:
What I said here is wrong, the constraints on the hash partition will also take
effect. But this constraint depends on the oid of the parent partition.
> Based on the analysis above, should the added constraint for a hash partition
> be dropped after detachment?I have initially implemented this logic in the attached patch and added a
testcase. I really hope that developers can give me some suggestions.
Best Regards,
Fei Changhong
Alibaba Cloud Computing Ltd.
Attachments:
[application/octet-stream] v1-0000-drop-constraint-for-hash-partition.patch (5.5K, ../tencent_4DA59E77900C87CEBA0F0213000E64040D07@qq.com/3-v1-0000-drop-constraint-for-hash-partition.patch)
download | inline diff:diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.cindex f798794556..ced4f9a658 100644--- a/src/backend/commands/tablecmds.c+++ b/src/backend/commands/tablecmds.c@@ -643,7 +643,7 @@ 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 ObjectAddress DetachAddConstraintIfNeeded(List **wqueue, Relation partRel);
static void DropClonedTriggersFromPartition(Oid partitionId);
static ObjectAddress ATExecDetachPartition(List **wqueue, AlteredTableInfo *tab,
Relation rel, RangeVar *name,
@@ -19579,12 +19579,14 @@ ATExecDetachPartition(List **wqueue, AlteredTableInfo *tab, Relation rel,
LOCKTAG tag;
char *parentrelname;
char *partrelname;
+ ObjectAddress consAddress = InvalidObjectAddress;+ PartitionDesc desc;
/*
* Add a new constraint to the partition being detached, which
* supplants the partition constraint (unless there is one already).
*/
- DetachAddConstraintIfNeeded(wqueue, partRel);+ consAddress = DetachAddConstraintIfNeeded(wqueue, partRel);
/*
* We're almost done now; the only traces that remain are the
@@ -19653,6 +19655,32 @@ ATExecDetachPartition(List **wqueue, AlteredTableInfo *tab, Relation rel,
errmsg("partition \"%s\" was removed concurrently", partrelname)));
tab->rel = rel;
++ /*+ * The hash partition's relpartbounds contains the oid of the parent+ * partition. If the parent partition is gone, a error is thrown when+ * checking constraints.+ */+ desc = RelationGetPartitionDesc(rel, true);+ if (desc && desc->boundinfo->strategy == PARTITION_STRATEGY_HASH &&+ OidIsValid(consAddress.objectId))+ {+ Relation constraintRelation;+ HeapTuple constraintTuple;+ Form_pg_constraint con;++ constraintRelation = table_open(ConstraintRelationId, RowExclusiveLock);+ constraintTuple = SearchSysCacheCopy1(CONSTROID, ObjectIdGetDatum(consAddress.objectId));+ if (!HeapTupleIsValid(constraintTuple))+ elog(ERROR, "cache lookup failed for constraint %u", consAddress.objectId);+ con = (Form_pg_constraint) GETSTRUCT(constraintTuple);++ ATExecDropConstraint(partRel, con->conname.data, DROP_CASCADE, true,+ true, ShareUpdateExclusiveLock);++ heap_freetuple(constraintTuple);+ table_close(constraintRelation, RowExclusiveLock);+ }
}
/* Do the final part of detaching */
@@ -19946,10 +19974,11 @@ ATExecDetachPartitionFinalize(Relation rel, RangeVar *name)
* a dupe if an constraint already exists which implies the needed
* constraint.
*/
-static void+static ObjectAddress
DetachAddConstraintIfNeeded(List **wqueue, Relation partRel)
{
List *constraintExpr;
+ ObjectAddress address = InvalidObjectAddress;
constraintExpr = RelationGetPartitionQual(partRel);
constraintExpr = (List *) eval_const_expressions(NULL, (Node *) constraintExpr);
@@ -19976,9 +20005,12 @@ DetachAddConstraintIfNeeded(List **wqueue, Relation partRel)
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);+ address = ATAddCheckNNConstraint(wqueue, tab, partRel, n,+ true, false, true,+ ShareUpdateExclusiveLock);
}
++ return address;
}
/*
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.outindex 7666c76238..af0d3c06a6 100644--- a/src/test/regress/expected/alter_table.out+++ b/src/test/regress/expected/alter_table.out@@ -4359,6 +4359,23 @@ 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,+ b int+) PARTITION BY HASH (a part_test_int4_ops);+CREATE TABLE hpart2_1 PARTITION OF hash_parted2 FOR VALUES WITH (MODULUS 4, REMAINDER 0);+ALTER TABLE hash_parted2 DETACH PARTITION hpart2_1 CONCURRENTLY;+DROP TABLE hash_parted2;+-- constraint should not be created+\d hpart2_1+ Table "public.hpart2_1"+ Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+---------+ a | integer | | | + b | integer | | | ++INSERT INTO hpart2_1 SELECT 1;+DROP TABLE hpart2_1;
-- 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 9df5a63bdf..55f2635ab9 100644--- a/src/test/regress/sql/alter_table.sql+++ b/src/test/regress/sql/alter_table.sql@@ -2786,6 +2786,18 @@ ALTER TABLE range_parted2 DETACH PARTITION part_rp100 CONCURRENTLY;
\d part_rp100
DROP TABLE range_parted2;
+CREATE TABLE hash_parted2 (+ a int,+ b int+) PARTITION BY HASH (a part_test_int4_ops);+CREATE TABLE hpart2_1 PARTITION OF hash_parted2 FOR VALUES WITH (MODULUS 4, REMAINDER 0);+ALTER TABLE hash_parted2 DETACH PARTITION hpart2_1 CONCURRENTLY;+DROP TABLE hash_parted2;+-- constraint should not be created+\d hpart2_1+INSERT INTO hpart2_1 SELECT 1;+DROP TABLE hpart2_1;+
-- 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: feichanghong@qq.com, pgsql-bugs@lists.postgresql.org, alvherre@alvh.no-ip.org
Subject: Re:BUG #18371: There are wrong constraint residues when detach hash partiton concurrently
In-Reply-To: <tencent_4DA59E77900C87CEBA0F0213000E64040D07@qq.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