Optimise default partition scanning while adding new partition

Started by Jeevan Ladheover 8 years ago12 messages

jeevan.ladhe@enterprisedb.com

over 8 years ago

1 attachment(s)

Hi,

Commit 6f6b99d1335be8ea1b74581fc489a97b109dd08a introduced default
partitioning support. This commit added a new function
check_default_allows_bound(),
which checks if there exists a row in the default partition that would
belong to
the new partition being added. If it finds one, it throws an error. Before
taking
the decision to scan the default partition, this function checks if there
are
existing constraints on default partition that would imply the new partition
constraints, if yes it skips scanning the default partition, otherwise it
scans the
default partition and its children(if any). But, while doing so the current
code
misses the fact that there can be constraints on the child of default
partition
such that they would imply the constraints of the new partition being added,
and hence individual child scan can also be skipped.
Attached is the patch which does this.

This is previously discussed in default partitioning thread[1]/messages/by-id/CA+TgmoZ8-q=2OahOXmvZbDNxi9G6i1iDi4OZfkb67MK242D_Aw@mail.gmail.com, and decision
was made that we can take this a separate patch rather than as a part of the
default partitioning support.

Amit Langote has a similar patch[2]/messages/by-id/4cd13b03-846d-dc65-89de-1fd9743a3869@lab.ntt.co.jp for scanning the children of a
partitioned
table which is being attached as partition of another partitioned table.

[1]: /messages/by-id/CA+TgmoZ8-q=2OahOXmvZbDNxi9G6i1iDi4OZfkb67MK242D_Aw@mail.gmail.com
/messages/by-id/CA+TgmoZ8-q=2OahOXmvZbDNxi9G6i1iDi4OZfkb67MK242D_Aw@mail.gmail.com
[2]: /messages/by-id/4cd13b03-846d-dc65-89de-1fd9743a3869@lab.ntt.co.jp
/messages/by-id/4cd13b03-846d-dc65-89de-1fd9743a3869@lab.ntt.co.jp

Regards,
Jeevan Ladhe

Attachments:

0001-Check-default-partitition-child-validation-scan-is.patchapplication/octet-stream; name=0001-Check-default-partitition-child-validation-scan-is.patchDownload

From 43265fea16a2a9523e1e90bd4675dddb08c54f52 Mon Sep 17 00:00:00 2001
From: Jeevan Ladhe <jeevan.ladhe@enterprisedb.com>
Date: Tue, 12 Sep 2017 14:44:36 +0530
Subject: [PATCH 1/1] Check default partitition child validation scan is
 skippable

Optimize check_default_allows_bound() such that the default
partition children constraints are checked against new partition
constraints for implication and avoid scan of the child of which
existing constraints are implied by new default partition
constraints. Also, added testcase for the same.

Jeevan Ladhe.
---
 src/backend/catalog/partition.c           | 18 ++++++++++++++++++
 src/test/regress/expected/alter_table.out | 14 ++++++++++++--
 src/test/regress/sql/alter_table.sql      |  9 +++++++++
 3 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c
