Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit72cf7f3

Browse files
committed
Fix ALTER TABLE .. ATTACH PARTITION ... DEFAULT
If the table being attached contained values that contradict the defaultpartition's partition constraint, it would fail to complain, becauseCommandCounterIncrement changes in4dba331 coupled with some boguscoding in the existing ValidatePartitionConstraints prevented thepartition constraint from being validated after all -- or rather, itcaused to constraint to become an empty one, always succeeding.Fix by not re-reading the OID of the default partition inATExecAttachPartition. To forestall similar problems, revise theexisting code:* rename routine from ValidatePartitionConstraints() to QueuePartitionConstraintValidation, to better represent what it actually does.* add an Assert() to make sure that when queueing a constraint for a partition we're not overwriting a constraint previously queued.* add an Assert() that we don't try to invoke the special-purpose validation of the default partition when attaching the default partition itself.While at it, change some loops to obtain partition OIDs frompartdesc->oids rather than find_all_inheritors; reduce the lock levelof partitions being scanned from AccessExclusiveLock to ShareLock;rewrite QueuePartitionConstraintValidation in a recursive fashion ratherthan repetitive.Author: Álvaro Herrera. Tests written by Amit LangoteReported-by: Rushabh LathiaDiagnosed-by: Kyotaro HORIGUCHI, who also provided the initial fix.Reviewed-by: Kyotaro HORIGUCHI, Amit Langote, Jeevan LadheDiscussion:https://postgr.es/m/CAGPqQf0W+v-Ci_qNV_5R3A=Z9LsK4+jO7LzgddRncpp_rrnJqQ@mail.gmail.com
1 parentcee83ef commit72cf7f3

File tree

3 files changed

+106
-98
lines changed

3 files changed

+106
-98
lines changed

‎src/backend/commands/tablecmds.c

