From 00765f2166c78902d0c92ed9d1a4dea033a50a12 Mon Sep 17 00:00:00 2001 From: amit Date: Fri, 15 Sep 2017 14:30:36 +0900 Subject: [PATCH 1/3] Teach ATExecAttachPartition to use check_default_allows_bound Currently it schedules validation scan of the default partition in the same way as it schedules the scan of the partition being attached. Instead, call check_default_allows_bound() to do the scanning, so the handling is symmetric with DefineRelation(). Also, update the INFO message in check_default_allows_bound(), so that it's clear from the message that the default partition is involved. --- src/backend/catalog/partition.c | 8 ++----- src/backend/commands/tablecmds.c | 40 ++++++++++--------------------- src/test/regress/expected/alter_table.out | 12 +++++----- 3 files changed, 20 insertions(+), 40 deletions(-) diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c index 1ab6dba7ae..027b98d850 100644 --- a/src/backend/catalog/partition.c +++ b/src/backend/catalog/partition.c @@ -912,15 +912,11 @@ check_default_allows_bound(Relation parent, Relation default_rel, def_part_constraints = get_proposed_default_constraint(new_part_constraints); - /* - * If the existing constraints on the default partition imply that it will - * not contain any row that would belong to the new partition, we can - * avoid scanning the default partition. - */ + /* Can we skip scanning default_rel? */ if (PartConstraintImpliedByRelConstraint(default_rel, def_part_constraints)) { ereport(INFO, - (errmsg("partition constraint for table \"%s\" is implied by existing constraints", + (errmsg("updated partition constraint for default partition \"%s\" is implied by existing constraints", RelationGetRelationName(default_rel)))); return; } diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 563bcda30c..ed4a2092b3 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -168,8 +168,6 @@ typedef struct AlteredTableInfo bool chgPersistence; /* T if SET LOGGED/UNLOGGED is used */ char newrelpersistence; /* if above is true */ Expr *partition_constraint; /* for attach partition validation */ - /* true, if validating default due to some other attach/detach */ - bool validate_default; /* Objects to rebuild after completing ALTER TYPE operations */ List *changedConstraintOids; /* OIDs of constraints to rebuild */ List *changedConstraintDefs; /* string definitions of same */ @@ -477,8 +475,7 @@ static ObjectAddress ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd); static void ValidatePartitionConstraints(List **wqueue, Relation scanrel, List *scanrel_children, - List *partConstraint, - bool validate_default); + List *partConstraint); static ObjectAddress ATExecDetachPartition(Relation rel, RangeVar *name); @@ -4639,16 +4636,9 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode) } if (partqualstate && !ExecCheck(partqualstate, econtext)) - { - if (tab->validate_default) - ereport(ERROR, - (errcode(ERRCODE_CHECK_VIOLATION), - errmsg("updated partition constraint for default partition would be violated by some row"))); - else - ereport(ERROR, - (errcode(ERRCODE_CHECK_VIOLATION), - errmsg("partition constraint is violated by some row"))); - } + ereport(ERROR, + (errcode(ERRCODE_CHECK_VIOLATION), + errmsg("partition constraint is violated by some row"))); /* Write the tuple out to the new relation */ if (newrel) @@ -13620,8 +13610,7 @@ PartConstraintImpliedByRelConstraint(Relation scanrel, static void ValidatePartitionConstraints(List **wqueue, Relation scanrel, List *scanrel_children, - List *partConstraint, - bool validate_default) + List *partConstraint) { bool found_whole_row; ListCell *lc; @@ -13683,7 +13672,6 @@ ValidatePartitionConstraints(List **wqueue, Relation scanrel, /* Grab a work queue entry. */ tab = ATGetQueueEntry(wqueue, part_rel); tab->partition_constraint = (Expr *) linitial(my_partconstr); - tab->validate_default = validate_default; /* keep our lock until commit */ if (part_rel != scanrel) @@ -13925,7 +13913,7 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd) /* Validate partition constraints against the table being attached. */ ValidatePartitionConstraints(wqueue, attachrel, attachrel_children, - partConstraint, false); + partConstraint); } /* @@ -13937,19 +13925,15 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd) if (OidIsValid(defaultPartOid)) { Relation defaultrel; - List *defaultrel_children; - List *defPartConstraint; /* We already have taken a lock on default partition. */ defaultrel = heap_open(defaultPartOid, NoLock); - defPartConstraint = - get_proposed_default_constraint(partBoundConstraint); - defaultrel_children = - find_all_inheritors(defaultPartOid, - AccessExclusiveLock, NULL); - ValidatePartitionConstraints(wqueue, defaultrel, - defaultrel_children, - defPartConstraint, true); + + /* + * Go scan the default partition to see if contains rows that belong + * to the new partition. Won't return and error instead if it does. + */ + check_default_allows_bound(rel, defaultrel, cmd->bound); /* keep our lock until commit. */ heap_close(defaultrel, NoLock); diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index 0478a8ac60..eaf5f4b507 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -3323,7 +3323,7 @@ CREATE TABLE list_parted2_def PARTITION OF list_parted2 DEFAULT; INSERT INTO list_parted2_def VALUES (11, 'z'); CREATE TABLE part_3 (LIKE list_parted2); ALTER TABLE list_parted2 ATTACH PARTITION part_3 FOR VALUES IN (11); -ERROR: updated partition constraint for default partition would be violated by some row +ERROR: updated partition constraint for default partition "list_parted2_def" would be violated by some row -- should be ok after deleting the bad row DELETE FROM list_parted2_def WHERE a = 11; ALTER TABLE list_parted2 ATTACH PARTITION part_3 FOR VALUES IN (11); @@ -3345,7 +3345,7 @@ INFO: partition constraint for table "part_3_4" is implied by existing constrai -- check if default partition scan skipped ALTER TABLE list_parted2_def ADD CONSTRAINT check_a CHECK (a IN (5, 6)); CREATE TABLE part_55_66 PARTITION OF list_parted2 FOR VALUES IN (55, 66); -INFO: partition constraint for table "list_parted2_def" is implied by existing constraints +INFO: updated partition constraint for default partition "list_parted2_def" is implied by existing constraints -- check validation when attaching range partitions CREATE TABLE range_parted ( a int, @@ -3381,7 +3381,7 @@ ERROR: partition "partr_def2" conflicts with existing default partition "partr_ INSERT INTO partr_def1 VALUES (2, 10); CREATE TABLE part3 (LIKE range_parted); ALTER TABLE range_parted ATTACH partition part3 FOR VALUES FROM (2, 10) TO (2, 20); -ERROR: updated partition constraint for default partition would be violated by some row +ERROR: updated partition constraint for default partition "partr_def1" would be violated by some row -- Attaching partitions should be successful when there are no overlapping rows ALTER TABLE range_parted ATTACH partition part3 FOR VALUES FROM (3, 10) TO (3, 20); -- check that leaf partitions are scanned when attaching a partitioned @@ -3436,7 +3436,7 @@ ALTER TABLE part_7 ATTACH PARTITION part_7_a_null FOR VALUES IN ('a', null); INFO: partition constraint for table "part_7_a_null" is implied by existing constraints ALTER TABLE list_parted2 ATTACH PARTITION part_7 FOR VALUES IN (7); INFO: partition constraint for table "part_7" is implied by existing constraints -INFO: partition constraint for table "list_parted2_def" is implied by existing constraints +INFO: updated partition constraint for default partition "list_parted2_def" is implied by existing constraints -- Same example, but check this time that the constraint correctly detects -- violating rows ALTER TABLE list_parted2 DETACH PARTITION part_7; @@ -3450,7 +3450,7 @@ SELECT tableoid::regclass, a, b FROM part_7 order by a; (2 rows) ALTER TABLE list_parted2 ATTACH PARTITION part_7 FOR VALUES IN (7); -INFO: partition constraint for table "list_parted2_def" is implied by existing constraints +INFO: updated partition constraint for default partition "list_parted2_def" is implied by existing constraints ERROR: partition constraint is violated by some row -- check that leaf partitions of default partition are scanned when -- attaching a partitioned table. @@ -3460,7 +3460,7 @@ CREATE TABLE part5_def_p1 PARTITION OF part5_def FOR VALUES IN (5); INSERT INTO part5_def_p1 VALUES (5, 'y'); CREATE TABLE part5_p1 (LIKE part_5); ALTER TABLE part_5 ATTACH PARTITION part5_p1 FOR VALUES IN ('y'); -ERROR: updated partition constraint for default partition would be violated by some row +ERROR: updated partition constraint for default partition "part5_def" would be violated by some row -- should be ok after deleting the bad row DELETE FROM part5_def_p1 WHERE b = 'y'; ALTER TABLE part_5 ATTACH PARTITION part5_p1 FOR VALUES IN ('y'); -- 2.11.0