index 73eff17..da3187d 100644
--- a/src/backend/catalog/partition.c
+++ b/src/backend/catalog/partition.c
@@ -951,7 +951,25 @@ check_default_allows_bound(Relation parent, Relation default_rel,
 
 		/* Lock already taken above. */
 		if (part_relid != RelationGetRelid(default_rel))
+		{
 			part_rel = heap_open(part_relid, NoLock);
+
+			/*
+			 * If the partition constraints on default partition child imply
+			 * that it will not contain any row that would belong to the new
+			 * partition, we can avoid scanning the child table.
+			 */
+			if (PartConstraintImpliedByRelConstraint(part_rel,
+													 def_part_constraints))
+			{
+				ereport(INFO,
+						(errmsg("partition constraint for table \"%s\" is implied by existing constraints",
+								RelationGetRelationName(part_rel))));
+
+				heap_close(part_rel, NoLock);
+				continue;
+			}
+		}
 		else
 			part_rel = default_rel;
 
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 0478a8a..32c4fda 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -3346,6 +3346,18 @@ INFO:  partition constraint for table "part_3_4" is implied by existing constrai
 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
+-- check if default partition child relation scan skipped
+DROP TABLE list_parted2_def;
+CREATE TABLE list_parted2_def PARTITION OF list_parted2 DEFAULT PARTITION BY RANGE (a);
+CREATE TABLE list_parted2_def_p1 PARTITION OF list_parted2_def FOR VALUES FROM (21) TO (30);
+CREATE TABLE list_parted2_def_p2 PARTITION OF list_parted2_def FOR VALUES FROM (31) TO (40);
+ALTER TABLE list_parted2_def_p1 ADD CONSTRAINT check_a CHECK (a IN (21, 22));
+ALTER TABLE list_parted2_def_p2 ADD CONSTRAINT check_a CHECK (a IN (31, 32));
+CREATE TABLE part_9 PARTITION OF list_parted2 FOR VALUES IN (9);
+INFO:  partition constraint for table "list_parted2_def_p1" is implied by existing constraints
+INFO:  partition constraint for table "list_parted2_def_p2" is implied by existing constraints
+CREATE TABLE part_31 PARTITION OF list_parted2 FOR VALUES IN (31);
+INFO:  partition constraint for table "list_parted2_def_p1" is implied by existing constraints
 -- check validation when attaching range partitions
 CREATE TABLE range_parted (
 	a int,
@@ -3436,7 +3448,6 @@ 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
 -- Same example, but check this time that the constraint correctly detects
 -- violating rows
 ALTER TABLE list_parted2 DETACH PARTITION part_7;
@@ -3450,7 +3461,6 @@ 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
 ERROR:  partition constraint is violated by some row
 -- check that leaf partitions of default partition are scanned when
 -- attaching a partitioned table.
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 37cca72..3ad6302 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -2163,6 +2163,15 @@ ALTER TABLE list_parted2 ATTACH PARTITION part_3_4 FOR VALUES IN (3, 4);
 -- 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);
+-- check if default partition child relation scan skipped
+DROP TABLE list_parted2_def;
+CREATE TABLE list_parted2_def PARTITION OF list_parted2 DEFAULT PARTITION BY RANGE (a);
+CREATE TABLE list_parted2_def_p1 PARTITION OF list_parted2_def FOR VALUES FROM (21) TO (30);
+CREATE TABLE list_parted2_def_p2 PARTITION OF list_parted2_def FOR VALUES FROM (31) TO (40);
+ALTER TABLE list_parted2_def_p1 ADD CONSTRAINT check_a CHECK (a IN (21, 22));
+ALTER TABLE list_parted2_def_p2 ADD CONSTRAINT check_a CHECK (a IN (31, 32));
+CREATE TABLE part_9 PARTITION OF list_parted2 FOR VALUES IN (9);
+CREATE TABLE part_31 PARTITION OF list_parted2 FOR VALUES IN (31);
 
 -- check validation when attaching range partitions
 CREATE TABLE range_parted (
-- 
2.7.4

Amit Langote

Langote_Amit_f8@lab.ntt.co.jp

over 8 years ago

In reply to: Jeevan Ladhe (#1)

Re: Optimise default partition scanning while adding new partition

Hi Jeevan,

On 2017/09/12 18:22, Jeevan Ladhe wrote:

Commit 6f6b99d1335be8ea1b74581fc489a97b109dd08a introduced default
partitioning support. This commit added a new function
check_default_allows_bound(),
which checks if there exists a row in the default partition that would
belong to
the new partition being added. If it finds one, it throws an error. Before
taking
the decision to scan the default partition, this function checks if there
are
existing constraints on default partition that would imply the new partition
constraints, if yes it skips scanning the default partition, otherwise it
scans the
default partition and its children(if any). But, while doing so the current
code
misses the fact that there can be constraints on the child of default
partition
such that they would imply the constraints of the new partition being added,
and hence individual child scan can also be skipped.
Attached is the patch which does this.

This is previously discussed in default partitioning thread[1], and decision
was made that we can take this a separate patch rather than as a part of the
default partitioning support.

Patch looks fine to me. By the way, why don't we just say "Can we skip
scanning part_rel?" in the comment before the newly added call to
PartConstraintImpliedByRelConstraint()? We don't need to repeat the
explanation of what it does at the every place we call it.

Amit Langote has a similar patch[2] for scanning the children of a
partitioned
table which is being attached as partition of another partitioned table.

I just posted my rebased patch there [1]/messages/by-id/a83a0899-19f5-594c-9aac-3ba0f16989a1@lab.ntt.co.jp.

Thanks,
Amit

[1]: /messages/by-id/a83a0899-19f5-594c-9aac-3ba0f16989a1@lab.ntt.co.jp
/messages/by-id/a83a0899-19f5-594c-9aac-3ba0f16989a1@lab.ntt.co.jp

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Jeevan Ladhe

jeevan.ladhe@enterprisedb.com

over 8 years ago

In reply to: Amit Langote (#2)

1 attachment(s)

Re: Optimise default partition scanning while adding new partition

Thanks Amit for reviewing.

Patch looks fine to me. By the way, why don't we just say "Can we skip

scanning part_rel?" in the comment before the newly added call to
PartConstraintImpliedByRelConstraint()? We don't need to repeat the
explanation of what it does at the every place we call it.

I have changed the comment.
PFA.

Regards,
Jeevan Ladhe

Attachments:

0001-Check-default-partitition-child-validation-scan-is.patchapplication/octet-stream; name=0001-Check-default-partitition-child-validation-scan-is.patchDownload

From 9866884c26e2bb4b08b50069472149fba8c7b4c9 Mon Sep 17 00:00:00 2001
From: Jeevan Ladhe <jeevan.ladhe@enterprisedb.com>
Date: Thu, 14 Sep 2017 13:31:08 +0530
Subject: [PATCH 1/1] Check default partitition child validation scan is
 skippable

Optimize check_default_allows_bound() such that the default
partition children constraints are checked against new partition
constraints for implication and avoid scan of the child of which
existing constraints are implied by new default partition
constraints. Also, added testcase for the same.

Jeevan Ladhe.
---
 src/backend/catalog/partition.c           | 14 ++++++++++++++
 src/test/regress/expected/alter_table.out | 14 ++++++++++++--
 src/test/regress/sql/alter_table.sql      |  9 +++++++++
 3 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c
index 73eff17..919b6b2 100644
--- a/src/backend/catalog/partition.c
+++ b/src/backend/catalog/partition.c
@@ -951,7 +951,21 @@ check_default_allows_bound(Relation parent, Relation default_rel,
 
 		/* Lock already taken above. */
 		if (part_relid != RelationGetRelid(default_rel))
+		{
 			part_rel = heap_open(part_relid, NoLock);
+
+			/* Can we skip scanning this part_rel? */
+			if (PartConstraintImpliedByRelConstraint(part_rel,
+													 def_part_constraints))
+			{
+				ereport(INFO,
+						(errmsg("partition constraint for table \"%s\" is implied by existing constraints",
+								RelationGetRelationName(part_rel))));
+
+				heap_close(part_rel, NoLock);
+				continue;
+			}
+		}
 		else
 			part_rel = default_rel;
 
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 0478a8a..32c4fda 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -3346,6 +3346,18 @@ INFO:  partition constraint for table "part_3_4" is implied by existing constrai
 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
+-- check if default partition child relation scan skipped
+DROP TABLE list_parted2_def;
+CREATE TABLE list_parted2_def PARTITION OF list_parted2 DEFAULT PARTITION BY RANGE (a);
+CREATE TABLE list_parted2_def_p1 PARTITION OF list_parted2_def FOR VALUES FROM (21) TO (30);
+CREATE TABLE list_parted2_def_p2 PARTITION OF list_parted2_def FOR VALUES FROM (31) TO (40);
+ALTER TABLE list_parted2_def_p1 ADD CONSTRAINT check_a CHECK (a IN (21, 22));
+ALTER TABLE list_parted2_def_p2 ADD CONSTRAINT check_a CHECK (a IN (31, 32));
+CREATE TABLE part_9 PARTITION OF list_parted2 FOR VALUES IN (9);
+INFO:  partition constraint for table "list_parted2_def_p1" is implied by existing constraints
+INFO:  partition constraint for table "list_parted2_def_p2" is implied by existing constraints
+CREATE TABLE part_31 PARTITION OF list_parted2 FOR VALUES IN (31);
+INFO:  partition constraint for table "list_parted2_def_p1" is implied by existing constraints
 -- check validation when attaching range partitions
 CREATE TABLE range_parted (
 	a int,
@@ -3436,7 +3448,6 @@ 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
 -- Same example, but check this time that the constraint correctly detects
 -- violating rows
 ALTER TABLE list_parted2 DETACH PARTITION part_7;
@@ -3450,7 +3461,6 @@ 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
 ERROR:  partition constraint is violated by some row
 -- check that leaf partitions of default partition are scanned when
 -- attaching a partitioned table.
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 37cca72..3ad6302 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -2163,6 +2163,15 @@ ALTER TABLE list_parted2 ATTACH PARTITION part_3_4 FOR VALUES IN (3, 4);
 -- 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);
+-- check if default partition child relation scan skipped
+DROP TABLE list_parted2_def;
+CREATE TABLE list_parted2_def PARTITION OF list_parted2 DEFAULT PARTITION BY RANGE (a);
+CREATE TABLE list_parted2_def_p1 PARTITION OF list_parted2_def FOR VALUES FROM (21) TO (30);
+CREATE TABLE list_parted2_def_p2 PARTITION OF list_parted2_def FOR VALUES FROM (31) TO (40);
+ALTER TABLE list_parted2_def_p1 ADD CONSTRAINT check_a CHECK (a IN (21, 22));
+ALTER TABLE list_parted2_def_p2 ADD CONSTRAINT check_a CHECK (a IN (31, 32));
+CREATE TABLE part_9 PARTITION OF list_parted2 FOR VALUES IN (9);
+CREATE TABLE part_31 PARTITION OF list_parted2 FOR VALUES IN (31);
 
 -- check validation when attaching range partitions
 CREATE TABLE range_parted (
-- 
2.7.4

Robert Haas

robertmhaas@gmail.com

over 8 years ago

In reply to: Jeevan Ladhe (#3)

Re: Optimise default partition scanning while adding new partition

On Thu, Sep 14, 2017 at 4:03 AM, Jeevan Ladhe
<jeevan.ladhe@enterprisedb.com> wrote:

Thanks Amit for reviewing.

Patch looks fine to me. By the way, why don't we just say "Can we skip
scanning part_rel?" in the comment before the newly added call to
PartConstraintImpliedByRelConstraint()? We don't need to repeat the
explanation of what it does at the every place we call it.

I have changed the comment.
PFA.

I'm probably missing something stupid, but why does this happen?

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
ERROR: partition constraint is violated by some row

Based on the regression test changes made up higher in the file, I'd
expect that line to be replaced by two lines, one for
list_parted2_def_p1 and another for list_parted2_def_p2, because at
this point, list_parted2_def is a partitioned table with those two
children, and they seem to have appropriate constraints.

list_parted2_def_p1 has
Check constraints:
"check_a" CHECK (a = ANY (ARRAY[21, 22]))

list_parted2_def_p2 has
Check constraints:
"check_a" CHECK (a = ANY (ARRAY[31, 32]))

Well, if a is 21 or 22 for the first, or 31 or 32 for the second, then
it's not 7. Or so I would think.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Amit Langote

Langote_Amit_f8@lab.ntt.co.jp

over 8 years ago

In reply to: Robert Haas (#4)

3 attachment(s)

Re: Optimise default partition scanning while adding new partition

On 2017/09/15 0:59, Robert Haas wrote:

On Thu, Sep 14, 2017 at 4:03 AM, Jeevan Ladhe
<jeevan.ladhe@enterprisedb.com> wrote:

Thanks Amit for reviewing.

Patch looks fine to me. By the way, why don't we just say "Can we skip
scanning part_rel?" in the comment before the newly added call to
PartConstraintImpliedByRelConstraint()? We don't need to repeat the
explanation of what it does at the every place we call it.

I have changed the comment.
PFA.

I'm probably missing something stupid, but why does this happen?

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
ERROR: partition constraint is violated by some row

Based on the regression test changes made up higher in the file, I'd
expect that line to be replaced by two lines, one for
list_parted2_def_p1 and another for list_parted2_def_p2, because at
this point, list_parted2_def is a partitioned table with those two
children, and they seem to have appropriate constraints.

list_parted2_def_p1 has
Check constraints:
"check_a" CHECK (a = ANY (ARRAY[21, 22]))

list_parted2_def_p2 has
Check constraints:
"check_a" CHECK (a = ANY (ARRAY[31, 32]))

Well, if a is 21 or 22 for the first, or 31 or 32 for the second, then
it's not 7. Or so I would think.

ISTM, Jeevan's patch modifies check_default_allows_bound(), which only
DefineRelation() calls as things stand today. IOW, it doesn't handle the
ATTACH PARTITION case, so you're not seeing the lines for
list_parted2_def_p1 and list_parted2_def_p2, because ATExecAttachPartition
doesn't yet know how to skip the validation scan for default partition's
children.

I wonder if we should call check_default_allows_bound() from
ATExecAttachPartition(), too, instead of validating updated default
partition constraint using ValidatePartitionConstraints()? That is, call
the latter only to validate the partition constraint of the table being
attached and call check_default_allows_bound() to validate the updated
default partition constraint. That way, INFO/ERROR messages related to
default partition constraint are consistent across the board. I hacked
that up in the attached 0001. The patch also modifies the INFO message
that check_default_allows_bound() emits when the scan is skipped to make
it apparent that a default partition is involved. (Sorry, this should've
been a review comment I should've posted before the default partition
patch was committed.)

Then apply 0002, which is Jeevan's patch. With 0001 in place, Robert's
complaint is taken care of.

Then comes 0003, which is my patch to teach ValidatePartitionConstraints()
to skip child table scans using their existing constraint.

Thanks,
Amit

Attachments:

0001-Teach-ATExecAttachPartition-to-use-check_default_all.patchtext/plain; charset=UTF-8; name=0001-Teach-ATExecAttachPartition-to-use-check_default_all.patchDownload

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

0002-Skip-scanning-default-partition-s-child-tables-if-po.patchtext/plain; charset=UTF-8; name=0002-Skip-scanning-default-partition-s-child-tables-if-po.patchDownload

From 37cf419684986b919dd1d563a358dd144919132a Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
Date: Fri, 15 Sep 2017 14:40:15 +0900
Subject: [PATCH 2/3] Skip scanning default partition's child tables if
 possible

Optimize check_default_allows_bound() such that the default
partition children constraints are checked against new partition
constraints for implication and avoid scan of the child of which
existing constraints are implied by new default partition
constraints. Also, added testcase for the same.

Jeevan Ladhe.
---
 src/backend/catalog/partition.c           | 14 ++++++++++++++
 src/test/regress/expected/alter_table.out | 26 ++++++++++++++++++++++++--
 src/test/regress/sql/alter_table.sql      |  9 +++++++++
 3 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c
index 027b98d850..ea88b70a8e 100644
--- a/src/backend/catalog/partition.c
+++ b/src/backend/catalog/partition.c
@@ -949,7 +949,21 @@ check_default_allows_bound(Relation parent, Relation default_rel,
 
 		/* Lock already taken above. */
 		if (part_relid != RelationGetRelid(default_rel))
+		{
 			part_rel = heap_open(part_relid, NoLock);
+
+			/* Can we skip scanning this part_rel? */
+			if (PartConstraintImpliedByRelConstraint(part_rel,
+													 def_part_constraints))
+			{
+				ereport(INFO,
+						(errmsg("updated partition constraint for default partition \"%s\" is implied by existing constraints",
+								RelationGetRelationName(part_rel))));
+
+				heap_close(part_rel, NoLock);
+				continue;
+			}
+		}
 		else
 			part_rel = default_rel;
 
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index eaf5f4b507..3af36db779 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -3346,6 +3346,18 @@ INFO:  partition constraint for table "part_3_4" is implied by existing constrai
 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:  updated partition constraint for default partition "list_parted2_def" is implied by existing constraints
+-- check if default partition child relation scan skipped
+DROP TABLE list_parted2_def;
+CREATE TABLE list_parted2_def PARTITION OF list_parted2 DEFAULT PARTITION BY RANGE (a);
+CREATE TABLE list_parted2_def_p1 PARTITION OF list_parted2_def FOR VALUES FROM (21) TO (30);
+CREATE TABLE list_parted2_def_p2 PARTITION OF list_parted2_def FOR VALUES FROM (31) TO (40);
+ALTER TABLE list_parted2_def_p1 ADD CONSTRAINT check_a CHECK (a IN (21, 22));
+ALTER TABLE list_parted2_def_p2 ADD CONSTRAINT check_a CHECK (a IN (31, 32));
+CREATE TABLE part_9 PARTITION OF list_parted2 FOR VALUES IN (9);
+INFO:  updated partition constraint for default partition "list_parted2_def_p1" is implied by existing constraints
+INFO:  updated partition constraint for default partition "list_parted2_def_p2" is implied by existing constraints
+CREATE TABLE part_31 PARTITION OF list_parted2 FOR VALUES IN (31);
+INFO:  updated partition constraint for default partition "list_parted2_def_p1" is implied by existing constraints
 -- check validation when attaching range partitions
 CREATE TABLE range_parted (
 	a int,
@@ -3393,18 +3405,24 @@ CREATE TABLE part_5 (
 CREATE TABLE part_5_a PARTITION OF part_5 FOR VALUES IN ('a');
 INSERT INTO part_5_a (a, b) VALUES (6, 'a');
 ALTER TABLE list_parted2 ATTACH PARTITION part_5 FOR VALUES IN (5);
+INFO:  updated partition constraint for default partition "list_parted2_def_p1" is implied by existing constraints
+INFO:  updated partition constraint for default partition "list_parted2_def_p2" is implied by existing constraints
 ERROR:  partition constraint is violated by some row
 -- delete the faulting row and also add a constraint to skip the scan
 DELETE FROM part_5_a WHERE a NOT IN (3);
 ALTER TABLE part_5 ADD CONSTRAINT check_a CHECK (a IS NOT NULL AND a = 5);
 ALTER TABLE list_parted2 ATTACH PARTITION part_5 FOR VALUES IN (5);
 INFO:  partition constraint for table "part_5" is implied by existing constraints
+INFO:  updated partition constraint for default partition "list_parted2_def_p1" is implied by existing constraints
+INFO:  updated partition constraint for default partition "list_parted2_def_p2" is implied by existing constraints
 ALTER TABLE list_parted2 DETACH PARTITION part_5;
 ALTER TABLE part_5 DROP CONSTRAINT check_a;
 -- scan should again be skipped, even though NOT NULL is now a column property
 ALTER TABLE part_5 ADD CONSTRAINT check_a CHECK (a IN (5)), ALTER a SET NOT NULL;
 ALTER TABLE list_parted2 ATTACH PARTITION part_5 FOR VALUES IN (5);
 INFO:  partition constraint for table "part_5" is implied by existing constraints
+INFO:  updated partition constraint for default partition "list_parted2_def_p1" is implied by existing constraints
+INFO:  updated partition constraint for default partition "list_parted2_def_p2" is implied by existing constraints
 -- Check the case where attnos of the partitioning columns in the table being
 -- attached differs from the parent.  It should not affect the constraint-
 -- checking logic that allows to skip the scan.
@@ -3416,6 +3434,8 @@ CREATE TABLE part_6 (
 ALTER TABLE part_6 DROP c;
 ALTER TABLE list_parted2 ATTACH PARTITION part_6 FOR VALUES IN (6);
 INFO:  partition constraint for table "part_6" is implied by existing constraints
+INFO:  updated partition constraint for default partition "list_parted2_def_p1" is implied by existing constraints
+INFO:  updated partition constraint for default partition "list_parted2_def_p2" is implied by existing constraints
 -- Similar to above, but the table being attached is a partitioned table
 -- whose partition has still different attnos for the root partitioning
 -- columns.
@@ -3436,7 +3456,8 @@ 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:  updated partition constraint for default partition "list_parted2_def" is implied by existing constraints
+INFO:  updated partition constraint for default partition "list_parted2_def_p1" is implied by existing constraints
+INFO:  updated partition constraint for default partition "list_parted2_def_p2" 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 +3471,8 @@ 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:  updated partition constraint for default partition "list_parted2_def" is implied by existing constraints
+INFO:  updated partition constraint for default partition "list_parted2_def_p1" is implied by existing constraints
+INFO:  updated partition constraint for default partition "list_parted2_def_p2" 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.
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 37cca72620..3ad6302f20 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -2163,6 +2163,15 @@ ALTER TABLE list_parted2 ATTACH PARTITION part_3_4 FOR VALUES IN (3, 4);
 -- 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);
+-- check if default partition child relation scan skipped
+DROP TABLE list_parted2_def;
+CREATE TABLE list_parted2_def PARTITION OF list_parted2 DEFAULT PARTITION BY RANGE (a);
+CREATE TABLE list_parted2_def_p1 PARTITION OF list_parted2_def FOR VALUES FROM (21) TO (30);
+CREATE TABLE list_parted2_def_p2 PARTITION OF list_parted2_def FOR VALUES FROM (31) TO (40);
+ALTER TABLE list_parted2_def_p1 ADD CONSTRAINT check_a CHECK (a IN (21, 22));
+ALTER TABLE list_parted2_def_p2 ADD CONSTRAINT check_a CHECK (a IN (31, 32));
+CREATE TABLE part_9 PARTITION OF list_parted2 FOR VALUES IN (9);
+CREATE TABLE part_31 PARTITION OF list_parted2 FOR VALUES IN (31);
 
 -- check validation when attaching range partitions
 CREATE TABLE range_parted (
-- 
2.11.0

0003-Teach-ValidatePartitionConstraints-to-skip-validatio.patchtext/plain; charset=UTF-8; name=0003-Teach-ValidatePartitionConstraints-to-skip-validatio.patchDownload

From af1cd6a655fe9eb3eac9d5fc51e1988296d45a83 Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
Date: Mon, 7 Aug 2017 10:51:47 +0900
Subject: [PATCH 3/3] Teach ValidatePartitionConstraints to skip validation in
 more cases

In cases where the table being attached is a partitioned table and
the table itself does not have constraints that would allow validation
on the whole table to be skipped, we can still skip the validations
of individual partitions if they each happen to have the requisite
constraints.

Per an idea of Robert Haas', with code refactoring suggestions from
Ashutosh Bapat.
---
 src/backend/commands/tablecmds.c          | 10 ++++++++++
 src/test/regress/expected/alter_table.out | 14 ++++++++++++++
 src/test/regress/sql/alter_table.sql      | 10 ++++++++++
 3 files changed, 34 insertions(+)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index ed4a2092b3..2f760bf355 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -13667,6 +13667,16 @@ ValidatePartitionConstraints(List **wqueue, Relation scanrel,
 			/* There can never be a whole-row reference here */
 			if (found_whole_row)
 				elog(ERROR, "unexpected whole-row reference found in partition key");
+
+			/* Can we skip scanning this part_rel? */
+			if (PartConstraintImpliedByRelConstraint(part_rel, my_partconstr))
+			{
+				ereport(INFO,
+						(errmsg("partition constraint for table \"%s\" is implied by existing constraints",
+								RelationGetRelationName(part_rel))));
+				heap_close(part_rel, NoLock);
+				continue;
+			}
 		}
 
 		/* Grab a work queue entry. */
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 3af36db779..5e6895c7d3 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -3486,6 +3486,20 @@ ERROR:  updated partition constraint for default partition "part5_def" would be
 -- 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');
+-- If the partitioned table being attached does not have a constraint that
+-- would allow validation scan to be skipped, but an individual partition
+-- does, then the partition's validation scan is skipped.  Note that the
+-- following leaf partition only allows rows that have a = 7 (and b = 'b' but
+-- that's irrelevant).
+CREATE TABLE part_7_b PARTITION OF part_7 (
+	CONSTRAINT check_a CHECK (a IS NOT NULL AND a = 7)
+) FOR VALUES IN ('b');
+-- The faulting row in part_7_a_null will still cause the command to fail
+ALTER TABLE list_parted2 ATTACH PARTITION part_7 FOR VALUES IN (7);
+INFO:  partition constraint for table "part_7_b" is implied by existing constraints
+INFO:  updated partition constraint for default partition "list_parted2_def_p1" is implied by existing constraints
+INFO:  updated partition constraint for default partition "list_parted2_def_p2" is implied by existing constraints
+ERROR:  partition constraint is violated by some row
 -- check that the table being attached is not already a partition
 ALTER TABLE list_parted2 ATTACH PARTITION part_2 FOR VALUES IN (2);
 ERROR:  "part_2" is already a partition
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 3ad6302f20..5f92486cb2 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -2286,6 +2286,16 @@ ALTER TABLE part_5 ATTACH PARTITION part5_p1 FOR VALUES IN ('y');
 -- 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');
+-- If the partitioned table being attached does not have a constraint that
+-- would allow validation scan to be skipped, but an individual partition
+-- does, then the partition's validation scan is skipped.  Note that the
+-- following leaf partition only allows rows that have a = 7 (and b = 'b' but
+-- that's irrelevant).
+CREATE TABLE part_7_b PARTITION OF part_7 (
+	CONSTRAINT check_a CHECK (a IS NOT NULL AND a = 7)
+) FOR VALUES IN ('b');
+-- The faulting row in part_7_a_null will still cause the command to fail
+ALTER TABLE list_parted2 ATTACH PARTITION part_7 FOR VALUES IN (7);
 
 -- check that the table being attached is not already a partition
 ALTER TABLE list_parted2 ATTACH PARTITION part_2 FOR VALUES IN (2);
-- 
2.11.0

Robert Haas

robertmhaas@gmail.com

over 8 years ago

In reply to: Amit Langote (#5)

Re: Optimise default partition scanning while adding new partition

On Fri, Sep 15, 2017 at 2:00 AM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:

I wonder if we should call check_default_allows_bound() from
ATExecAttachPartition(), too, instead of validating updated default
partition constraint using ValidatePartitionConstraints()? That is, call
the latter only to validate the partition constraint of the table being
attached and call check_default_allows_bound() to validate the updated
default partition constraint. That way, INFO/ERROR messages related to
default partition constraint are consistent across the board.

I believe the intended advantage of the current system is that if you
specify multiple operations in a single ALTER TABLE command, you only
do one scan rather than having a second scan per operation. If that's
currently working, we probably don't want to make it stop working.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Amit Langote

amitlangote09@gmail.com

over 8 years ago

In reply to: Robert Haas (#6)

Re: Optimise default partition scanning while adding new partition

On Sat, Sep 16, 2017 at 12:59 AM, Robert Haas <robertmhaas@gmail.com> wrote:

On Fri, Sep 15, 2017 at 2:00 AM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:

I wonder if we should call check_default_allows_bound() from
ATExecAttachPartition(), too, instead of validating updated default
partition constraint using ValidatePartitionConstraints()? That is, call
the latter only to validate the partition constraint of the table being
attached and call check_default_allows_bound() to validate the updated
default partition constraint. That way, INFO/ERROR messages related to
default partition constraint are consistent across the board.

I believe the intended advantage of the current system is that if you
specify multiple operations in a single ALTER TABLE command, you only
do one scan rather than having a second scan per operation. If that's
currently working, we probably don't want to make it stop working.

OK.

How about squash Jeevan's and my patch, so both
check_default_allows_bound() and ValidatePartitionConstraints() know
to scan default partition's children and there won't be any surprises
in the regression test output as you found after applying just the
Jeevan's patch. Unfortunately, I'm not able to post such a patch
right now.

Thanks,
Amit

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Amit Langote

Langote_Amit_f8@lab.ntt.co.jp

over 8 years ago

In reply to: Amit Langote (#7)

2 attachment(s)

Re: Optimise default partition scanning while adding new partition

On 2017/09/16 1:57, Amit Langote wrote:

On Sat, Sep 16, 2017 at 12:59 AM, Robert Haas <robertmhaas@gmail.com> wrote:

I believe the intended advantage of the current system is that if you
specify multiple operations in a single ALTER TABLE command, you only
do one scan rather than having a second scan per operation. If that's
currently working, we probably don't want to make it stop working.

OK.

How about squash Jeevan's and my patch, so both
check_default_allows_bound() and ValidatePartitionConstraints() know
to scan default partition's children and there won't be any surprises
in the regression test output as you found after applying just the
Jeevan's patch. Unfortunately, I'm not able to post such a patch
right now.

I guess we don't need to squash, as they could be seen as implementing
different features. Reordering the patches helps though. So, apply them
in this order:

1. My patch to teach ValidatePartitionConstraints() to skip scanning
a partition's own partitions, which optimizes ATTACH PARTITION
command's partition constraint validation scan (this also covers the
case of scanning the default partition to validate its updated
constraint when attaching a new partition)

2. Jeevan's patch to teach check_default_allows_bound() to skip scanning
the default partition's own partitions, which covers the case of
scanning the default partition to validate its updated constraint when
adding a new partition using CREATE TABLE

Attached 0001 and 0002 are ordered that way.

In addition to implementing the features mentioned in 1 and 2 above, the
patches also modify the INFO message to mention "updated partition
constraint for default partition \"%s\"", instead of "partition constraint
for table \"%s\"", when the default partition is involved. That's so that
it's consistent with the error message that would be emitted by either
check_default_allows_bound() or ATRewriteTable() when the scan finds a row
that violates updated default partition constraint, viz. the following:
"updated partition constraint for default partition \"%s\" would be
violated by some row"

Thoughts?

Thanks,
Amit

Attachments:

0001-Teach-ValidatePartitionConstraints-to-skip-validatio.patchtext/plain; charset=UTF-8; name=0001-Teach-ValidatePartitionConstraints-to-skip-validatio.patchDownload

From 166cc082b9d652e186df4ef7b6c41976a22e55a8 Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
Date: Mon, 7 Aug 2017 10:51:47 +0900
Subject: [PATCH 1/2] Teach ValidatePartitionConstraints to skip validation in
 more cases

In cases where the table being attached is a partitioned table and
the table itself does not have constraints that would allow validation
on the whole table to be skipped, we can still skip the validations
of individual partitions if they each happen to have the requisite
constraints.

Idea by Robert Haas.
---
 src/backend/commands/tablecmds.c          | 26 +++++++++++++++++++++++---
 src/test/regress/expected/alter_table.out | 17 +++++++++++++++--
 src/test/regress/sql/alter_table.sql      | 10 ++++++++++
 3 files changed, 48 insertions(+), 5 deletions(-)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 563bcda30c..2d4dcd7556 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -13635,9 +13635,14 @@ ValidatePartitionConstraints(List **wqueue, Relation scanrel,
 	 */
 	if (PartConstraintImpliedByRelConstraint(scanrel, partConstraint))
 	{
-		ereport(INFO,
-				(errmsg("partition constraint for table \"%s\" is implied by existing constraints",
-						RelationGetRelationName(scanrel))));
+		if (!validate_default)
+			ereport(INFO,
+					(errmsg("partition constraint for table \"%s\" is implied by existing constraints",
+							RelationGetRelationName(scanrel))));
+		else
+			ereport(INFO,
+					(errmsg("updated partition constraint for default partition \"%s\" is implied by existing constraints",
+							RelationGetRelationName(scanrel))));
 		return;
 	}
 
@@ -13678,6 +13683,21 @@ ValidatePartitionConstraints(List **wqueue, Relation scanrel,
 			/* There can never be a whole-row reference here */
 			if (found_whole_row)
 				elog(ERROR, "unexpected whole-row reference found in partition key");
+
+			/* Can we skip scanning this part_rel? */
+			if (PartConstraintImpliedByRelConstraint(part_rel, my_partconstr))
+			{
+				if (!validate_default)
+					ereport(INFO,
+							(errmsg("partition constraint for table \"%s\" is implied by existing constraints",
+									RelationGetRelationName(part_rel))));
+				else
+					ereport(INFO,
+							(errmsg("updated partition constraint for default partition \"%s\" is implied by existing constraints",
+									RelationGetRelationName(part_rel))));
+				heap_close(part_rel, NoLock);
+				continue;
+			}
 		}
 
 		/* Grab a work queue entry. */
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 0478a8ac60..fc644c12ae 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -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.
@@ -3464,6 +3464,19 @@ ERROR:  updated partition constraint for default partition would be violated by
 -- 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');
+-- If the partitioned table being attached does not have a constraint that
+-- would allow validation scan to be skipped, but an individual partition
+-- does, then the partition's validation scan is skipped.  Note that the
+-- following leaf partition only allows rows that have a = 7 (and b = 'b' but
+-- that's irrelevant).
+CREATE TABLE part_7_b PARTITION OF part_7 (
+	CONSTRAINT check_a CHECK (a IS NOT NULL AND a = 7)
+) FOR VALUES IN ('b');
+-- The faulting row in part_7_a_null will still cause the command to fail
+ALTER TABLE list_parted2 ATTACH PARTITION part_7 FOR VALUES IN (7);
+INFO:  partition constraint for table "part_7_b" 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 the table being attached is not already a partition
 ALTER TABLE list_parted2 ATTACH PARTITION part_2 FOR VALUES IN (2);
 ERROR:  "part_2" is already a partition
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 37cca72620..d296314ca9 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -2277,6 +2277,16 @@ ALTER TABLE part_5 ATTACH PARTITION part5_p1 FOR VALUES IN ('y');
 -- 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');
+-- If the partitioned table being attached does not have a constraint that
+-- would allow validation scan to be skipped, but an individual partition
+-- does, then the partition's validation scan is skipped.  Note that the
+-- following leaf partition only allows rows that have a = 7 (and b = 'b' but
+-- that's irrelevant).
+CREATE TABLE part_7_b PARTITION OF part_7 (
+	CONSTRAINT check_a CHECK (a IS NOT NULL AND a = 7)
+) FOR VALUES IN ('b');
+-- The faulting row in part_7_a_null will still cause the command to fail
+ALTER TABLE list_parted2 ATTACH PARTITION part_7 FOR VALUES IN (7);
 
 -- check that the table being attached is not already a partition
 ALTER TABLE list_parted2 ATTACH PARTITION part_2 FOR VALUES IN (2);
-- 
2.11.0

0002-Skip-scanning-default-partition-s-child-tables-if-po.patchtext/plain; charset=UTF-8; name=0002-Skip-scanning-default-partition-s-child-tables-if-po.patchDownload

From 619fa7dc53bea151201ef5d5893c4df039a073a1 Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
Date: Fri, 15 Sep 2017 14:40:15 +0900
Subject: [PATCH 2/2] Skip scanning default partition's child tables if
 possible

Optimize check_default_allows_bound() such that the default
partition children constraints are checked against new partition
constraints for implication and avoid scan of the child of which
existing constraints are implied by new default partition
constraints. Also, added testcase for the same.

Jeevan Ladhe.
---
 src/backend/catalog/partition.c           | 16 +++++++++++++++-
 src/test/regress/expected/alter_table.out | 31 +++++++++++++++++++++++++++----
 src/test/regress/sql/alter_table.sql      |  9 +++++++++
 3 files changed, 51 insertions(+), 5 deletions(-)

diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c
index 1ab6dba7ae..25ef0f39d2 100644
--- a/src/backend/catalog/partition.c
+++ b/src/backend/catalog/partition.c
@@ -920,7 +920,7 @@ check_default_allows_bound(Relation parent, Relation 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;
 	}
@@ -953,7 +953,21 @@ check_default_allows_bound(Relation parent, Relation default_rel,
 
 		/* Lock already taken above. */
 		if (part_relid != RelationGetRelid(default_rel))
+		{
 			part_rel = heap_open(part_relid, NoLock);
+
+			/* Can we skip scanning this part_rel? */
+			if (PartConstraintImpliedByRelConstraint(part_rel,
+													 def_part_constraints))
+			{
+				ereport(INFO,
+						(errmsg("updated partition constraint for default partition \"%s\" is implied by existing constraints",
+								RelationGetRelationName(part_rel))));
+
+				heap_close(part_rel, NoLock);
+				continue;
+			}
+		}
 		else
 			part_rel = default_rel;
 
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index fc644c12ae..77b05ee5e5 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -3345,7 +3345,19 @@ 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 if default partition child relation scan skipped
+DROP TABLE list_parted2_def;
+CREATE TABLE list_parted2_def PARTITION OF list_parted2 DEFAULT PARTITION BY RANGE (a);
+CREATE TABLE list_parted2_def_p1 PARTITION OF list_parted2_def FOR VALUES FROM (21) TO (30);
+CREATE TABLE list_parted2_def_p2 PARTITION OF list_parted2_def FOR VALUES FROM (31) TO (40);
+ALTER TABLE list_parted2_def_p1 ADD CONSTRAINT check_a CHECK (a IN (21, 22));
+ALTER TABLE list_parted2_def_p2 ADD CONSTRAINT check_a CHECK (a IN (31, 32));
+CREATE TABLE part_9 PARTITION OF list_parted2 FOR VALUES IN (9);
+INFO:  updated partition constraint for default partition "list_parted2_def_p1" is implied by existing constraints
+INFO:  updated partition constraint for default partition "list_parted2_def_p2" is implied by existing constraints
+CREATE TABLE part_31 PARTITION OF list_parted2 FOR VALUES IN (31);
+INFO:  updated partition constraint for default partition "list_parted2_def_p1" is implied by existing constraints
 -- check validation when attaching range partitions
 CREATE TABLE range_parted (
 	a int,
@@ -3393,18 +3405,24 @@ CREATE TABLE part_5 (
 CREATE TABLE part_5_a PARTITION OF part_5 FOR VALUES IN ('a');
 INSERT INTO part_5_a (a, b) VALUES (6, 'a');
 ALTER TABLE list_parted2 ATTACH PARTITION part_5 FOR VALUES IN (5);
+INFO:  updated partition constraint for default partition "list_parted2_def_p1" is implied by existing constraints
+INFO:  updated partition constraint for default partition "list_parted2_def_p2" is implied by existing constraints
 ERROR:  partition constraint is violated by some row
 -- delete the faulting row and also add a constraint to skip the scan
 DELETE FROM part_5_a WHERE a NOT IN (3);
 ALTER TABLE part_5 ADD CONSTRAINT check_a CHECK (a IS NOT NULL AND a = 5);
 ALTER TABLE list_parted2 ATTACH PARTITION part_5 FOR VALUES IN (5);
 INFO:  partition constraint for table "part_5" is implied by existing constraints
+INFO:  updated partition constraint for default partition "list_parted2_def_p1" is implied by existing constraints
+INFO:  updated partition constraint for default partition "list_parted2_def_p2" is implied by existing constraints
 ALTER TABLE list_parted2 DETACH PARTITION part_5;
 ALTER TABLE part_5 DROP CONSTRAINT check_a;
 -- scan should again be skipped, even though NOT NULL is now a column property
 ALTER TABLE part_5 ADD CONSTRAINT check_a CHECK (a IN (5)), ALTER a SET NOT NULL;
 ALTER TABLE list_parted2 ATTACH PARTITION part_5 FOR VALUES IN (5);
 INFO:  partition constraint for table "part_5" is implied by existing constraints
+INFO:  updated partition constraint for default partition "list_parted2_def_p1" is implied by existing constraints
+INFO:  updated partition constraint for default partition "list_parted2_def_p2" is implied by existing constraints
 -- Check the case where attnos of the partitioning columns in the table being
 -- attached differs from the parent.  It should not affect the constraint-
 -- checking logic that allows to skip the scan.
@@ -3416,6 +3434,8 @@ CREATE TABLE part_6 (
 ALTER TABLE part_6 DROP c;
 ALTER TABLE list_parted2 ATTACH PARTITION part_6 FOR VALUES IN (6);
 INFO:  partition constraint for table "part_6" is implied by existing constraints
+INFO:  updated partition constraint for default partition "list_parted2_def_p1" is implied by existing constraints
+INFO:  updated partition constraint for default partition "list_parted2_def_p2" is implied by existing constraints
 -- Similar to above, but the table being attached is a partitioned table
 -- whose partition has still different attnos for the root partitioning
 -- columns.
@@ -3436,7 +3456,8 @@ 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:  updated partition constraint for default partition "list_parted2_def" is implied by existing constraints
+INFO:  updated partition constraint for default partition "list_parted2_def_p1" is implied by existing constraints
+INFO:  updated partition constraint for default partition "list_parted2_def_p2" 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 +3471,8 @@ 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:  updated partition constraint for default partition "list_parted2_def" is implied by existing constraints
+INFO:  updated partition constraint for default partition "list_parted2_def_p1" is implied by existing constraints
+INFO:  updated partition constraint for default partition "list_parted2_def_p2" 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.
@@ -3475,7 +3497,8 @@ CREATE TABLE part_7_b PARTITION OF part_7 (
 -- The faulting row in part_7_a_null will still cause the command to fail
 ALTER TABLE list_parted2 ATTACH PARTITION part_7 FOR VALUES IN (7);
 INFO:  partition constraint for table "part_7_b" is implied by existing constraints
-INFO:  updated partition constraint for default partition "list_parted2_def" is implied by existing constraints
+INFO:  updated partition constraint for default partition "list_parted2_def_p1" is implied by existing constraints
+INFO:  updated partition constraint for default partition "list_parted2_def_p2" is implied by existing constraints
 ERROR:  partition constraint is violated by some row
 -- check that the table being attached is not already a partition
 ALTER TABLE list_parted2 ATTACH PARTITION part_2 FOR VALUES IN (2);
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index d296314ca9..5f92486cb2 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -2163,6 +2163,15 @@ ALTER TABLE list_parted2 ATTACH PARTITION part_3_4 FOR VALUES IN (3, 4);
 -- 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);
+-- check if default partition child relation scan skipped
+DROP TABLE list_parted2_def;
+CREATE TABLE list_parted2_def PARTITION OF list_parted2 DEFAULT PARTITION BY RANGE (a);
+CREATE TABLE list_parted2_def_p1 PARTITION OF list_parted2_def FOR VALUES FROM (21) TO (30);
+CREATE TABLE list_parted2_def_p2 PARTITION OF list_parted2_def FOR VALUES FROM (31) TO (40);
+ALTER TABLE list_parted2_def_p1 ADD CONSTRAINT check_a CHECK (a IN (21, 22));
+ALTER TABLE list_parted2_def_p2 ADD CONSTRAINT check_a CHECK (a IN (31, 32));
+CREATE TABLE part_9 PARTITION OF list_parted2 FOR VALUES IN (9);
+CREATE TABLE part_31 PARTITION OF list_parted2 FOR VALUES IN (31);
 
 -- check validation when attaching range partitions
 CREATE TABLE range_parted (
-- 
2.11.0

Robert Haas

robertmhaas@gmail.com

over 8 years ago

In reply to: Amit Langote (#8)

Re: Optimise default partition scanning while adding new partition

On Tue, Sep 26, 2017 at 4:27 AM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:

On 2017/09/16 1:57, Amit Langote wrote:

On Sat, Sep 16, 2017 at 12:59 AM, Robert Haas <robertmhaas@gmail.com> wrote:

I believe the intended advantage of the current system is that if you
specify multiple operations in a single ALTER TABLE command, you only
do one scan rather than having a second scan per operation. If that's
currently working, we probably don't want to make it stop working.

OK.

How about squash Jeevan's and my patch, so both
check_default_allows_bound() and ValidatePartitionConstraints() know
to scan default partition's children and there won't be any surprises
in the regression test output as you found after applying just the
Jeevan's patch. Unfortunately, I'm not able to post such a patch
right now.

I guess we don't need to squash, as they could be seen as implementing
different features. Reordering the patches helps though. So, apply them
in this order:

1. My patch to teach ValidatePartitionConstraints() to skip scanning
a partition's own partitions, which optimizes ATTACH PARTITION
command's partition constraint validation scan (this also covers the
case of scanning the default partition to validate its updated
constraint when attaching a new partition)

2. Jeevan's patch to teach check_default_allows_bound() to skip scanning
the default partition's own partitions, which covers the case of
scanning the default partition to validate its updated constraint when
adding a new partition using CREATE TABLE

Attached 0001 and 0002 are ordered that way.

OK, I pushed all of this, spread out over 3 commits. I reworked the
test cases not to be entangled with the existing test cases, and also
to test both of these highly-related features together. Hopefully you
like the result.

Thanks for the patches and the analysis.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#10

Amit Langote

Langote_Amit_f8@lab.ntt.co.jp

over 8 years ago

In reply to: Robert Haas (#9)

1 attachment(s)

Re: Optimise default partition scanning while adding new partition

On 2017/10/06 2:25, Robert Haas wrote:

On Tue, Sep 26, 2017 at 4:27 AM, Amit Langote wrote:

I guess we don't need to squash, as they could be seen as implementing
different features. Reordering the patches helps though. So, apply them
in this order:

1. My patch to teach ValidatePartitionConstraints() to skip scanning
a partition's own partitions, which optimizes ATTACH PARTITION
command's partition constraint validation scan (this also covers the
case of scanning the default partition to validate its updated
constraint when attaching a new partition)

2. Jeevan's patch to teach check_default_allows_bound() to skip scanning
the default partition's own partitions, which covers the case of
scanning the default partition to validate its updated constraint when
adding a new partition using CREATE TABLE

Attached 0001 and 0002 are ordered that way.

OK, I pushed all of this, spread out over 3 commits. I reworked the
test cases not to be entangled with the existing test cases, and also
to test both of these highly-related features together. Hopefully you
like the result.

Thanks for committing.

I noticed that 6476b26115f3 doesn't use the same INFO message as
14f67a8ee28. You can notice in the following example that the message
emitted (that the default partition scan is skipped) is different when a
new partition is added with CREATE TABLE and with ATTACH PARTITION,
respectively.

create table foo (a int, b char) partition by list (a);

-- default partition
create table food partition of foo default partition by list (b);

-- default partition's partition with the check constraint
create table fooda partition of food (check (not (a is not null and a = 1
and a = 2))) for values in ('a');

create table foo1 partition of foo for values in (1);
INFO: partition constraint for table "fooda" is implied by existing
constraints
CREATE TABLE

create table foo2 (like foo);
alter table foo attach partition foo2 for values in (2);
INFO: updated partition constraint for default partition "fooda" is
implied by existing constraints
ALTER TABLE

That's because it appears that you applied Jeevan's original patch.
Revised version of his patch that I had last posted contained the new
message. Actually the revised patch had not only addressed the case where
the default partition's child's scan is skipped, but also the case where
the scan of default partition itself is skipped. As things stand now:

alter table food add constraint skip_check check (not (a is not null and
(a = any (array[1, 2]))));

create table foo1 partition of foo for values in (1);
INFO: partition constraint for table "food" is implied by existing
constraints
CREATE TABLE

create table foo2 (like foo);
CREATE TABLE

alter table foo attach partition foo2 for values in (2);
INFO: updated partition constraint for default partition "food" is
implied by existing constraints
ALTER TABLE

Attached a patch to modify the INFO messages in check_default_allows_bound.

Thanks,
Amit

Attachments:

0001-Like-c31e9d4bafd-but-for-check_default_allows_bound.patchtext/plain; charset=UTF-8; name=0001-Like-c31e9d4bafd-but-for-check_default_allows_bound.patchDownload

From 1bd3b03bcd1f8af6cec3a22c40e96c9cde46e705 Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
Date: Fri, 6 Oct 2017 10:23:24 +0900
Subject: [PATCH] Like c31e9d4bafd, but for check_default_allows_bound

---
 src/backend/catalog/partition.c           | 10 +++-------
 src/test/regress/expected/alter_table.out |  4 ++--
 2 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c
index 3a8306a055..1459fba778 100644
--- a/src/backend/catalog/partition.c
+++ b/src/backend/catalog/partition.c
@@ -920,7 +920,7 @@ check_default_allows_bound(Relation parent, Relation 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;
 	}
@@ -956,16 +956,12 @@ check_default_allows_bound(Relation parent, Relation default_rel,
 		{
 			part_rel = heap_open(part_relid, NoLock);
 
-			/*
-			 * If the partition constraints on default partition child imply
-			 * that it will not contain any row that would belong to the new
-			 * partition, we can avoid scanning the child table.
-			 */
+			/* Can we skip scanning this part_rel? */
 			if (PartConstraintImpliedByRelConstraint(part_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(part_rel))));
 
 				heap_close(part_rel, NoLock);
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index dbe438dcd4..98f4db1f85 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -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,
@@ -3492,7 +3492,7 @@ DROP TABLE quuux1, quuux2;
 -- should validate for quuux1, but not for quuux2
 CREATE TABLE quuux1 PARTITION OF quuux FOR VALUES IN (1);
 CREATE TABLE quuux2 PARTITION OF quuux FOR VALUES IN (2);
-INFO:  partition constraint for table "quuux_default1" is implied by existing constraints
+INFO:  updated partition constraint for default partition "quuux_default1" is implied by existing constraints
 DROP TABLE quuux;
 --
 -- DETACH PARTITION
-- 
2.11.0

#11

Robert Haas

robertmhaas@gmail.com

over 8 years ago

In reply to: Amit Langote (#10)

Re: Optimise default partition scanning while adding new partition

On Thu, Oct 5, 2017 at 9:29 PM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:

Attached a patch to modify the INFO messages in check_default_allows_bound.

Committed. However, I didn't see a reason to adopt the comment change
you proposed, so I left that part out.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#12

Amit Langote

Langote_Amit_f8@lab.ntt.co.jp

over 8 years ago

In reply to: Robert Haas (#11)

Re: Optimise default partition scanning while adding new partition

On 2017/10/13 4:18, Robert Haas wrote:

On Thu, Oct 5, 2017 at 9:29 PM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:

Attached a patch to modify the INFO messages in check_default_allows_bound.

Committed. However, I didn't see a reason to adopt the comment change
you proposed, so I left that part out.

Okay, thanks.

Regards,
Amit

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers