Problem with default partition pruning

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
89 messages Options
12345
Reply | Threaded
Open this post in threaded view
|

Re: Problem with default partition pruning

Thibaut Madelaine

Le 22/03/2019 à 07:38, Amit Langote a écrit :

> Hosoya-san,
>
> On 2019/03/22 15:02, Yuzuko Hosoya wrote:
>> I understood Amit's proposal.  But I think the issue Thibaut reported would
>> occur regardless of whether clauses have OR clauses or not as follows.
>> I tested a query which should output "One-Time Filter: false".
>>
>> # explain select * from test2_0_20 where id = 25;
>>                               QUERY PLAN                              
>> -----------------------------------------------------------------------
>>  Append  (cost=0.00..25.91 rows=6 width=36)
>>    ->  Seq Scan on test2_10_20_def  (cost=0.00..25.88 rows=6 width=36)
>>          Filter: (id = 25)
>>
> Good catch, thanks.
>
>> As Amit described in the previous email, id = 25 contradicts test2_0_20's
>> partition constraint, so I think this clause should be ignored and we can
>> also handle this case in the similar way as Amit proposal.
>>
>> I attached v1-delta-2.patch which fix the above issue.  
>>
>> What do you think about it?
> It looks fine to me.  You put the code block to check whether a give
> clause contradicts the partition constraint in its perfect place. :)
>
> Maybe we should have two patches as we seem to be improving two things:
>
> 1. Patch to fix problems with default partition pruning originally
> reported by Hosoya-san
>
> 2. Patch to determine if a given clause contradicts a sub-partitioned
> table's partition constraint, fixing problems unearthed by Thibaut's tests
>
> About the patch that Horiguchi-san proposed upthread, I think it has merit
> that it will make partprune.c code easier to reason about, but I think we
> should pursue it separately.
>
> Thanks,
> Amit

Hosoya-san, very good idea to run queries directly on tables partitions!

I tested your last patch and if I didn't mix up patches on the end of a
too long week, I get a problem when querying the sub-sub partition:

test=# explain select * from test2_0_10 where id = 25;
                         QUERY PLAN                        
------------------------------------------------------------
 Seq Scan on test2_0_10  (cost=0.00..25.88 rows=6 width=36)
   Filter: (id = 25)
(2 rows)


Cordialement,

Thibaut


Reply | Threaded
Open this post in threaded view
|

Re: Problem with default partition pruning

Amit Langote-2
Hi,

On 2019/03/23 2:36, Thibaut Madelaine wrote:
> I tested your last patch and if I didn't mix up patches on the end of a
> too long week, I get a problem when querying the sub-sub partition:
>
> test=# explain select * from test2_0_10 where id = 25;
>                          QUERY PLAN                        
> ------------------------------------------------------------
>  Seq Scan on test2_0_10  (cost=0.00..25.88 rows=6 width=36)
>    Filter: (id = 25)
> (2 rows)

The problem here is not really related to partition pruning, but another
problem I recently sent an email about:

https://www.postgresql.org/message-id/9813f079-f16b-61c8-9ab7-4363cab28d80%40lab.ntt.co.jp

The problem in this case is that *constraint exclusion* is not working,
because partition constraint is not loaded by the planner.  Note that
pruning is only used if a query specifies the parent table, not a partition.

Thanks,
Amit


Reply | Threaded
Open this post in threaded view
|

RE: Problem with default partition pruning

Yuzuko Hosoya
Hi,

>
> Hi,
>
> On 2019/03/23 2:36, Thibaut Madelaine wrote:
> > I tested your last patch and if I didn't mix up patches on the end of
> > a too long week, I get a problem when querying the sub-sub partition:
> >
> > test=# explain select * from test2_0_10 where id = 25;
> >                          QUERY PLAN
> > ------------------------------------------------------------
> >  Seq Scan on test2_0_10  (cost=0.00..25.88 rows=6 width=36)
> >    Filter: (id = 25)
> > (2 rows)
>
> The problem here is not really related to partition pruning, but another problem I recently sent an
> email about:
>
> https://www.postgresql.org/message-id/9813f079-f16b-61c8-9ab7-4363cab28d80%40lab.ntt.co.jp
>
> The problem in this case is that *constraint exclusion* is not working, because partition constraint
> is not loaded by the planner.  Note that pruning is only used if a query specifies the parent table,
> not a partition.

Thanks for the comments.

I saw that email.  Also, I checked that query Thibaut mentioned worked
correctly with Amit's patch discussed in that thread.


Best regards,
Yuzuko Hosoya



Reply | Threaded
Open this post in threaded view
|

RE: Problem with default partition pruning

Yuzuko Hosoya
In reply to this post by Amit Langote-2
Hi,

> Maybe we should have two patches as we seem to be improving two things:
>
> 1. Patch to fix problems with default partition pruning originally reported by Hosoya-san
>
> 2. Patch to determine if a given clause contradicts a sub-partitioned table's partition constraint,
> fixing problems unearthed by Thibaut's tests

I attached the latest patches according to Amit comment.
v3_default_partition_pruning.patch fixes default partition pruning problems
and ignore_contradictory_where_clauses_at_partprune_step.patch fixes
sub-partition problems Thibaut tested.

Best regards,
Yuzuko Hosoya

ignore_contradictory_where_clauses_at_partprune_step.patch (3K) Download Attachment
v3_default_partition_pruning.patch (14K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Problem with default partition pruning

Amit Langote-2
Hosoya-san,

On 2019/04/02 14:02, Yuzuko Hosoya wrote:

> Hi,
>
>> Maybe we should have two patches as we seem to be improving two things:
>>
>> 1. Patch to fix problems with default partition pruning originally reported by Hosoya-san
>>
>> 2. Patch to determine if a given clause contradicts a sub-partitioned table's partition constraint,
>> fixing problems unearthed by Thibaut's tests
>
> I attached the latest patches according to Amit comment.
> v3_default_partition_pruning.patch fixes default partition pruning problems
> and ignore_contradictory_where_clauses_at_partprune_step.patch fixes
> sub-partition problems Thibaut tested.

Thanks for dividing patches that way.

Would it be a good idea to add some new test cases to these patches, just
so it's easily apparent what we're changing?

So, we could add the test case presented by Thibaut at the following link
to the default_partition_pruning.patch:

https://www.postgresql.org/message-id/a4968068-6401-7a9c-8bd4-6a3bc9164a86%40dalibo.com

And, another reported at the following link to
ignore_contradictory_where_clauses_at_partprune_step.patch:

https://www.postgresql.org/message-id/bd03f475-30d4-c4d0-3d7f-d2fbde755971%40dalibo.com

Actually, it might be possible/better to construct the test queries in
partition_prune.sql using the existing tables in that script, that is,
without defining new tables just for adding the new test cases.  If not,
maybe it's OK to create the new tables too.

Thanks,
Amit



Reply | Threaded
Open this post in threaded view
|

RE: Problem with default partition pruning

Yuzuko Hosoya
Amit-san,

Thanks for the comments.

>
> Thanks for dividing patches that way.
>
> Would it be a good idea to add some new test cases to these patches, just so it's easily apparent what
> we're changing?
Yes, I agree with you.

>
> So, we could add the test case presented by Thibaut at the following link to the
> default_partition_pruning.patch:
>
> https://www.postgresql.org/message-id/a4968068-6401-7a9c-8bd4-6a3bc9164a86%40dalibo.com
>
> And, another reported at the following link to
> ignore_contradictory_where_clauses_at_partprune_step.patch:
>
> https://www.postgresql.org/message-id/bd03f475-30d4-c4d0-3d7f-d2fbde755971%40dalibo.com
>
> Actually, it might be possible/better to construct the test queries in partition_prune.sql using the
> existing tables in that script, that is, without defining new tables just for adding the new test cases.
> If not, maybe it's OK to create the new tables too.
>
I see.  I added some test cases to each patch according to tests
discussed in this thread.

However, I found another problem as follows. This query should
output "One-Time Filter: false" because rlp3's constraints
contradict WHERE clause.

-----
postgres=# \d+ rlp3
                                   Partitioned table "public.rlp3"
 Column |       Type        | Collation | Nullable | Default | Storage  | Stats target | Description
--------+-------------------+-----------+----------+---------+----------+--------------+-------------
 b      | character varying |           |          |         | extended |              |
 a      | integer           |           |          |         | plain    |              |
Partition of: rlp FOR VALUES FROM (15) TO (20)
Partition constraint: ((a IS NOT NULL) AND (a >= 15) AND (a < 20))
Partition key: LIST (b varchar_ops)
Partitions: rlp3abcd FOR VALUES IN ('ab', 'cd'),
            rlp3efgh FOR VALUES IN ('ef', 'gh'),
            rlp3nullxy FOR VALUES IN (NULL, 'xy'),
            rlp3_default DEFAULT

postgres=# explain select * from rlp3 where a = 2;
                             QUERY PLAN                            
--------------------------------------------------------------------
 Append  (cost=0.00..103.62 rows=24 width=36)
   ->  Seq Scan on rlp3abcd  (cost=0.00..25.88 rows=6 width=36)
         Filter: (a = 2)
   ->  Seq Scan on rlp3efgh  (cost=0.00..25.88 rows=6 width=36)
         Filter: (a = 2)
   ->  Seq Scan on rlp3nullxy  (cost=0.00..25.88 rows=6 width=36)
         Filter: (a = 2)
   ->  Seq Scan on rlp3_default  (cost=0.00..25.88 rows=6 width=36)
         Filter: (a = 2)
(9 rows)
-----

I think that the place of check contradiction process was wrong
At ignore_contradictory_where_clauses_at_partprune_step.patch.
So I fixed it.

Attached the latest patches. Please check it again.

Best regards,
Yuzuko Hosoya

v2_ignore_contradictory_where_clauses_at_partprune_step.patch (4K) Download Attachment
v4_default_partition_pruning.patch (15K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Problem with default partition pruning

Amit Langote-2
Hosoya-san,


On 2019/04/04 13:00, Yuzuko Hosoya wrote:
> I added some test cases to each patch according to tests
> discussed in this thread.

Thanks a lot.

> However, I found another problem as follows. This query should
> output "One-Time Filter: false" because rlp3's constraints
> contradict WHERE clause.
>
> -----
> postgres=# \d+ rlp3
>                                    Partitioned table "public.rlp3"
>  Column |       Type        | Collation | Nullable | Default | Storage  | Stats target | Description
> --------+-------------------+-----------+----------+---------+----------+--------------+-------------
>  b      | character varying |           |          |         | extended |              |
>  a      | integer           |           |          |         | plain    |              |
> Partition of: rlp FOR VALUES FROM (15) TO (20)
> Partition constraint: ((a IS NOT NULL) AND (a >= 15) AND (a < 20))
> Partition key: LIST (b varchar_ops)
> Partitions: rlp3abcd FOR VALUES IN ('ab', 'cd'),
>             rlp3efgh FOR VALUES IN ('ef', 'gh'),
>             rlp3nullxy FOR VALUES IN (NULL, 'xy'),
>             rlp3_default DEFAULT
>
> postgres=# explain select * from rlp3 where a = 2;
>                              QUERY PLAN                            
> --------------------------------------------------------------------
>  Append  (cost=0.00..103.62 rows=24 width=36)
>    ->  Seq Scan on rlp3abcd  (cost=0.00..25.88 rows=6 width=36)
>          Filter: (a = 2)
>    ->  Seq Scan on rlp3efgh  (cost=0.00..25.88 rows=6 width=36)
>          Filter: (a = 2)
>    ->  Seq Scan on rlp3nullxy  (cost=0.00..25.88 rows=6 width=36)
>          Filter: (a = 2)
>    ->  Seq Scan on rlp3_default  (cost=0.00..25.88 rows=6 width=36)
>          Filter: (a = 2)
> (9 rows)
> -----

This one too would be solved with the other patch I mentioned to fix
get_relation_info() to load the partition constraint so that constraint
exclusion can use it.  Partition in the earlier example given by Thibaut
is a leaf partition, whereas rlp3 above is a sub-partitioned partition,
but both are partitions nonetheless.

Fixing partprune.c like we're doing with the
v2_ignore_contradictory_where_clauses_at_partprune_step.patch only works
for the latter, because only partitioned tables visit partprune.c.

OTOH, the other patch only applies to situations where
constraint_exclusion = on.

> I think that the place of check contradiction process was wrong
> At ignore_contradictory_where_clauses_at_partprune_step.patch.
> So I fixed it.

Thanks.  Patch contains some whitespace noise:

$ git diff --check
src/backend/partitioning/partprune.c:790: trailing whitespace.
+         * given its partition constraint, we can ignore it,
src/backend/partitioning/partprune.c:791: trailing whitespace.
+         * that is not try to pass it to the pruning code.
src/backend/partitioning/partprune.c:792: trailing whitespace.
+         * We should do that especially to avoid pruning code
src/backend/partitioning/partprune.c:810: trailing whitespace.
+
src/test/regress/sql/partition_prune.sql:87: trailing whitespace.
+-- where clause contradicts sub-partition's constraint

Can you please fix it?


BTW, now I'm a bit puzzled between whether this case should be fixed by
hacking on partprune.c like this patch does or whether to work on getting
the other patch committed and expect users to set constraint_exclusion =
on for this to behave as expected.  The original intention of setting
partition_qual in set_relation_partition_info() was for partprune.c to use
it to remove useless arguments of OR clauses which otherwise would cause
the failure to correctly prune the default partitions of sub-partitioned
tables.  As shown by the examples in this thread, the original effort was
insufficient, which this patch aims to improve.  But, it also expands the
scope of partprune.c's usage of partition_qual, which is to effectively
perform full-blown constraint exclusion without being controllable by
constraint_exclusion GUC, which may be seen as being good or bad.  The
fact that it helps in getting partition pruning working correctly in more
obscure cases like those discussed in this thread means it's good maybe.

Thanks,
Amit



Reply | Threaded
Open this post in threaded view
|

RE: Problem with default partition pruning

Yuzuko Hosoya
Amit-san,

> -----Original Message-----
> From: Amit Langote [mailto:[hidden email]]
> Sent: Friday, April 05, 2019 6:47 PM
> To: Yuzuko Hosoya <[hidden email]>; 'Thibaut' <[hidden email]>; 'Imai,
> Yoshikazu' <[hidden email]>
> Cc: 'PostgreSQL Hackers' <[hidden email]>
> Subject: Re: Problem with default partition pruning
>
> Hosoya-san,
>
>
> On 2019/04/04 13:00, Yuzuko Hosoya wrote:
> > I added some test cases to each patch according to tests discussed in
> > this thread.
>
> Thanks a lot.
>
> > However, I found another problem as follows. This query should output
> > "One-Time Filter: false" because rlp3's constraints contradict WHERE
> > clause.
> >
> > -----
> > postgres=# \d+ rlp3
> >                                    Partitioned table "public.rlp3"
> >  Column |       Type        | Collation | Nullable | Default | Storage  | Stats target | Description
> >
> --------+-------------------+-----------+----------+---------+----------+--------------+---------
> ----
> >  b      | character varying |           |          |         | extended |              |
> >  a      | integer           |           |          |         | plain    |              |
> > Partition of: rlp FOR VALUES FROM (15) TO (20) Partition constraint:
> > ((a IS NOT NULL) AND (a >= 15) AND (a < 20)) Partition key: LIST (b
> > varchar_ops)
> > Partitions: rlp3abcd FOR VALUES IN ('ab', 'cd'),
> >             rlp3efgh FOR VALUES IN ('ef', 'gh'),
> >             rlp3nullxy FOR VALUES IN (NULL, 'xy'),
> >             rlp3_default DEFAULT
> >
> > postgres=# explain select * from rlp3 where a = 2;
> >                              QUERY PLAN
> > --------------------------------------------------------------------
> >  Append  (cost=0.00..103.62 rows=24 width=36)
> >    ->  Seq Scan on rlp3abcd  (cost=0.00..25.88 rows=6 width=36)
> >          Filter: (a = 2)
> >    ->  Seq Scan on rlp3efgh  (cost=0.00..25.88 rows=6 width=36)
> >          Filter: (a = 2)
> >    ->  Seq Scan on rlp3nullxy  (cost=0.00..25.88 rows=6 width=36)
> >          Filter: (a = 2)
> >    ->  Seq Scan on rlp3_default  (cost=0.00..25.88 rows=6 width=36)
> >          Filter: (a = 2)
> > (9 rows)
> > -----
>
> This one too would be solved with the other patch I mentioned to fix
> get_relation_info() to load the partition constraint so that constraint exclusion can use it.
> Partition in the earlier example given by Thibaut is a leaf partition, whereas rlp3 above is a
> sub-partitioned partition, but both are partitions nonetheless.
>
> Fixing partprune.c like we're doing with the
> v2_ignore_contradictory_where_clauses_at_partprune_step.patch only works for the latter, because only
> partitioned tables visit partprune.c.
>
> OTOH, the other patch only applies to situations where constraint_exclusion = on.
>
I see.  I think that following example discussed in this thread before would
also be solved with your patch, not v2_ignore_contradictory_where_clauses_at_partprune_step.patch.

postgres=# set constraint_exclusion to on;

postgres=# explain select * from test2_0_20 where id = 25;
                QUERY PLAN                
------------------------------------------
 Result  (cost=0.00..0.00 rows=0 width=0)
   One-Time Filter: false
(2 rows)


> > I think that the place of check contradiction process was wrong At
> > ignore_contradictory_where_clauses_at_partprune_step.patch.
> > So I fixed it.
>
> Thanks.  Patch contains some whitespace noise:
>
> $ git diff --check
> src/backend/partitioning/partprune.c:790: trailing whitespace.
> +         * given its partition constraint, we can ignore it,
> src/backend/partitioning/partprune.c:791: trailing whitespace.
> +         * that is not try to pass it to the pruning code.
> src/backend/partitioning/partprune.c:792: trailing whitespace.
> +         * We should do that especially to avoid pruning code
> src/backend/partitioning/partprune.c:810: trailing whitespace.
> +
> src/test/regress/sql/partition_prune.sql:87: trailing whitespace.
> +-- where clause contradicts sub-partition's constraint
>
> Can you please fix it?
>
Thanks for checking.
I'm attaching the latest patch.

>
> BTW, now I'm a bit puzzled between whether this case should be fixed by hacking on partprune.c like
> this patch does or whether to work on getting the other patch committed and expect users to set
> constraint_exclusion = on for this to behave as expected.  The original intention of setting
> partition_qual in set_relation_partition_info() was for partprune.c to use it to remove useless
> arguments of OR clauses which otherwise would cause the failure to correctly prune the default partitions
> of sub-partitioned tables.  As shown by the examples in this thread, the original effort was
> insufficient, which this patch aims to improve.  But, it also expands the scope of partprune.c's usage
> of partition_qual, which is to effectively perform full-blown constraint exclusion without being
> controllable by constraint_exclusion GUC, which may be seen as being good or bad.  The fact that it
> helps in getting partition pruning working correctly in more obscure cases like those discussed in
> this thread means it's good maybe.
>
Umm, even though this modification might be overhead, I think this problem should be solved
without setting constraint_exclusion GUC. But I'm not sure.

Best regards,
Yuzuko Hosoya

v3_ignore_contradictory_where_clauses_at_partprune_step.patch (4K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Problem with default partition pruning

Kyotaro HORIGUCHI-2
At Mon, 8 Apr 2019 16:57:35 +0900, "Yuzuko Hosoya" <[hidden email]> wrote in <00c101d4ede0$babd4390$3037cab0$@lab.ntt.co.jp>

> > BTW, now I'm a bit puzzled between whether this case should be fixed by hacking on partprune.c like
> > this patch does or whether to work on getting the other patch committed and expect users to set
> > constraint_exclusion = on for this to behave as expected.  The original intention of setting
> > partition_qual in set_relation_partition_info() was for partprune.c to use it to remove useless
> > arguments of OR clauses which otherwise would cause the failure to correctly prune the default partitions
> > of sub-partitioned tables.  As shown by the examples in this thread, the original effort was
> > insufficient, which this patch aims to improve.  But, it also expands the scope of partprune.c's usage
> > of partition_qual, which is to effectively perform full-blown constraint exclusion without being
> > controllable by constraint_exclusion GUC, which may be seen as being good or bad.  The fact that it
> > helps in getting partition pruning working correctly in more obscure cases like those discussed in
> > this thread means it's good maybe.
> >
> Umm, even though this modification might be overhead, I think this problem should be solved
> without setting constraint_exclusion GUC. But I'm not sure.

Partition pruning and constraint exclusion are orthogonal
functions. Note that the default value of the variable is
CONSTRAINT_EXCLUSION_PARTITION and the behavior is not a perfect
bug.  So I think we can reasonably ignore constraints when
constraint_exclusion is intentionally turned off.

As the result I propose to move the "if(partconstr)" block in the
latest patches after the constant-false block, changing the
condition as "if (partconstr && constraint_exclusion !=
CONSTRAINT_EXCLUSION_OFF)".

This make partprune reacts to constraint_exclusion the consistent
way with the old-fashioned partitioning.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center



Reply | Threaded
Open this post in threaded view
|

Re: Problem with default partition pruning

Kyotaro HORIGUCHI-2
At Mon, 08 Apr 2019 20:42:51 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <[hidden email]> wrote in <[hidden email]>

> At Mon, 8 Apr 2019 16:57:35 +0900, "Yuzuko Hosoya" <[hidden email]> wrote in <00c101d4ede0$babd4390$3037cab0$@lab.ntt.co.jp>
> > > BTW, now I'm a bit puzzled between whether this case should be fixed by hacking on partprune.c like
> > > this patch does or whether to work on getting the other patch committed and expect users to set
> > > constraint_exclusion = on for this to behave as expected.  The original intention of setting
> > > partition_qual in set_relation_partition_info() was for partprune.c to use it to remove useless
> > > arguments of OR clauses which otherwise would cause the failure to correctly prune the default partitions
> > > of sub-partitioned tables.  As shown by the examples in this thread, the original effort was
> > > insufficient, which this patch aims to improve.  But, it also expands the scope of partprune.c's usage
> > > of partition_qual, which is to effectively perform full-blown constraint exclusion without being
> > > controllable by constraint_exclusion GUC, which may be seen as being good or bad.  The fact that it
> > > helps in getting partition pruning working correctly in more obscure cases like those discussed in
> > > this thread means it's good maybe.
> > >
> > Umm, even though this modification might be overhead, I think this problem should be solved
> > without setting constraint_exclusion GUC. But I'm not sure.
>
> Partition pruning and constraint exclusion are orthogonal
> functions. Note that the default value of the variable is
> CONSTRAINT_EXCLUSION_PARTITION and the behavior is not a perfect
> bug.  So I think we can reasonably ignore constraints when
> constraint_exclusion is intentionally turned off.

> As the result I propose to move the "if(partconstr)" block in the
> latest patches after the constant-false block, changing the
> condition as "if (partconstr && constraint_exclusion !=
> CONSTRAINT_EXCLUSION_OFF)".

Mmm. Something is wrong. I should have been sleeping at the
time. In my opinion, what we should there is:

- Try partition pruning first.

- If the partition was not pruned, and constraint is set, check
  for constant false.

- if constraint_exclusion is turned on and constraint is set,
  examine the constraint.

Sorry for the stupidity.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center



Reply | Threaded
Open this post in threaded view
|

Re: Problem with default partition pruning

Kyotaro HORIGUCHI-2
In reply to this post by Yuzuko Hosoya
At Mon, 8 Apr 2019 16:57:35 +0900, "Yuzuko Hosoya" <[hidden email]> wrote in <00c101d4ede0$babd4390$3037cab0$@lab.ntt.co.jp>

> > BTW, now I'm a bit puzzled between whether this case should be fixed by hacking on partprune.c like
> > this patch does or whether to work on getting the other patch committed and expect users to set
> > constraint_exclusion = on for this to behave as expected.  The original intention of setting
> > partition_qual in set_relation_partition_info() was for partprune.c to use it to remove useless
> > arguments of OR clauses which otherwise would cause the failure to correctly prune the default partitions
> > of sub-partitioned tables.  As shown by the examples in this thread, the original effort was
> > insufficient, which this patch aims to improve.  But, it also expands the scope of partprune.c's usage
> > of partition_qual, which is to effectively perform full-blown constraint exclusion without being
> > controllable by constraint_exclusion GUC, which may be seen as being good or bad.  The fact that it
> > helps in getting partition pruning working correctly in more obscure cases like those discussed in
> > this thread means it's good maybe.
> >
> Umm, even though this modification might be overhead, I think this problem should be solved
> without setting constraint_exclusion GUC. But I'm not sure.

As the second thought. Partition constraint is not constraint
expression so that's fair to apply partqual ignoring
constraint_exclusion. The variable is set false to skip useless
expression evaluation on all partitions, but partqual should be
evaluated just once.  Sorry for my confusion.

So still it is wrong that the new code is added in
gen_partprune_steps_internal. If partqual results true and the
clause is long, the partqual is evaluated uselessly at every
recursion.

Maybe we should do that when we find that the current clause
doesn't match part attributes. Specifically just after the for
loop "for (i = 0 ; i < part_scheme->partnattrs; i++)".

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center



Reply | Threaded
Open this post in threaded view
|

Re: Problem with default partition pruning

Kyotaro HORIGUCHI-2
Sigh..

At Tue, 09 Apr 2019 10:28:48 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <[hidden email]> wrote in <[hidden email]>
> As the second thought. Partition constraint is not constraint
> expression so that's fair to apply partqual ignoring
> constraint_exclusion. The variable is set false to skip useless
> expression evaluation on all partitions, but partqual should be
> evaluated just once.  Sorry for my confusion.
>
> So still it is wrong that the new code is added in
> gen_partprune_steps_internal.

So still it is wrong that the new code is added at the beginning
of the loop on clauses in gen_partprune_steps_internal.

>                               If partqual results true and the
> clause is long, the partqual is evaluated uselessly at every
> recursion.
>
> Maybe we should do that when we find that the current clause
> doesn't match part attributes. Specifically just after the for
> loop "for (i = 0 ; i < part_scheme->partnattrs; i++)".

--
Kyotaro Horiguchi
NTT Open Source Software Center



Reply | Threaded
Open this post in threaded view
|

RE: Problem with default partition pruning

Yuzuko Hosoya
Horiguchi-san,

Thanks for your comments.

> -----Original Message-----
> From: Kyotaro HORIGUCHI [mailto:[hidden email]]
> Sent: Tuesday, April 09, 2019 10:33 AM
> To: [hidden email]
> Cc: [hidden email]; [hidden email]; [hidden email];
> [hidden email]
> Subject: Re: Problem with default partition pruning
>
> Sigh..
>
> At Tue, 09 Apr 2019 10:28:48 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI
> <[hidden email]> wrote in
> <[hidden email]>
> > As the second thought. Partition constraint is not constraint
> > expression so that's fair to apply partqual ignoring
> > constraint_exclusion. The variable is set false to skip useless
> > expression evaluation on all partitions, but partqual should be
> > evaluated just once.  Sorry for my confusion.
> >
> > So still it is wrong that the new code is added in
> > gen_partprune_steps_internal.
>
> So still it is wrong that the new code is added at the beginning of the loop on clauses in
> gen_partprune_steps_internal.
>
> >                               If partqual results true and the clause
> > is long, the partqual is evaluated uselessly at every recursion.
> >
> > Maybe we should do that when we find that the current clause doesn't
> > match part attributes. Specifically just after the for loop "for (i =
> > 0 ; i < part_scheme->partnattrs; i++)".
>
I think we should check whether WHERE clause contradicts partition
constraint even when the clause matches part attributes.  So I moved
"if (partqual)" block to the beginning of the loop you mentioned.

