Re: selecting from partitions and constraint exclusion
Hi Thibaut, On 2019/04/06 1:12, Thibaut wrote: > Le 25/03/2019 à 01:31, Amit Langote a écrit : >> On 2019/03/22 17:17, Amit Langote wrote: >>> I'll add this to July fest to avoid forgetting about this. >> I'd forgotten to do this, but done today. :) >> >> Thanks, >> Amit > > Hello Amit, > > Just a quick information that your last patch does not apply on head: > > $ git apply > ~/Téléchargements/v2-0001-Fix-planner-to-load-partition-constraint-in-some-.patch > error: patch failed: src/test/regress/expected/partition_prune.out:3637 > error: src/test/regress/expected/partition_prune.out: patch does not apply > > Manually applying it on top of Hosoya's last 2 patches, It corrects the > different cases we found so far. > I will keep on testing next week. Thanks for the heads up. We are discussing this and another related matter on a different thread (titled "speeding up planning with partitions" [1]). Maybe, the problem originally reported here will get resolved there once we reach consensus first on what to do in the HEAD branch and what's back-patchable as a bug-fix to the PG 11 branch. [1] https://www.postgresql.org/message-id/50415da6-0258-d135-2ba4-197041b57c5b%40lab.ntt.co.jp
Re: selecting from partitions and constraint exclusion
Le 25/03/2019 à 01:31, Amit Langote a écrit : > On 2019/03/22 17:17, Amit Langote wrote: >> I'll add this to July fest to avoid forgetting about this. > I'd forgotten to do this, but done today. :) > > Thanks, > Amit Hello Amit, Just a quick information that your last patch does not apply on head: $ git apply ~/Téléchargements/v2-0001-Fix-planner-to-load-partition-constraint-in-some-.patch error: patch failed: src/test/regress/expected/partition_prune.out:3637 error: src/test/regress/expected/partition_prune.out: patch does not apply Manually applying it on top of Hosoya's last 2 patches, It corrects the different cases we found so far. I will keep on testing next week. Cordialement, Thibaut
Re: selecting from partitions and constraint exclusion
On 2019/03/26 0:21, Robert Haas wrote: > On Wed, Mar 20, 2019 at 12:37 AM Amit Langote > wrote: >> That's because get_relation_constraints() no longer (as of PG 11) includes >> the partition constraint for SELECT queries. > > What commit made that change? That would be 9fdb675fc5d2 (faster partition pruning) that got into PG 11. > This sounds to me like maybe it should be an open item. I've added this under Older Bugs. Thanks, Amit
Re: selecting from partitions and constraint exclusion
On Wed, Mar 20, 2019 at 12:37 AM Amit Langote wrote: > That's because get_relation_constraints() no longer (as of PG 11) includes > the partition constraint for SELECT queries. What commit made that change? This sounds to me like maybe it should be an open item. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: selecting from partitions and constraint exclusion
On 2019/03/22 17:17, Amit Langote wrote: > I'll add this to July fest to avoid forgetting about this. I'd forgotten to do this, but done today. :) Thanks, Amit
Re: selecting from partitions and constraint exclusion
Hi David,
Thanks for checking.
On 2019/03/20 19:41, David Rowley wrote:
> On Wed, 20 Mar 2019 at 17:37, Amit Langote
> 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
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 =
Re: selecting from partitions and constraint exclusion
On Wed, 20 Mar 2019 at 17:37, Amit Langote 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. 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) ? 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. 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)) For the tests, it seems excessive to create some new tables for this. Won't the tables in the previous test work just fine? [1] https://www.postgresql.org/docs/devel/runtime-config-query.html#GUC-CONSTRAINT-EXCLUSION -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
selecting from partitions and constraint exclusion
Hi,
While looking at a partition pruning bug [1], I noticed something that
started to feel like a regression:
Setup:
create table p (a int) partition by list (a);
create table p1 partition of p for values in (1);
In PG 10:
set constraint_exclusion to on;
explain select * from p1 where a = 2;
QUERY PLAN
──
Result (cost=0.00..0.00 rows=0 width=4)
One-Time Filter: false
(2 rows)
In PG 11 (and HEAD):
set constraint_exclusion to on;
explain select * from p1 where a = 2;
QUERY PLAN
Seq Scan on p1 (cost=0.00..41.88 rows=13 width=4)
Filter: (a = 2)
(2 rows)
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.
Thanks,
Amit
[1]
https://www.postgresql.org/message-id/00e601d4ca86$932b8bc0$b982a340$@lab.ntt.co.jp
From 97dc0a032426a798d4dbc957783168567ed285ed Mon Sep 17 00:00:00 2001
From: amit
Date: Wed, 20 Mar 2019 13:27:37 +0900
Subject: [PATCH v1] 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 | 15 ++---
src/test/regress/expected/partition_prune.out | 44 +++
src/test/regress/sql/partition_prune.sql | 20
3 files changed, 75 insertions(+), 4 deletions(-)
diff --git a/src/backend/optimizer/util/plancat.c
b/src/backend/optimizer/util/plancat.c
index 30f4dc151b..a8ba586565 100644
--- a/src/backend/optimizer/util/plancat.c
+++ b/src/backend/optimizer/util/plancat.c
@@ -1269,11 +1269,18 @@ 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.
+* 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.
*/
- if (enable_partition_pruning && root->parse->commandType != CMD_SELECT)
+ if ((root->parse->commandType == CMD_SELECT && !IS_OTHER_REL(rel)) ||
+ (root->parse->commandType != CMD_SELECT &&
enable_partition_pruning))
{
List *pcqual = RelationGetPartitionQual(relation);
diff --git a/src/test/regress/expected/partition_prune.out
b/src/test/regress/expected/partition_prune.out
index 30946f77b6..23f042d0ba 100644
--- a/src/test/regress/expected/partition_prune.out
+++ b/src/test/regress/expected/partition_prune.out
@@ -3638,3 +3638,47 @@ select * from listp where a = (select 2) and b <> 10;
(5 rows)
drop table listp;
+-- check that a directly accessed in a query is excluded with
+-- constraint_exclusion = on
+create table part_excl_test (a int) partition by list (a);
+create table part_excl_test1 partition of part_excl_test for values in (1);
+create table part_excl_test2 partition of part_excl_test for values in (2)
partition by list (a);
+create table part_excl_test22 partition of part_excl_test2 for values in (2);
+-- first off, 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 part_excl_test1 where a = 2;
+ QUERY PLAN
+-
+ Seq Scan on part_excl_test1
+ Filter: (a = 2)
+(2 rows)
+
+explain (costs off) select * from part_excl_test2 where a = 1;
+ QUERY PLAN
+
+ Append
+ -> Seq Scan on part_excl_test22
+ Filter:
