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
From 1bd3b03bcd1f8af6cec3a22c40e96c9cde46e705 Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
