On 2017/08/07 11:05, Amit Langote wrote:
> By the way, bulk of 0004 is refactoring which it seems is what Jeevan's
> default partition patch set also includes as one of the patches [1]. It
> got a decent amount review from Ashutosh. I broke it down into a separate
> patch, so that the patch to add the new feature is its own tiny patch.
>
> I also spotted a couple of comments referring to attachRel that we just
> recently renamed.
>
> So, attached are:
>
> 0001: s/attachRel/attachrel/g
> 0002: Refactoring to introduce a PartConstraintImpliedByRelConstraint
> 0003: Add the feature to skip the scan of individual leaf partitions
Since Jeevan Ladhe mentioned this patch [1] earlier this week, sending the
rebased patches here for consideration. Actually there are only 2 patches
now, because 0002 above is rendered unnecessary by ecfe59e50fb [2].
Thanks,
Amit
[1]
https://www.postgresql.org/message-id/CAOgcT0MWwG8WBw8frFMtRYHAgDD=tpt6u7wcso_l2k0kypm...@mail.gmail.com
[2] https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=ecfe59e50fb
From 55e1e14a821de541c2d24c152c193bf57eb91d43 Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@gmail.com>
Date: Mon, 7 Aug 2017 10:45:39 +0900
Subject: [PATCH 1/2] Typo: attachRel is now attachrel
---
src/backend/commands/tablecmds.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 96354bdee5..563bcda30c 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -13779,7 +13779,7 @@ ATExecAttachPartition(List **wqueue, Relation rel,
PartitionCmd *cmd)
* Prevent circularity by seeing if rel is a partition of attachrel. (In
* particular, this disallows making a rel a partition of itself.)
*
- * We do that by checking if rel is a member of the list of attachRel's
+ * We do that by checking if rel is a member of the list of attachrel's
* partitions provided the latter is partitioned at all. We want to
avoid
* having to construct this list again, so we request the strongest lock
* on all partitions. We need the strongest lock, because we may decide
--
2.11.0
From a7d0d781bd9e3730f90d902d0e09abf79962f872 Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@gmail.com>
Date: Mon, 7 Aug 2017 10:51:47 +0900
Subject: [PATCH 2/2] Teach ATExecAttachPartition 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 | 13 +++++++++++++
src/test/regress/sql/alter_table.sql | 10 ++++++++++
3 files changed, 33 insertions(+)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 563bcda30c..901eea7fe2 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -13678,6 +13678,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");
+
+ /* Check if we 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 0478a8ac60..e3415837b6 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -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: partition constraint for table "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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