I'm attaching the latest version.  Could you please check it again?

Best regards,
Yuzuko Hosoya

v4_ignore_contradictory_where_clauses_at_partprune_step.patch (4K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Problem with default partition pruning

Kyotaro HORIGUCHI-2
Hi.

At Tue, 9 Apr 2019 16:41:47 +0900, "Yuzuko Hosoya" <[hidden email]> wrote in <00cf01d4eea7$afa43370$0eec9a50$@lab.ntt.co.jp>

> > So still it is wrong that the new code is added at the beginning of the loop on clauses in
> > gen_partprune_steps_internal.
> >
> > >                               If partqual results true and the clause
> > > is long, the partqual is evaluated uselessly at every recursion.
> > >
> > > Maybe we should do that when we find that the current clause doesn't
> > > match part attributes. Specifically just after the for loop "for (i =
> > > 0 ; i < part_scheme->partnattrs; i++)".
> >
> I think we should check whether WHERE clause contradicts partition
> constraint even when the clause matches part attributes.  So I moved

Why?  If clauses contains a clause on a partition key, the clause
is involved in determination of whether a partition survives or
not in ordinary way. Could you show how or on what configuration
(tables and query) it happens that such a matching clause needs
to be checked against partqual?

The "if (partconstr)" block uselessly runs for every clause in
the clause tree other than BoolExpr. If we want do that, isn't
just doing predicate_refuted_by(partconstr, clauses, false)
sufficient before looping over clauses?


