0001-Teach-ATExecAttachPartition-to-use-check_default_all.patch

text/plain

Filename: 0001-Teach-ATExecAttachPartition-to-use-check_default_all.patch
Type: text/plain
Part: 0
Message: Re: Optimise default partition scanning while adding new partition

Patch

Format: format-patch
Series: patch 0001
Subject: Teach ATExecAttachPartition to use check_default_allows_bound
File+
src/backend/catalog/partition.c 2 6
src/backend/commands/tablecmds.c 12 28
src/test/regress/expected/alter_table.out 6 6
From 00765f2166c78902d0c92ed9d1a4dea033a50a12 Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
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