🌐 AI搜索 & 代理 主页
Skip to content

Commit 7b357cc

Browse files
alvherrealvherre
committed
Don't add a redundant constraint when detaching a partition
On ALTER TABLE .. DETACH CONCURRENTLY, we add a new table constraint that duplicates the partition constraint. But if the partition already has another constraint that implies that one, then that's unnecessary. We were already avoiding the addition of a duplicate constraint if there was an exact 'equal' match -- this just improves the quality of the check. Author: Justin Pryzby <pryzby@telsasoft.com> Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://postgr.es/m/20210410184226.GY6592@telsasoft.com
1 parent e014d25 commit 7b357cc

File tree

3 files changed

+54
-33
lines changed

3 files changed

+54
-33
lines changed

‎src/backend/commands/tablecmds.c‎

Lines changed: 28 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -17915,47 +17915,42 @@ ATExecDetachPartitionFinalize(Relation rel, RangeVar *name)
1791517915
* DetachAddConstraintIfNeeded
1791617916
* Subroutine for ATExecDetachPartition. Create a constraint that
1791717917
* takes the place of the partition constraint, but avoid creating
17918-
* a dupe if an equivalent constraint already exists.
17918+
* a dupe if an constraint already exists which implies the needed
17919+
* constraint.
1791917920
*/
1792017921
static void
1792117922
DetachAddConstraintIfNeeded(List **wqueue, Relation partRel)
1792217923
{
17923-
AlteredTableInfo *tab;
17924-
Expr *constraintExpr;
17925-
TupleDesc td = RelationGetDescr(partRel);
17926-
Constraint *n;
17924+
List *constraintExpr;
1792717925

17928-
constraintExpr = make_ands_explicit(RelationGetPartitionQual(partRel));
17926+
constraintExpr = RelationGetPartitionQual(partRel);
17927+
constraintExpr = (List *) eval_const_expressions(NULL, (Node *) constraintExpr);
1792917928

17930-
/* If an identical constraint exists, we don't need to create one */
17931-
if (td->constr && td->constr->num_check > 0)
17929+
/*
17930+
* Avoid adding a new constraint if the needed constraint is implied by an
17931+
* existing constraint
17932+
*/
17933+
if (!PartConstraintImpliedByRelConstraint(partRel, constraintExpr))
1793217934
{
17933-
for (int i = 0; i < td->constr->num_check; i++)
17934-
{
17935-
Node *thisconstr;
17936-
17937-
thisconstr = stringToNode(td->constr->check[i].ccbin);
17938-
17939-
if (equal(constraintExpr, thisconstr))
17940-
return;
17941-
}
17935+
AlteredTableInfo *tab;
17936+
Constraint *n;
17937+
17938+
tab = ATGetQueueEntry(wqueue, partRel);
17939+
17940+
/* Add constraint on partition, equivalent to the partition constraint */
17941+
n = makeNode(Constraint);
17942+
n->contype = CONSTR_CHECK;
17943+
n->conname = NULL;
17944+
n->location = -1;
17945+
n->is_no_inherit = false;
17946+
n->raw_expr = NULL;
17947+
n->cooked_expr = nodeToString(make_ands_explicit(constraintExpr));
17948+
n->initially_valid = true;
17949+
n->skip_validation = true;
17950+
/* It's a re-add, since it nominally already exists */
17951+
ATAddCheckConstraint(wqueue, tab, partRel, n,
17952+
true, false, true, ShareUpdateExclusiveLock);
1794217953
}
17943-
17944-
tab = ATGetQueueEntry(wqueue, partRel);
17945-
17946-
/* Add constraint on partition, equivalent to the partition constraint */
17947-
n = makeNode(Constraint);
17948-
n->contype = CONSTR_CHECK;
17949-
n->conname = NULL;
17950-
n->location = -1;
17951-
n->is_no_inherit = false;
17952-
n->raw_expr = NULL;
17953-
n->cooked_expr = nodeToString(constraintExpr);
17954-
n->initially_valid = true;
17955-
n->skip_validation = true;
17956-
/* It's a re-add, since it nominally already exists */
17957-
ATAddCheckConstraint(wqueue, tab, partRel, n,
17958-
true, false, true, ShareUpdateExclusiveLock);
1795917954
}
1796017955

1796117956
/*

‎src/test/regress/expected/alter_table.out‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4191,6 +4191,26 @@ ALTER TABLE range_parted2 DETACH PARTITION part_rp CONCURRENTLY;
41914191
Partition key: RANGE (a)
41924192
Number of partitions: 0
41934193

4194+
-- constraint should be created
4195+
\d part_rp
4196+
Table "public.part_rp"
4197+
Column | Type | Collation | Nullable | Default
4198+
--------+---------+-----------+----------+---------
4199+
a | integer | | |
4200+
Check constraints:
4201+
"part_rp_a_check" CHECK (a IS NOT NULL AND a >= 0 AND a < 100)
4202+
4203+
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);
4204+
ALTER TABLE range_parted2 DETACH PARTITION part_rp100 CONCURRENTLY;
4205+
-- redundant constraint should not be created
4206+
\d part_rp100
4207+
Table "public.part_rp100"
4208+
Column | Type | Collation | Nullable | Default
4209+
--------+---------+-----------+----------+---------
4210+
a | integer | | |
4211+
Check constraints:
4212+
"part_rp100_a_check" CHECK (a >= 123 AND a < 133 AND a IS NOT NULL)
4213+
41944214
DROP TABLE range_parted2;
41954215
-- Check ALTER TABLE commands for partitioned tables and partitions
41964216
-- cannot add/drop column to/from *only* the parent

‎src/test/regress/sql/alter_table.sql‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2696,6 +2696,12 @@ DROP TABLE part_rpd;
26962696
-- works fine
26972697
ALTER TABLE range_parted2 DETACH PARTITION part_rp CONCURRENTLY;
26982698
\d+ range_parted2
2699+
-- constraint should be created
2700+
\d part_rp
2701+
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);
2702+
ALTER TABLE range_parted2 DETACH PARTITION part_rp100 CONCURRENTLY;
2703+
-- redundant constraint should not be created
2704+
\d part_rp100
26992705
DROP TABLE range_parted2;
27002706

27012707
-- Check ALTER TABLE commands for partitioned tables and partitions

0 commit comments

Comments
 (0)