> "if (partqual)" block to the beginning of the loop you mentioned.
>
> I'm attaching the latest version.  Could you please check it again?

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center



Reply | Threaded
Open this post in threaded view
|

Re: Problem with default partition pruning

Kyotaro HORIGUCHI-2
At Tue, 09 Apr 2019 17:37:25 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <[hidden email]> wrote in <[hidden email]>
> > I'm attaching the latest version.  Could you please check it again?

By the way, I noticed that partition constraint in a multi-level
partition contains duplicate clauses.

create table p (a int) partition by range (a);
create table c1 partition of p for values from (0) to (10) partition by range (a);
create table c11 partition of c1 for values from (0) to (2) partition by range (a);
create table c12 partition of c1 for values from (2) to (4) partition by range (a);

=# \d+ c12
|                               Partitioned table "public.c12"
|  Column |  Type   | Collation | Nullable | Default | Storage | Stats target | De
| scription
| --------+---------+-----------+----------+---------+---------+--------------+---
| ----------
|  a      | integer |           |          |         | plain   |              |
| Partition of: c1 FOR VALUES FROM (2) TO (4)
| Partition constraint: ((a IS NOT NULL) AND (a >= 0) AND (a < 10) AND (a IS NOT N
| ULL) AND (a >= 2) AND (a < 4))
| Partition key: RANGE (a)
| Number of partitions: 0


The partition constraint is equivalent to "(a IS NOT NULL) AND (a
>= 2) AND (a < 4)". Is it intentional (for, for example,
performance reasons)? Or is it reasonable to deduplicate the
quals?

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center



Reply | Threaded
Open this post in threaded view
|

Re: Problem with default partition pruning

Amit Langote-2
Horiguchi-san,

On 2019/04/09 17:51, Kyotaro HORIGUCHI wrote:

> At Tue, 09 Apr 2019 17:37:25 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <[hidden email]> wrote in <[hidden email]>
>>> I'm attaching the latest version.  Could you please check it again?
>
> By the way, I noticed that partition constraint in a multi-level
> partition contains duplicate clauses.
>
> create table p (a int) partition by range (a);
> create table c1 partition of p for values from (0) to (10) partition by range (a);
> create table c11 partition of c1 for values from (0) to (2) partition by range (a);
> create table c12 partition of c1 for values from (2) to (4) partition by range (a);
>
> =# \d+ c12
> |                               Partitioned table "public.c12"
> |  Column |  Type   | Collation | Nullable | Default | Storage | Stats target | De
> | scription
> | --------+---------+-----------+----------+---------+---------+--------------+---
> | ----------
> |  a      | integer |           |          |         | plain   |              |
> | Partition of: c1 FOR VALUES FROM (2) TO (4)
> | Partition constraint: ((a IS NOT NULL) AND (a >= 0) AND (a < 10) AND (a IS NOT N
> | ULL) AND (a >= 2) AND (a < 4))
> | Partition key: RANGE (a)
> | Number of partitions: 0
>
>
> The partition constraint is equivalent to "(a IS NOT NULL) AND (a
>> = 2) AND (a < 4)". Is it intentional (for, for example,
> performance reasons)? Or is it reasonable to deduplicate the
> quals?

Yeah, we don't try to simplify that due to lack of infrastructure, maybe.
If said infrastructure was present, maybe CHECK constraints would already
be using that, which doesn't seem to be the case.

create table foo (a int check ((a IS NOT NULL) AND (a >= 0) AND (a < 10)
AND (a IS NOT NULL) AND (a >= 2) AND (a < 4)));

\d foo
                Table "public.foo"
 Column │  Type   │ Collation │ Nullable │ Default
────────┼─────────┼───────────┼──────────┼─────────
 a      │ integer │           │          │
Check constraints:
    "foo_a_check" CHECK (a IS NOT NULL AND a >= 0 AND a < 10 AND a IS NOT
NULL AND a >= 2 AND a < 4)

Now it's true that users wouldn't manually write expressions like that,
but the expressions might be an automatically generated, which is also the
case with partition constraint of a deeply nested partition.

Thanks,
Amit



Reply | Threaded
Open this post in threaded view
|

Re: Problem with default partition pruning

Kyotaro HORIGUCHI-2
Hi, Amit. Thank you for the explanation.

At Tue, 9 Apr 2019 18:09:20 +0900, Amit Langote <[hidden email]> wrote in <[hidden email]>
> > The partition constraint is equivalent to "(a IS NOT NULL) AND (a
> >> = 2) AND (a < 4)". Is it intentional (for, for example,
> > performance reasons)? Or is it reasonable to deduplicate the
> > quals?
>
> Yeah, we don't try to simplify that due to lack of infrastructure, maybe.
> If said infrastructure was present, maybe CHECK constraints would already
> be using that, which doesn't seem to be the case.

Doesn't predicate_implied_by do that?
With the attached small patch, the partqual in my example becomes.

Partition constraint: ((a IS NOT NULL) AND (a >= 2) AND (a < 4))

And for in a more complex case:

create table p2 (a int, b int) partition by range (a, b);
create table c21 partition of p2 for values from (0, 0) to (1, 50) partition by range (a, b);
create table c22 partition of p2 for values from (1, 50) to (2, 100) partition by range (a, b);
create table c211 partition of c21 for values from (0, 0) to (0, 1000);
create table c212 partition of c21 for values from (0, 1000) to (0, 2000);

\d+ c212
..
Partition constraint: ((a IS NOT NULL) AND (b IS NOT NULL) AND ((a > 0) OR ((a =
 0) AND (b >= 0))) AND ((a < 1) OR ((a = 1) AND (b < 50))) AND (a IS NOT NULL) A
ND (b IS NOT NULL) AND (a = 0) AND (b >= 1000) AND (b < 2000))

is reduced to:

Partition constraint: ((a IS NOT NULL) AND (b IS NOT NULL) AND (a = 0) AND (b >=
 1000) AND (b < 2000))

Of course this cannot be reducible:

create table p3 (a int, b int) partition by range (a);
create table c31 partition of p3 for values from (0) to (1) partition by range(b);
create table c311 partition of c31 for values from (0) to (1);
\d+ c311

Partition constraint: ((a IS NOT NULL) AND (a >= 0) AND (a < 1) AND (b IS NOT NU
LL) AND (b >= 0) AND (b < 1))

I think this is useful even counting possible degradation, and I
believe generate_partition_qual is not called so often.


> create table foo (a int check ((a IS NOT NULL) AND (a >= 0) AND (a < 10)
> AND (a IS NOT NULL) AND (a >= 2) AND (a < 4)));
>
> \d foo
>                 Table "public.foo"
>  Column │  Type   │ Collation │ Nullable │ Default
> ────────┼─────────┼───────────┼──────────┼─────────
>  a      │ integer │           │          │
> Check constraints:
>     "foo_a_check" CHECK (a IS NOT NULL AND a >= 0 AND a < 10 AND a IS NOT
> NULL AND a >= 2 AND a < 4)
>
> Now it's true that users wouldn't manually write expressions like that,
> but the expressions might be an automatically generated, which is also the
> case with partition constraint of a deeply nested partition.
Differently from manually written constraint, partition
constraint is highly reducible.

Thoughts?

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

diff --git a/src/backend/utils/cache/partcache.c b/src/backend/utils/cache/partcache.c
index 8f43d682cf..c2f6d472c2 100644
--- a/src/backend/utils/cache/partcache.c
+++ b/src/backend/utils/cache/partcache.c
@@ -357,7 +357,14 @@ generate_partition_qual(Relation rel)
 
  /* Add the parent's quals to the list (if any) */
  if (parent->rd_rel->relispartition)
- result = list_concat(generate_partition_qual(parent), my_qual);
+ {
+ List *pqual = generate_partition_qual(parent);
+
+ if (predicate_implied_by(pqual, my_qual, false))
+ result = my_qual;
+ else
+ result = list_concat(pqual, my_qual);
+ }
  else
  result = my_qual;
 
Reply | Threaded
Open this post in threaded view
|

Re: Problem with default partition pruning

Amit Langote-2
Horiguchi-san,

On 2019/04/09 18:49, Kyotaro HORIGUCHI wrote:

> Hi, Amit. Thank you for the explanation.
>
> At Tue, 9 Apr 2019 18:09:20 +0900, Amit Langote <[hidden email]> wrote in <[hidden email]>
>>> The partition constraint is equivalent to "(a IS NOT NULL) AND (a
>>>> = 2) AND (a < 4)". Is it intentional (for, for example,
>>> performance reasons)? Or is it reasonable to deduplicate the
>>> quals?
>>
>> Yeah, we don't try to simplify that due to lack of infrastructure, maybe.
>> If said infrastructure was present, maybe CHECK constraints would already
>> be using that, which doesn't seem to be the case.
>
> Doesn't predicate_implied_by do that?
>
> With the attached small patch, the partqual in my example becomes.

Ah, I was wrong in saying we lack the infrastructure.

> Partition constraint: ((a IS NOT NULL) AND (a >= 2) AND (a < 4))
>
> And for in a more complex case:
>
> create table p2 (a int, b int) partition by range (a, b);
> create table c21 partition of p2 for values from (0, 0) to (1, 50) partition by range (a, b);
> create table c22 partition of p2 for values from (1, 50) to (2, 100) partition by range (a, b);
> create table c211 partition of c21 for values from (0, 0) to (0, 1000);
> create table c212 partition of c21 for values from (0, 1000) to (0, 2000);
>
> \d+ c212
> ..
> Partition constraint: ((a IS NOT NULL) AND (b IS NOT NULL) AND ((a > 0) OR ((a =
>  0) AND (b >= 0))) AND ((a < 1) OR ((a = 1) AND (b < 50))) AND (a IS NOT NULL) A
> ND (b IS NOT NULL) AND (a = 0) AND (b >= 1000) AND (b < 2000))
>
> is reduced to:
>
> Partition constraint: ((a IS NOT NULL) AND (b IS NOT NULL) AND (a = 0) AND (b >=
>  1000) AND (b < 2000))
>
> Of course this cannot be reducible:
>
> create table p3 (a int, b int) partition by range (a);
> create table c31 partition of p3 for values from (0) to (1) partition by range(b);
> create table c311 partition of c31 for values from (0) to (1);
> \d+ c311
>
> Partition constraint: ((a IS NOT NULL) AND (a >= 0) AND (a < 1) AND (b IS NOT NU
> LL) AND (b >= 0) AND (b < 1))
>
> I think this is useful even counting possible degradation, and I
> believe generate_partition_qual is not called so often.

I think more commonly used forms of sub-partitioning will use different
columns at different levels as in the 2nd example.  So, although we don't
call generate_partition_qual() as much as we used to before, even at the
times we do, we'd encounter this type of sub-partitioning more often and
the proposed optimization step will end up being futile in more cases than
the cases in which it would help.  Maybe, that was the reason not to try
too hard in the first place, not the lack of infrastructure as I was saying.

Thanks,
Amit



Reply | Threaded
Open this post in threaded view
|

Re: Problem with default partition pruning

Amit Langote-2
In reply to this post by Kyotaro HORIGUCHI-2
On 2019/04/09 17:37, Kyotaro HORIGUCHI wrote:

> At Tue, 9 Apr 2019 16:41:47 +0900, "Yuzuko Hosoya" <[hidden email]> wrote
>>> So still it is wrong that the new code is added at the beginning of the loop on clauses in
>>> gen_partprune_steps_internal.
>>>
>>>>                               If partqual results true and the clause
>>>> is long, the partqual is evaluated uselessly at every recursion.
>>>>
>>>> Maybe we should do that when we find that the current clause doesn't
>>>> match part attributes. Specifically just after the for loop "for (i =
>>>> 0 ; i < part_scheme->partnattrs; i++)".
>>>
>> I think we should check whether WHERE clause contradicts partition
>> constraint even when the clause matches part attributes.  So I moved
>
> Why?  If clauses contains a clause on a partition key, the clause
> is involved in determination of whether a partition survives or
> not in ordinary way. Could you show how or on what configuration
> (tables and query) it happens that such a matching clause needs
> to be checked against partqual?
>
> The "if (partconstr)" block uselessly runs for every clause in
> the clause tree other than BoolExpr. If we want do that, isn't
> just doing predicate_refuted_by(partconstr, clauses, false)
> sufficient before looping over clauses?

Yeah, I think we should move the "if (partconstr)" block to the "if
(is_orclause(clause))" block as I originally proposed in:

https://www.postgresql.org/message-id/9bb31dfe-b0d0-53f3-3ea6-e64b811424cf%40lab.ntt.co.jp

It's kind of a hack to get over the limitation that
get_matching_partitions() can't prune default partitions for certain OR
clauses and I think we shouldn't let that hack grow into what seems like
almost duplicating relation_excluded_by_constraints() but without the
constraint_exclusion GUC guard.

Thanks,
Amit



Reply | Threaded
Open this post in threaded view
|

Re: Problem with default partition pruning

Yuzuko Hosoya
In reply to this post by Yuzuko Hosoya
Horiguchi-san,

> -----Original Message-----
> From: Kyotaro HORIGUCHI [mailto:[hidden email]]
> Sent: Tuesday, April 09, 2019 5:37 PM
> To: [hidden email]
> Cc: [hidden email]; [hidden email];
> [hidden email]; [hidden email]
> Subject: Re: Problem with default partition pruning
>
> Hi.
>
> At Tue, 9 Apr 2019 16:41:47 +0900, "Yuzuko Hosoya"
> <[hidden email]> wrote in
> <00cf01d4eea7$afa43370$0eec9a50$@lab.ntt.co.jp>
> > > So still it is wrong that the new code is added at the beginning
> > > of the loop on clauses in gen_partprune_steps_internal.
> > >
> > > >                               If partqual results true and the
> > > > clause is long, the partqual is evaluated uselessly at every recursion.
> > > >
> > > > Maybe we should do that when we find that the current clause
> > > > doesn't match part attributes. Specifically just after the for
> > > > loop "for (i =
> > > > 0 ; i < part_scheme->partnattrs; i++)".
> > >
> > I think we should check whether WHERE clause contradicts partition
> > constraint even when the clause matches part attributes.  So I moved
>
> Why?  If clauses contains a clause on a partition key, the clause is
> involved in determination of whether a partition survives or not in
> ordinary way. Could you show how or on what configuration (tables and
> query) it happens that such a matching clause needs to be checked against partqual?
>
We found that partition pruning didn't work as expect when we scanned a sub-partition using WHERE
clause which contradicts the sub-partition's constraint by Thibaut tests.
The example discussed in this thread as follows.

postgres=# \d+ test2
                             Partitioned table "public.test2"
 Column |  Type   | Collation | Nullable | Default | Storage  | Stats target | Description
--------+---------+-----------+----------+---------+----------+--------------+-------------
 id     | integer |           |          |         | plain    |              |
 val    | text    |           |          |         | extended |              |
Partition key: RANGE (id)
Partitions: test2_0_20 FOR VALUES FROM (0) TO (20), PARTITIONED,
            test2_20_plus_def DEFAULT

postgres=# \d+ test2_0_20
                           Partitioned table "public.test2_0_20"
 Column |  Type   | Collation | Nullable | Default | Storage  | Stats target | Description
--------+---------+-----------+----------+---------+----------+--------------+-------------
 id     | integer |           |          |         | plain    |              |
 val    | text    |           |          |         | extended |              |
Partition of: test2 FOR VALUES FROM (0) TO (20) Partition constraint: ((id IS NOT NULL) AND (id >=
0) AND (id < 20)) Partition key: RANGE (id)
Partitions: test2_0_10 FOR VALUES FROM (0) TO (10),
            test2_10_20_def DEFAULT

postgres=# explain (costs off) select * from test2 where id=5 or id=20;
               QUERY PLAN                
-----------------------------------------
 Append
   ->  Seq Scan on test2_0_10
         Filter: ((id = 5) OR (id = 20))
   ->  Seq Scan on test2_10_20_def
         Filter: ((id = 5) OR (id = 20))
   ->  Seq Scan on test2_20_plus_def
         Filter: ((id = 5) OR (id = 20))
(7 rows)

postgres=# explain (costs off) select * from test2_0_20 where id=25;
         QUERY PLAN          
-----------------------------
 Seq Scan on test2_10_20_def
   Filter: (id = 25)
(2 rows)

So I think we have to check if WHERE clause contradicts sub-partition's constraint regardless of
whether the clause matches part attributes or not.

> The "if (partconstr)" block uselessly runs for every clause in the clause tree other than
BoolExpr.
> If we want do that, isn't just doing predicate_refuted_by(partconstr,
> clauses, false) sufficient before looping over clauses?
Yes, I tried doing that in the original patch.

>
>
> > "if (partqual)" block to the beginning of the loop you mentioned.
> >
> > I'm attaching the latest version.  Could you please check it again?
>
> regards.
>
> --
> Kyotaro Horiguchi
> NTT Open Source Software Center

Best regards,
Yuzuko Hosoya




12345