Hi David,
Thanks for checking.
On 2019/03/20 19:41, David Rowley wrote:
> On Wed, 20 Mar 2019 at 17:37, Amit Langote
> <langote_amit...@lab.ntt.co.jp> wrote:
>> That's because get_relation_constraints() no longer (as of PG 11) includes
>> the partition constraint for SELECT queries. But that's based on an
>> assumption that partitions are always accessed via parent, so partition
>> pruning would make loading the partition constraint unnecessary. That's
>> not always true, as shown in the above example.
>>
>> Should we fix that? I'm attaching a patch here.
>
> Perhaps we should. The constraint_exclusion documents [1] just mention:
>
>> Controls the query planner's use of table constraints to optimize queries.
>
> and I'm pretty sure you could class the partition constraint as a
> table constraint.
Yes.
> As for the patch:
>
> + if ((root->parse->commandType == CMD_SELECT && !IS_OTHER_REL(rel)) ||
>
> Shouldn't this really be checking rel->reloptkind == RELOPT_BASEREL
> instead of !IS_OTHER_REL(rel) ?
Hmm, thought I'd use the macro if we have one, but I'll change as you
suggest if that's what makes the code easier to follow. As you might
know, we can only get "simple" relations here.
> For the comments:
>
> + * For selects, we only need those if the partition is directly mentioned
> + * in the query, that is not via parent. In case of the latter, partition
> + * pruning, which uses the parent table's partition bound descriptor,
> + * ensures that we only consider partitions whose partition constraint
> + * satisfy the query quals (or, the two don't contradict each other), so
> + * loading them is pointless.
> + *
> + * For updates and deletes, we always need those for performing partition
> + * pruning using constraint exclusion, but, only if pruning is enabled.
>
> You mention "the latter", normally you'd only do that if there was a
> former, but in this case there's not.
I was trying to go for "accessing partition directly" as the former and
"accessing it via the parent" as the latter, but maybe the sentence as
written cannot be read that way.
> How about just making it:
>
> /*
> * Append partition predicates, if any.
> *
> * For selects, partition pruning uses the parent table's partition bound
> * descriptor, so there's no need to include the partition constraint for
> * this case. However, if the partition is referenced directly in the query
> * then no partition pruning will occur, so we'll include it in the case.
> */
> if ((root->parse->commandType != CMD_SELECT && enable_partition_pruning) ||
> (root->parse->commandType == CMD_SELECT && rel->reloptkind ==
> RELOPT_BASEREL))
OK, I will use this text.
> For the tests, it seems excessive to create some new tables for this.
> Won't the tables in the previous test work just fine?
OK, I have revised the tests to use existing tables.
I'll add this to July fest to avoid forgetting about this.
Thanks,
Amit
From 2fadf7c9cb35f3993e2d9cf91f8cd580fe5f59fb Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@gmail.com>
Date: Wed, 20 Mar 2019 13:27:37 +0900
Subject: [PATCH v2] Fix planner to load partition constraint in some cases
For select queries that access a partition directly, we should
load the partition constraint so that constraint exclusion can
use it to exclude the partition based on query quals. When
partitions are accessed indirectly via the parent table, it's
unnecessary to load the partition constraint, because partition
pruning will only select those partitions whose partition
constraint satisfies query quals, making it unnecessary to run
constraint exclusion on partitions.
---
src/backend/optimizer/util/plancat.c | 10 +++++--
src/test/regress/expected/partition_prune.out | 41 +++++++++++++++++++++++++++
src/test/regress/sql/partition_prune.sql | 18 ++++++++++++
3 files changed, 66 insertions(+), 3 deletions(-)
diff --git a/src/backend/optimizer/util/plancat.c
b/src/backend/optimizer/util/plancat.c
index 30f4dc151b..ce38a50afb 100644
--- a/src/backend/optimizer/util/plancat.c
+++ b/src/backend/optimizer/util/plancat.c
@@ -1270,10 +1270,14 @@ get_relation_constraints(PlannerInfo *root,
* Append partition predicates, if any.
*
* For selects, partition pruning uses the parent table's partition
bound
- * descriptor, instead of constraint exclusion which is driven by the
- * individual partition's partition constraint.
+ * descriptor, so there's no need to include the partition constraint
for
+ * this case. However, if the partition is referenced directly in the
+ * query then no partition pruning will occur, so we'll include it in
that
+ * case.
*/
- if (enable_partition_pruning && root->parse->commandType != CMD_SELECT)
+ if ((root->parse->commandType != CMD_SELECT &&
enable_partition_pruning) ||
+ (root->parse->commandType == CMD_SELECT &&
+ rel->reloptkind == RELOPT_BASEREL))
{
List *pcqual = RelationGetPartitionQual(relation);
diff --git a/src/test/regress/expected/partition_prune.out
b/src/test/regress/expected/partition_prune.out
index 30946f77b6..dd2a3e27db 100644
--- a/src/test/regress/expected/partition_prune.out
+++ b/src/test/regress/expected/partition_prune.out
@@ -3637,4 +3637,45 @@ select * from listp where a = (select 2) and b <> 10;
Filter: ((b <> 10) AND (a = $0))
(5 rows)
+--
+-- check that a partition directly accessed in a query is excluded with
+-- constraint_exclusion = on
+--
+-- turn off partition pruning, so that it doesn't interfere
+set enable_partition_pruning to off;
+-- constraint exclusion doesn't apply
+set constraint_exclusion to 'partition';
+explain (costs off) select * from listp1 where a = 2;
+ QUERY PLAN
+--------------------
+ Seq Scan on listp1
+ Filter: (a = 2)
+(2 rows)
+
+explain (costs off) select * from listp2 where a = 1;
+ QUERY PLAN
+-----------------------------
+ Append
+ -> Seq Scan on listp2_10
+ Filter: (a = 1)
+(3 rows)
+
+-- constraint exclusion applies
+set constraint_exclusion to 'on';
+explain (costs off) select * from listp1 where a = 2;
+ QUERY PLAN
+--------------------------
+ Result
+ One-Time Filter: false
+(2 rows)
+
+explain (costs off) select * from listp2 where a = 1;
+ QUERY PLAN
+--------------------------
+ Result
+ One-Time Filter: false
+(2 rows)
+
+reset constraint_exclusion;
+reset enable_partition_pruning;
drop table listp;
diff --git a/src/test/regress/sql/partition_prune.sql
b/src/test/regress/sql/partition_prune.sql
index dc327caffd..6650188918 100644
--- a/src/test/regress/sql/partition_prune.sql
+++ b/src/test/regress/sql/partition_prune.sql
@@ -985,4 +985,22 @@ create table listp2_10 partition of listp2 for values in
(10);
explain (analyze, costs off, summary off, timing off)
select * from listp where a = (select 2) and b <> 10;
+--
+-- check that a partition directly accessed in a query is excluded with
+-- constraint_exclusion = on
+--
+
+-- turn off partition pruning, so that it doesn't interfere
+set enable_partition_pruning to off;
+
+-- constraint exclusion doesn't apply
+set constraint_exclusion to 'partition';
+explain (costs off) select * from listp1 where a = 2;
+explain (costs off) select * from listp2 where a = 1;
+-- constraint exclusion applies
+set constraint_exclusion to 'on';
+explain (costs off) select * from listp1 where a = 2;
+explain (costs off) select * from listp2 where a = 1;
+reset constraint_exclusion;
+reset enable_partition_pruning;
drop table listp;
--
2.11.0