Lines changed: 72 additions & 98 deletions
Original file line numberDiff line numberDiff line change
@@ -480,10 +480,9 @@ static void RemoveInheritance(Relation child_rel, Relation parent_rel);
480480
staticObjectAddressATExecAttachPartition(List**wqueue,Relationrel,
481481
PartitionCmd*cmd);
482482
staticvoidAttachPartitionEnsureIndexes(Relationrel,Relationattachrel);
483-
staticvoidValidatePartitionConstraints(List**wqueue,Relationscanrel,
484-
List*scanrel_children,
485-
List*partConstraint,
486-
boolvalidate_default);
483+
staticvoidQueuePartitionConstraintValidation(List**wqueue,Relationscanrel,
484+
List*partConstraint,
485+
boolvalidate_default);
487486
staticvoidCloneRowTriggersToPartition(Relationparent,Relationpartition);
488487
staticObjectAddressATExecDetachPartition(Relationrel,RangeVar*name);
489488
staticObjectAddressATExecAttachPartitionIdx(List**wqueue,Relationrel,
@@ -13939,29 +13938,23 @@ PartConstraintImpliedByRelConstraint(Relation scanrel,
1393913938
}
1394013939

1394113940
/*
13942-
*ValidatePartitionConstraints
13941+
*QueuePartitionConstraintValidation
1394313942
*
13944-
* Check whether all rows in the given table obey the given partition
13945-
* constraint; if so, it can be attached as a partition.  We do this by
13946-
* scanning the table (or all of its leaf partitions) row by row, except when
13947-
* the existing constraints are sufficient to prove that the new partitioning
13948-
* constraint must already hold.
13943+
* Add an entry to wqueue to have the given partition constraint validated by
13944+
* Phase 3, for the given relation, and all its children.
13945+
*
13946+
* We first verify whether the given constraint is implied by pre-existing
13947+
* relation constraints; if it is, there's no need to scan the table to
13948+
* validate, so don't queue in that case.
1394913949
*/
1395013950
staticvoid
13951-
ValidatePartitionConstraints(List**wqueue,Relationscanrel,
13952-
List*scanrel_children,
13953-
List*partConstraint,
13954-
boolvalidate_default)
13951+
QueuePartitionConstraintValidation(List**wqueue,Relationscanrel,
13952+
List*partConstraint,
13953+
boolvalidate_default)
1395513954
{
13956-
boolfound_whole_row;
13957-
ListCell*lc;
13958-
13959-
if (partConstraint==NIL)
13960-
return;
13961-
1396213955
/*
13963-
* Based on the table's existing constraints, determineif we can skip
13964-
*scanning the table to validatethepartition constraint.
13956+
* Based on the table's existing constraints, determinewhether or not we
13957+
*may skip scanningthetable.
1396513958
*/
1396613959
if (PartConstraintImpliedByRelConstraint(scanrel,partConstraint))
1396713960
{
@@ -13976,68 +13969,53 @@ ValidatePartitionConstraints(List **wqueue, Relation scanrel,
1397613969
return;
1397713970
}
1397813971

13979-
/* Constraints proved insufficient, so we need to scan the table. */
13980-
foreach(lc,scanrel_children)
13972+
/*
13973+
* Constraints proved insufficient. For plain relations, queue a validation
13974+
* item now; for partitioned tables, recurse to process each partition.
13975+
*/
13976+
if (scanrel->rd_rel->relkind==RELKIND_RELATION)
1398113977
{
1398213978
AlteredTableInfo*tab;
13983-
Oidpart_relid=lfirst_oid(lc);
13984-
Relationpart_rel;
13985-
List*my_partconstr=partConstraint;
1398613979

13987-
/* Lock already taken */
13988-
if (part_relid!=RelationGetRelid(scanrel))
13989-
part_rel=heap_open(part_relid,NoLock);
13990-
else
13991-
part_rel=scanrel;
13980+
/* Grab a work queue entry. */
13981+
tab=ATGetQueueEntry(wqueue,scanrel);
13982+
Assert(tab->partition_constraint==NULL);
13983+
tab->partition_constraint= (Expr*)linitial(partConstraint);
13984+
tab->validate_default=validate_default;
13985+
}
13986+
elseif (scanrel->rd_rel->relkind==RELKIND_PARTITIONED_TABLE)
13987+
{
13988+
PartitionDescpartdesc=RelationGetPartitionDesc(scanrel);
13989+
inti;
1399213990

13993-
/*
13994-
* Skip if the partition is itself a partitioned table. We can only
13995-
* ever scan RELKIND_RELATION relations.
13996-
*/
13997-
if (part_rel->rd_rel->relkind==RELKIND_PARTITIONED_TABLE)
13991+
for (i=0;i<partdesc->nparts;i++)
1399813992
{
13999-
if (part_rel!=scanrel)
14000-
heap_close(part_rel,NoLock);
14001-
continue;
14002-
}
13993+
Relationpart_rel;
13994+
boolfound_whole_row;
13995+
List*thisPartConstraint;
13996+
13997+
/*
13998+
* This is the minimum lock we need to prevent concurrent data
13999+
* additions.
14000+
*/
14001+
part_rel=heap_open(partdesc->oids[i],ShareLock);
1400314002

14004-
if (part_rel!=scanrel)
14005-
{
1400614003
/*
1400714004
* Adjust the constraint for scanrel so that it matches this
1400814005
* partition's attribute numbers.
1400914006
*/
14010-
my_partconstr=map_partition_varattnos(my_partconstr,1,
14011-
part_rel,scanrel,
14012-
&found_whole_row);
14007+
thisPartConstraint=
14008+
map_partition_varattnos(partConstraint,1,
14009+
part_rel,scanrel,&found_whole_row);
1401314010
/* There can never be a whole-row reference here */
1401414011
if (found_whole_row)
14015-
elog(ERROR,"unexpected whole-row reference found in partitionkey");
14012+
elog(ERROR,"unexpected whole-row reference found in partitionconstraint");
1401614013

14017-
/* Can we skip scanning this part_rel? */
14018-
if (PartConstraintImpliedByRelConstraint(part_rel,my_partconstr))
14019-
{
14020-
if (!validate_default)
14021-
ereport(INFO,
14022-
(errmsg("partition constraint for table \"%s\" is implied by existing constraints",
14023-
RelationGetRelationName(part_rel))));
14024-
else
14025-
ereport(INFO,
14026-
(errmsg("updated partition constraint for default partition \"%s\" is implied by existing constraints",
14027-
RelationGetRelationName(part_rel))));
14028-
heap_close(part_rel,NoLock);
14029-
continue;
14030-
}
14014+
QueuePartitionConstraintValidation(wqueue,part_rel,
14015+
thisPartConstraint,
14016+
validate_default);
14017+
heap_close(part_rel,NoLock);/* keep lock till commit */
1403114018
}
14032-
14033-
/* Grab a work queue entry. */
14034-
tab=ATGetQueueEntry(wqueue,part_rel);
14035-
tab->partition_constraint= (Expr*)linitial(my_partconstr);
14036-
tab->validate_default=validate_default;
14037-
14038-
/* keep our lock until commit */
14039-
if (part_rel!=scanrel)
14040-
heap_close(part_rel,NoLock);
1404114019
}
1404214020
}
1404314021

@@ -14067,8 +14045,8 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd)
1406714045
ListCell*l;
1406814046

1406914047
/*
14070-
* We must lock the default partition, because attaching a new partition
14071-
* will change its partition constraint.
14048+
* We must lock the default partition if one exists, because attaching a
14049+
*new partitionwill change its partition constraint.
1407214050
*/
1407314051
defaultPartOid=
1407414052
get_default_oid_from_partdesc(RelationGetPartitionDesc(rel));
@@ -14133,17 +14111,17 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd)
1413314111
*
1413414112
* We do that by checking if rel is a member of the list of attachrel's
1413514113
* partitions provided the latter is partitioned at all. We want to avoid
14136-
* having to construct this list again, so we requestthe strongestlock
14137-
*on allpartitions. We needthe strongest lock, because we may decide
14138-
* to scan them if we find out that the table being attached (or its leaf
14139-
* partitions) may contain rows that violate the partition constraint. If
14140-
* the table has a constraint that would prevent such rows, which by
14141-
* definition is present in all the partitions, we need not scan the
14142-
* table, nor its partitions. But we cannot risk a deadlock by taking a
14143-
* weaker lock now and the stronger one only when needed.
14114+
* having to construct this list again, so we requestalock on all
14115+
* partitions. We needShareLock, preventing data changes, because we
14116+
*may decideto scan them if we find out that the table being attached (or
14117+
*its leafpartitions) may contain rows that violate the partition
14118+
*constraint. Ifthe table has a constraint that would prevent such rows,
14119+
*which bydefinition is present in all the partitions, we need not scan
14120+
*thetable, nor its partitions. But we cannot risk a deadlock by taking
14121+
*aweaker lock now and the stronger one only when needed.
1414414122
*/
1414514123
attachrel_children=find_all_inheritors(RelationGetRelid(attachrel),
14146-
AccessExclusiveLock,NULL);
14124+
ShareLock,NULL);
1414714125
if (list_member_oid(attachrel_children,RelationGetRelid(rel)))
1414814126
ereport(ERROR,
1414914127
(errcode(ERRCODE_DUPLICATE_TABLE),
@@ -14291,9 +14269,7 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd)
1429114269
/*
1429214270
* Run the partition quals through const-simplification similar to
1429314271
* check constraints. We skip canonicalize_qual, though, because
14294-
* partition quals should be in canonical form already; also, since
14295-
* the qual is in implicit-AND format, we'd have to explicitly convert
14296-
* it to explicit-AND format and back again.
14272+
* partition quals should be in canonical form already.
1429714273
*/
1429814274
partConstraint=
1429914275
(List*)eval_const_expressions(NULL,
@@ -14314,32 +14290,30 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd)
1431414290
"unexpected whole-row reference found in partition key");
1431514291

