pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedBUG #18371: There are wrong constraint residues when detach hash partiton concurrently
11+ messages / 8 participants
[nested] [flat]
* BUG #18371: There are wrong constraint residues when detach hash partiton concurrently
@ 2024-02-28 11:30 PG Bug reporting form <noreply@postgresql.org>
0 siblings, 1 reply; 11+ messages in thread
From: PG Bug reporting form @ 2024-02-28 11:30 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: feichanghong@qq.com
The following bug has been logged on the website:
Bug reference: 18371
Logged by: Fei Changhong
Email address: feichanghong@qq.com
PostgreSQL version: 16.2
Operating system: Operating system: centos 7,Kernel version: 5.10.13
Description:
Hi all,
When I tried to insert data into a table that had just been detached, I
encountered an error: "ERROR: could not open relation with OID 16384".
The environment information is as follows:
PostgreSQL branch: master.
commit: bcdfa5f2e2f274caeed20b2f986012a9cb6a259c
This issue can be reproduced with the following steps:
```
create table t(a int) partition by hash (a);
create table t_p1 partition of t for values with (modulus 2, remainder 0);
alter table t detach partition t_p1 concurrently ;
drop table t;
insert into t_p1 select 1;
```
When detaching a partition concurrently, the function
DetachAddConstraintIfNeeded
is called to convert the partition constraint of the detached partition into
a
regular constraint, and it is not dropped at the end of the detach
process.
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:
```
create table t(a int) partition by hash (a);
create table t_p1 partition of t for values with (modulus 2, remainder 0);
alter table t detach partition t_p1 concurrently ;
postgres=# \d+ t_p1
Table "public.t_p1"
Column | Type | Collation | Nullable | Default | Storage | Compression |
Stats target | Description
--------+---------+-----------+----------+---------+---------+-------------+--------------+-------------
a | integer | | | | plain | |
|
Check constraints:
"t_p1_a_check" CHECK (satisfies_hash_partition('16384'::oid, 2, 0, a))
Access method: heap
postgres=# insert into t_p1 select 1;
INSERT 0 1
postgres=#
```
If the parent table has already been dropped, an error will occur during
constraint checking.
Based on the analysis above, should the added constraint for a hash
partition
be dropped after detachment?
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re:BUG #18371: There are wrong constraint residues when detach hash partiton concurrently
@ 2024-02-29 06:21 =?ISO-8859-1?B?ZmVpY2hhbmdob25n?= <feichanghong@qq.com>
parent: PG Bug reporting form <noreply@postgresql.org>
0 siblings, 2 replies; 11+ messages in thread
From: feichanghong @ 2024-02-29 06:21 UTC (permalink / raw)
To: feichanghong <feichanghong@qq.com>; pgsql-bugs <pgsql-bugs@lists.postgresql.org>; alvherre <alvherre@alvh.no-ip.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.c
index 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.out
index 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.sql
index 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
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: BUG #18371: There are wrong constraint residues when detach hash partiton concurrently
@ 2024-03-05 03:13 Michael Paquier <michael@paquier.xyz>
parent: =?ISO-8859-1?B?ZmVpY2hhbmdob25n?= <feichanghong@qq.com>
1 sibling, 2 replies; 11+ messages in thread
From: Michael Paquier @ 2024-03-05 03:13 UTC (permalink / raw)
To: feichanghong <feichanghong@qq.com>; +Cc: pgsql-bugs <pgsql-bugs@lists.postgresql.org>; alvherre <alvherre@alvh.no-ip.org>
On Thu, Feb 29, 2024 at 02:21:58PM +0800, feichanghong wrote:
> 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.
Something is weird with the format of the messages you have sent, by
the way.
> 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.
I am not much a fan of relying on ATExecDropConstraint(), where the
logic introduced is a copy-paste of get_constraint_name() to retrieve
the name of the constraint to drop. If any, we ought to rely more on
dropconstraint_internal() instead using the pg_constraint tuple based
on the OID of the constraint, and not do a cross-operation with the
constraint name.
But actually, why do you do a drop of the constraint at all?
DetachAddConstraintIfNeeded() re-adds the constraint as it is, so
instead of re-adding it as-is and then dropping it again, wouldn't it
be better to just *not* re-add to begin with it if we don't need it at
all?
This reproduces down to 14, for what I am assuming is an issue
introduced around 7b357cc6ae55. Alvaro, what do you think?
--
Michael
Attachments:
[application/pgp-signature] signature.asc (832B, ../../ZeaNxNW4m2106fSk@paquier.xyz/2-signature.asc)
download
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: BUG #18371: There are wrong constraint residues when detach hash partiton concurrently
@ 2024-03-05 07:37 alvherre <alvherre@alvh.no-ip.org>
parent: Michael Paquier <michael@paquier.xyz>
1 sibling, 1 reply; 11+ messages in thread
From: alvherre @ 2024-03-05 07:37 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: feichanghong <feichanghong@qq.com>; pgsql-bugs <pgsql-bugs@lists.postgresql.org>
On 2024-Mar-05, Michael Paquier wrote:
> This reproduces down to 14, for what I am assuming is an issue
> introduced around 7b357cc6ae55. Alvaro, what do you think?
Yeah, I have this on my list of things to fix this month. I haven't
looked at it closely, but I will soon.
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"Los cuentos de hadas no dan al niño su primera idea sobre los monstruos.
Lo que le dan es su primera idea de la posible derrota del monstruo."
(G. K. Chesterton)
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: BUG #18371: There are wrong constraint residues when detach hash partiton concurrently
@ 2024-03-05 08:57 费长红 <feichanghong@qq.com>
parent: Michael Paquier <michael@paquier.xyz>
1 sibling, 0 replies; 11+ messages in thread
From: 费长红 @ 2024-03-05 08:57 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: pgsql-bugs <pgsql-bugs@lists.postgresql.org>; alvherre <alvherre@alvh.no-ip.org>
On 2024/3/5, 11:13, "Michael Paquier" <michael@paquier.xyz> wrote:
Something is weird with the format of the messages you have sent, by
the way.
Ah ha! This should be a problem with my email client, I will try another client.
I am not much a fan of relying on ATExecDropConstraint(), where the
logic introduced is a copy-paste of get_constraint_name() to retrieve
the name of the constraint to drop. If any, we ought to rely more on
dropconstraint_internal() instead using the pg_constraint tuple based
on the OID of the constraint, and not do a cross-operation with the
constraint name.
But actually, why do you do a drop of the constraint at all?
DetachAddConstraintIfNeeded() re-adds the constraint as it is, so
instead of re-adding it as-is and then dropping it again, wouldn't it
be better to just *not* re-add to begin with it if we don't need it at
all?
After verification, the partition constraints on the partition being detached
remain in effect until the second transaction committed. This newly added
constraint should only be for the purpose of speeding up the re-attach
operation. Perhaps I've missed something?
So, for hash partitioning, it should be better not to re-add new constraints.
Perhaps we should filter out hash partitions in DetachAddConstraintIfNeeded().
--
Best Regards,
Fei Changhong
Alibaba Cloud Computing Ltd.
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: BUG #18371: There are wrong constraint residues when detach hash partiton concurrently
@ 2024-03-05 23:12 Michael Paquier <michael@paquier.xyz>
parent: alvherre <alvherre@alvh.no-ip.org>
0 siblings, 0 replies; 11+ messages in thread
From: Michael Paquier @ 2024-03-05 23:12 UTC (permalink / raw)
To: alvherre <alvherre@alvh.no-ip.org>; +Cc: feichanghong <feichanghong@qq.com>; pgsql-bugs <pgsql-bugs@lists.postgresql.org>
On Tue, Mar 05, 2024 at 08:37:27AM +0100, Alvaro Herrera wrote:
> Yeah, I have this on my list of things to fix this month. I haven't
> looked at it closely, but I will soon.
Cool, thanks!
--
Michael
Attachments:
[application/pgp-signature] signature.asc (832B, ../../Zeem8HGh6aPW3uWm@paquier.xyz/2-signature.asc)
download
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: BUG #18371: There are wrong constraint residues when detach hash partiton concurrently
@ 2024-07-15 19:35 alvherre <alvherre@alvh.no-ip.org>
parent: =?ISO-8859-1?B?ZmVpY2hhbmdob25n?= <feichanghong@qq.com>
1 sibling, 4 replies; 11+ messages in thread
From: alvherre @ 2024-07-15 19:35 UTC (permalink / raw)
To: feichanghong <feichanghong@qq.com>; +Cc: pgsql-bugs <pgsql-bugs@lists.postgresql.org>
On 2024-Feb-29, feichanghong wrote:
> > 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.
We should definitely not have this constraint on hash-partition tables
after the detach. However, I wonder if instead of adding it and later
removing it as you propose, it wouldn't be better to just not add it in
the first place. As a first step, I tried commenting out and found that
no interesting test fails (only alter_table.sql fails but only because
the constraint is not there when looking for it specifically.)
The current code does not explain *why* we have to add this constraint,
and I had forgotten, so I went to look at the first patch submission in
that thread [1] and saw this comment there:
+ /*
+ * Concurrent mode has to work harder; first we add a new constraint to the
+ * partition that matches the partition constraint. The reason for this is
+ * that the planner may have made optimizations that depend on the
+ * constraint. XXX Isn't it sufficient to invalidate the partition's
+ * relcache entry?
I'm trying to figure out whether it's possible that the planner would
make optimizations based on the hashing function. Quite possibly it
won't. If that's so, then we should just not make the constraint at
all, which would make the fix even simpler. I also wonder, maybe that
XXX comment (which I removed before committing the patch) is right and
we don't actually need the constraint with _any_ partition strategy, not
just hash.
[1] https://postgr.es/m/20200803234854.GA24158@alvherre.pgsql
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: BUG #18371: There are wrong constraint residues when detach hash partiton concurrently
@ 2024-07-16 05:03 feichanghong <feichanghong@qq.com>
parent: alvherre <alvherre@alvh.no-ip.org>
3 siblings, 0 replies; 11+ messages in thread
From: feichanghong @ 2024-07-16 05:03 UTC (permalink / raw)
To: alvherre <alvherre@alvh.no-ip.org>; +Cc: pgsql-bugs <pgsql-bugs@lists.postgresql.org>
Hi Alvaro,
> On Jul 16, 2024, at 03:35, alvherre <alvherre@alvh.no-ip.org> wrote:
>
> We should definitely not have this constraint on hash-partition tables
> after the detach. However, I wonder if instead of adding it and later
> removing it as you propose, it wouldn't be better to just not add it in
> the first place. As a first step, I tried commenting out and found that
> no interesting test fails (only alter_table.sql fails but only because
> the constraint is not there when looking for it specifically.)
>
> The current code does not explain *why* we have to add this constraint,
> and I had forgotten, so I went to look at the first patch submission in
> that thread [1] and saw this comment there:
>
> + /*
> + * Concurrent mode has to work harder; first we add a new constraint to the
> + * partition that matches the partition constraint. The reason for this is
> + * that the planner may have made optimizations that depend on the
> + * constraint. XXX Isn't it sufficient to invalidate the partition's
> + * relcache entry?
>
>
> I'm trying to figure out whether it's possible that the planner would
> make optimizations based on the hashing function. Quite possibly it
> won't. If that's so, then we should just not make the constraint at
> all, which would make the fix even simpler. I also wonder, maybe that
> XXX comment (which I removed before committing the patch) is right and
> we don't actually need the constraint with _any_ partition strategy, not
> just hash.
I agree with your point of view. There is no need to add an extra partition
constraint for "DETACH PARTITION CONCURRENTLY". In the first transaction, we
merely set inhdetachpending to true without actually dropping the existing
partition constraint.
At the start of the second transaction, we wait for all queries that were
planned with the partition to finish. Then, we acquire an AccessExclusiveLock
on the sub-partition and drop the original partition constraint. Most likely,
the newly added constraint is useless for the optimizer.
The only useful scenario I can think of is that, if we were to attach this
sub-partition back to the parent table using the same strategy, it might reduce
the checks for partition constraints.
Best Regards,
Fei Changhong
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: BUG #18371: There are wrong constraint residues when detach hash partiton concurrently
@ 2024-07-16 07:39 Tender Wang <tndrwang@gmail.com>
parent: alvherre <alvherre@alvh.no-ip.org>
3 siblings, 0 replies; 11+ messages in thread
From: Tender Wang @ 2024-07-16 07:39 UTC (permalink / raw)
To: alvherre <alvherre@alvh.no-ip.org>; +Cc: feichanghong <feichanghong@qq.com>; pgsql-bugs <pgsql-bugs@lists.postgresql.org>
alvherre <alvherre@alvh.no-ip.org> 于2024年7月16日周二 03:35写道：
> On 2024-Feb-29, feichanghong wrote:
>
> > > 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.
>
> We should definitely not have this constraint on hash-partition tables
> after the detach. However, I wonder if instead of adding it and later
> removing it as you propose, it wouldn't be better to just not add it in
> the first place. As a first step, I tried commenting out and found that
> no interesting test fails (only alter_table.sql fails but only because
> the constraint is not there when looking for it specifically.)
>
> The current code does not explain *why* we have to add this constraint,
> and I had forgotten, so I went to look at the first patch submission in
> that thread [1] and saw this comment there:
>
> + /*
> + * Concurrent mode has to work harder; first we add a new constraint
> to the
> + * partition that matches the partition constraint. The reason for
> this is
> + * that the planner may have made optimizations that depend on the
> + * constraint. XXX Isn't it sufficient to invalidate the partition's
> + * relcache entry?
>
>
> I'm trying to figure out whether it's possible that the planner would
> make optimizations based on the hashing function. Quite possibly it
> won't. If that's so, then we should just not make the constraint at
> all, which would make the fix even simpler. I also wonder, maybe that
> XXX comment (which I removed before committing the patch) is right and
> we don't actually need the constraint with _any_ partition strategy, not
> just hash.
>
The planner will do partition pruning based on partbound, if there is no the
new adding constraint when detaching concurrently, can we make sure that
the query see the consistent result?
--
Tender Wang
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: BUG #18371: There are wrong constraint residues when detach hash partiton concurrently
@ 2024-11-19 09:57 Tender Wang <tndrwang@gmail.com>
parent: alvherre <alvherre@alvh.no-ip.org>
3 siblings, 0 replies; 11+ messages in thread
From: Tender Wang @ 2024-11-19 09:57 UTC (permalink / raw)
To: alvherre <alvherre@alvh.no-ip.org>; +Cc: feichanghong <feichanghong@qq.com>; pgsql-bugs <pgsql-bugs@lists.postgresql.org>; Michael Paquier <michael@paquier.xyz>
alvherre <alvherre@alvh.no-ip.org> 于2024年7月16日周二 03:35写道：
> On 2024-Feb-29, feichanghong wrote:
>
> > > 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.
>
> We should definitely not have this constraint on hash-partition tables
> after the detach. However, I wonder if instead of adding it and later
> removing it as you propose, it wouldn't be better to just not add it in
> the first place. As a first step, I tried commenting out and found that
> no interesting test fails (only alter_table.sql fails but only because
> the constraint is not there when looking for it specifically.)
>
> The current code does not explain *why* we have to add this constraint,
> and I had forgotten, so I went to look at the first patch submission in
> that thread [1] and saw this comment there:
>
> + /*
> + * Concurrent mode has to work harder; first we add a new constraint
> to the
> + * partition that matches the partition constraint. The reason for
> this is
> + * that the planner may have made optimizations that depend on the
> + * constraint. XXX Isn't it sufficient to invalidate the partition's
> + * relcache entry?
>
>
> I'm trying to figure out whether it's possible that the planner would
> make optimizations based on the hashing function. Quite possibly it
> won't. If that's so, then we should just not make the constraint at
> all, which would make the fix even simpler. I also wonder, maybe that
> XXX comment (which I removed before committing the patch) is right and
> we don't actually need the constraint with _any_ partition strategy, not
> just hash.
>
I pick up this issue again. I skim through the codes and I agree with your
point.
We don't acually need the constraint with any partition strategy, not just
hash.
If we have old queries when we are doing detach, the detach session will be
stopped at
WaitForLockersMultiple(). Other sessions can't insert tuples that violate
partition constraint.
Because the detached partition still has partition constranit.
--
Thanks,
Tender Wang
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: BUG #18371: There are wrong constraint residues when detach hash partiton concurrently
@ 2025-10-11 18:35 Álvaro Herrera <alvherre@kurilemu.de>
parent: alvherre <alvherre@alvh.no-ip.org>
3 siblings, 0 replies; 11+ messages in thread
From: Álvaro Herrera @ 2025-10-11 18:35 UTC (permalink / raw)
To: feichanghong <feichanghong@qq.com>; +Cc: pgsql-bugs <pgsql-bugs@lists.postgresql.org>
On 2024-Jul-15, alvherre wrote:
> We should definitely not have this constraint on hash-partition tables
> after the detach. However, I wonder if instead of adding it and later
> removing it as you propose, it wouldn't be better to just not add it in
> the first place. As a first step, I tried commenting out and found that
> no interesting test fails (only alter_table.sql fails but only because
> the constraint is not there when looking for it specifically.)
Pushed a fix for this per report in
https://postgr.es/m/19070-781326347ade7c57@postgresql.org
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
<Schwern> It does it in a really, really complicated way
<crab> why does it need to be complicated?
<Schwern> Because it's MakeMaker.
^ permalink raw reply [nested|flat] 11+ messages in thread
end of thread, other threads:[~2025-10-11 18:35 UTC | newest]
Thread overview: 11+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-02-28 11:30 BUG #18371: There are wrong constraint residues when detach hash partiton concurrently PG Bug reporting form <noreply@postgresql.org>
2024-02-29 06:21 ` =?ISO-8859-1?B?ZmVpY2hhbmdob25n?= <feichanghong@qq.com>
2024-03-05 03:13 ` Michael Paquier <michael@paquier.xyz>
2024-03-05 07:37 ` alvherre <alvherre@alvh.no-ip.org>
2024-03-05 23:12 ` Michael Paquier <michael@paquier.xyz>
2024-03-05 08:57 ` 费长红 <feichanghong@qq.com>
2024-07-15 19:35 ` alvherre <alvherre@alvh.no-ip.org>
2024-07-16 05:03 ` feichanghong <feichanghong@qq.com>
2024-07-16 07:39 ` Tender Wang <tndrwang@gmail.com>
2024-11-19 09:57 ` Tender Wang <tndrwang@gmail.com>
2025-10-11 18:35 ` Álvaro Herrera <alvherre@kurilemu.de>
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