1431614292
/* Validate partition constraints against the table being attached. */
14317-
ValidatePartitionConstraints(wqueue,attachrel,attachrel_children,
14318-
partConstraint, false);
14293+
QueuePartitionConstraintValidation(wqueue,attachrel,partConstraint,
14294+
false);
1431914295
}
1432014296

1432114297
/*
14322-
* Check whether default partition has a row that would fit the partition
14323-
* being attached.
14298+
* If we're attaching a partition other than the default partition and a
14299+
* default one exists, then that partition's partition constraint changes,
14300+
* so add an entry to the work queue to validate it, too. (We must not
14301+
* do this when the partition being attached is the default one; we
14302+
* already did it above!)
1432414303
*/
14325-
defaultPartOid=
14326-
get_default_oid_from_partdesc(RelationGetPartitionDesc(rel));
1432714304
if (OidIsValid(defaultPartOid))
1432814305
{
1432914306
Relationdefaultrel;
14330-
List*defaultrel_children;
1433114307
List*defPartConstraint;
1433214308

14333-
/* We already have taken a lock on default partition. */
14309+
Assert(!cmd->bound->is_default);
14310+
14311+
/* we already hold a lock on the default partition */
1433414312
defaultrel=heap_open(defaultPartOid,NoLock);
1433514313
defPartConstraint=
1433614314
get_proposed_default_constraint(partBoundConstraint);
14337-
defaultrel_children=
14338-
find_all_inheritors(defaultPartOid,
14339-
AccessExclusiveLock,NULL);
14340-
ValidatePartitionConstraints(wqueue,defaultrel,
14341-
defaultrel_children,
14342-
defPartConstraint, true);
14315+
QueuePartitionConstraintValidation(wqueue,defaultrel,
14316+
defPartConstraint, true);
1434314317

1434414318
/* keep our lock until commit. */
1434514319
heap_close(defaultrel,NoLock);

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3887,3 +3887,19 @@ ALTER TABLE attmp ALTER COLUMN i RESET (n_distinct_inherited);
38873887
ANALYZE attmp;
38883888
DROP TABLE attmp;
38893889
DROP USER regress_alter_table_user1;
3890+
-- check that violating rows are correctly reported when attaching as the
3891+
-- default partition
3892+
create table defpart_attach_test (a int) partition by list (a);
3893+
create table defpart_attach_test1 partition of defpart_attach_test for values in (1);
3894+
create table defpart_attach_test_d (like defpart_attach_test);
3895+
insert into defpart_attach_test_d values (1), (2);
3896+
-- error because its constraint as the default partition would be violated
3897+
-- by the row containing 1
3898+
alter table defpart_attach_test attach partition defpart_attach_test_d default;
3899+
ERROR: partition constraint is violated by some row
3900+
delete from defpart_attach_test_d where a = 1;
3901+
alter table defpart_attach_test_d add check (a > 1);
3902+
-- should be attached successfully and without needing to be scanned
3903+
alter table defpart_attach_test attach partition defpart_attach_test_d default;
3904+
INFO: partition constraint for table "defpart_attach_test_d" is implied by existing constraints
3905+
drop table defpart_attach_test;

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2564,3 +2564,21 @@ ANALYZE attmp;
25642564
DROPTABLE attmp;
25652565

25662566
DROPUSER regress_alter_table_user1;
2567+
2568+
-- check that violating rows are correctly reported when attaching as the
2569+
-- default partition
2570+
createtabledefpart_attach_test (aint) partition by list (a);
2571+
createtabledefpart_attach_test1 partition of defpart_attach_test forvaluesin (1);
2572+
createtabledefpart_attach_test_d (like defpart_attach_test);
2573+
insert into defpart_attach_test_dvalues (1), (2);
2574+
2575+
-- error because its constraint as the default partition would be violated
2576+
-- by the row containing 1
2577+
altertable defpart_attach_test attach partition defpart_attach_test_d default;
2578+
deletefrom defpart_attach_test_dwhere a=1;
2579+
altertable defpart_attach_test_d addcheck (a>1);
2580+
2581+
-- should be attached successfully and without needing to be scanned
2582+
altertable defpart_attach_test attach partition defpart_attach_test_d default;
2583+
2584+
droptable defpart_attach_test;

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp