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
|

Problem with default partition pruning

Yuzuko Hosoya
Hi,

I found the bug of default partition pruning when executing a range query.

-----
postgres=# create table test1(id int, val text) partition by range (id);
postgres=# create table test1_1 partition of test1 for values from (0) to (100);
postgres=# create table test1_2 partition of test1 for values from (150) to (200);
postgres=# create table test1_def partition of test1 default;

postgres=# explain select * from test1 where id > 0 and id < 30;
                           QUERY PLAN                          
----------------------------------------------------------------
 Append  (cost=0.00..11.83 rows=59 width=11)
   ->  Seq Scan on test1_1  (cost=0.00..5.00 rows=58 width=11)
         Filter: ((id > 0) AND (id < 30))
   ->  Seq Scan on test1_def  (cost=0.00..6.53 rows=1 width=12)
         Filter: ((id > 0) AND (id < 30))
(5 rows)

There is no need to scan the default partition, but it's scanned.
-----

In the current implement, whether the default partition is scanned
or not is determined according to each condition of given WHERE
clause at get_matching_range_bounds().  In this example, scan_default
is set true according to id > 0 because id >= 200 matches the default
partition.  Similarly, according to id < 30, scan_default is set true.
Then, these results are combined according to AND/OR at perform_pruning_combine_step().
In this case, final result's scan_default is set true.

The modifications I made are as follows:
- get_matching_range_bounds() determines only offsets of range bounds
  according to each condition
- These results are combined at perform_pruning_combine_step()
- Whether the default partition is scanned or not is determined at
  get_matching_partitions()

Attached the patch.  Any feedback is greatly appreciated.

Best regards,
---
Yuzuko Hosoya
NTT Open Source Software Center

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/02/22 17:14, Yuzuko Hosoya wrote:

> Hi,
>
> I found the bug of default partition pruning when executing a range query.
>
> -----
> postgres=# create table test1(id int, val text) partition by range (id);
> postgres=# create table test1_1 partition of test1 for values from (0) to (100);
> postgres=# create table test1_2 partition of test1 for values from (150) to (200);
> postgres=# create table test1_def partition of test1 default;
>
> postgres=# explain select * from test1 where id > 0 and id < 30;
>                            QUERY PLAN                          
> ----------------------------------------------------------------
>  Append  (cost=0.00..11.83 rows=59 width=11)
>    ->  Seq Scan on test1_1  (cost=0.00..5.00 rows=58 width=11)
>          Filter: ((id > 0) AND (id < 30))
>    ->  Seq Scan on test1_def  (cost=0.00..6.53 rows=1 width=12)
>          Filter: ((id > 0) AND (id < 30))
> (5 rows)
>
> There is no need to scan the default partition, but it's scanned.
> -----
>
> In the current implement, whether the default partition is scanned
> or not is determined according to each condition of given WHERE
> clause at get_matching_range_bounds().  In this example, scan_default
> is set true according to id > 0 because id >= 200 matches the default
> partition.  Similarly, according to id < 30, scan_default is set true.
> Then, these results are combined according to AND/OR at perform_pruning_combine_step().
> In this case, final result's scan_default is set true.
>
> The modifications I made are as follows:
> - get_matching_range_bounds() determines only offsets of range bounds
>   according to each condition
> - These results are combined at perform_pruning_combine_step()
> - Whether the default partition is scanned or not is determined at
>   get_matching_partitions()
>
> Attached the patch.  Any feedback is greatly appreciated.

Thank you for reporting.  Can you please add this to March CF in Bugs
category so as not to lose track of this?

I will try to send review comments soon.

Regards,
Amit


Reply | Threaded
Open this post in threaded view
|

RE: Problem with default partition pruning

Yuzuko Hosoya
Amit-san,

> From: Amit Langote [mailto:[hidden email]]
> Sent: Wednesday, February 27, 2019 11:22 AM
>
> Hosoya-san,
>
> On 2019/02/22 17:14, Yuzuko Hosoya wrote:
> > Hi,
> >
> > I found the bug of default partition pruning when executing a range query.
> >
> > -----
> > postgres=# create table test1(id int, val text) partition by range
> > (id); postgres=# create table test1_1 partition of test1 for values
> > from (0) to (100); postgres=# create table test1_2 partition of test1
> > for values from (150) to (200); postgres=# create table test1_def
> > partition of test1 default;
> >
> > postgres=# explain select * from test1 where id > 0 and id < 30;
> >                            QUERY PLAN
> > ----------------------------------------------------------------
> >  Append  (cost=0.00..11.83 rows=59 width=11)
> >    ->  Seq Scan on test1_1  (cost=0.00..5.00 rows=58 width=11)
> >          Filter: ((id > 0) AND (id < 30))
> >    ->  Seq Scan on test1_def  (cost=0.00..6.53 rows=1 width=12)
> >          Filter: ((id > 0) AND (id < 30))
> > (5 rows)
> >
> > There is no need to scan the default partition, but it's scanned.
> > -----
> >
> > In the current implement, whether the default partition is scanned or
> > not is determined according to each condition of given WHERE clause at
> > get_matching_range_bounds().  In this example, scan_default is set
> > true according to id > 0 because id >= 200 matches the default
> > partition.  Similarly, according to id < 30, scan_default is set true.
> > Then, these results are combined according to AND/OR at perform_pruning_combine_step().
> > In this case, final result's scan_default is set true.
> >
> > The modifications I made are as follows:
> > - get_matching_range_bounds() determines only offsets of range bounds
> >   according to each condition
> > - These results are combined at perform_pruning_combine_step()
> > - Whether the default partition is scanned or not is determined at
> >   get_matching_partitions()
> >
> > Attached the patch.  Any feedback is greatly appreciated.
>
> Thank you for reporting.  Can you please add this to March CF in Bugs category so as not to lose
track
> of this?
>
> I will try to send review comments soon.
>
Thank you for your reply.  I added this to March CF.

Regards,
Yuzuko Hosoya
NTT Open Source Software Center



Reply | Threaded
Open this post in threaded view
|

RE: Problem with default partition pruning

Imai, Yoshikazu
Hosoya-san

On Wed, Feb 27, 2019 at 6:51 AM, Yuzuko Hosoya wrote:

> > From: Amit Langote [mailto:[hidden email]]
> > Sent: Wednesday, February 27, 2019 11:22 AM
> >
> > Hosoya-san,
> >
> > On 2019/02/22 17:14, Yuzuko Hosoya wrote:
> > > Hi,
> > >
> > > I found the bug of default partition pruning when executing a range
> query.
> > >
> > > -----
> > > postgres=# create table test1(id int, val text) partition by range
> > > (id); postgres=# create table test1_1 partition of test1 for values
> > > from (0) to (100); postgres=# create table test1_2 partition of
> > > test1 for values from (150) to (200); postgres=# create table
> > > test1_def partition of test1 default;
> > >
> > > postgres=# explain select * from test1 where id > 0 and id < 30;
> > >                            QUERY PLAN
> > > ----------------------------------------------------------------
> > >  Append  (cost=0.00..11.83 rows=59 width=11)
> > >    ->  Seq Scan on test1_1  (cost=0.00..5.00 rows=58 width=11)
> > >          Filter: ((id > 0) AND (id < 30))
> > >    ->  Seq Scan on test1_def  (cost=0.00..6.53 rows=1 width=12)
> > >          Filter: ((id > 0) AND (id < 30))
> > > (5 rows)
> > >
> > > There is no need to scan the default partition, but it's scanned.
> > > -----
> > >
> > > In the current implement, whether the default partition is scanned
> > > or not is determined according to each condition of given WHERE
> > > clause at get_matching_range_bounds().  In this example,
> > > scan_default is set true according to id > 0 because id >= 200
> > > matches the default partition.  Similarly, according to id < 30,
> scan_default is set true.
> > > Then, these results are combined according to AND/OR at
> perform_pruning_combine_step().
> > > In this case, final result's scan_default is set true.
> > >
> > > The modifications I made are as follows:
> > > - get_matching_range_bounds() determines only offsets of range bounds
> > >   according to each condition
> > > - These results are combined at perform_pruning_combine_step()
> > > - Whether the default partition is scanned or not is determined at
> > >   get_matching_partitions()
> > >
> > > Attached the patch.  Any feedback is greatly appreciated.
> >
> > Thank you for reporting.  Can you please add this to March CF in Bugs
> > category so as not to lose
> track
> > of this?
> >
> > I will try to send review comments soon.
> >
> Thank you for your reply.  I added this to March CF.

I tested with simple use case and I confirmed it works correctly like below.

In case using between clause:
postgres=# create table test1(id int, val text) partition by range (id);
postgres=# create table test1_1 partition of test1 for values from (0) to (100);
postgres=# create table test1_2 partition of test1 for values from (150) to (200);
postgres=# create table test1_def partition of test1 default;

[HEAD]
postgres=# explain analyze select * from test1 where id between 0 and 50;
                                                QUERY PLAN                                                
-----------------------------------------------------------------------------------------------------------
 Append  (cost=0.00..58.16 rows=12 width=36) (actual time=0.008..0.008 rows=0 loops=1)
   ->  Seq Scan on test1_1  (cost=0.00..29.05 rows=6 width=36) (actual time=0.005..0.005 rows=0 loops=1)
         Filter: ((id >= 0) AND (id <= 50))
   ->  Seq Scan on test1_def  (cost=0.00..29.05 rows=6 width=36) (actual time=0.002..0.002 rows=0 loops=1)
         Filter: ((id >= 0) AND (id <= 50))


[patched]
postgres=# explain analyze select * from test1 where id between 0 and 50;
                                               QUERY PLAN                                                
---------------------------------------------------------------------------------------------------------
 Append  (cost=0.00..29.08 rows=6 width=36) (actual time=0.006..0.006 rows=0 loops=1)
   ->  Seq Scan on test1_1  (cost=0.00..29.05 rows=6 width=36) (actual time=0.004..0.005 rows=0 loops=1)
         Filter: ((id >= 0) AND (id <= 50))



I considered about another use case. If default partition contains rows whose id = 300 and then we add another partition which have constraints like id >= 300 and id < 400, I thought we won't scan the rows anymore. But I noticed we simply can't add such a partition.

postgres=# insert into test1 values (300);
INSERT 0 1
postgres=# create table test1_3 partition of test1 for values from (300) to (400);
ERROR:  updated partition constraint for default partition "test1_def" would be violated by some row


So I haven't come up with bad cases so far :)

--
Yoshikazu Imai



Reply | Threaded
Open this post in threaded view
|

Re: Problem with default partition pruning

Ibrar Ahmed-4
Hi

Patch work fine to me, but I have one test case where default partition still scanned.

postgres=# explain select * from test1 where (id < 10) and true;
                            QUERY PLAN                            
-------------------------------------------------------------------
 Append  (cost=0.00..55.98 rows=846 width=36)
   ->  Seq Scan on test1_1  (cost=0.00..25.88 rows=423 width=36)
         Filter: (id < 10)
   ->  Seq Scan on test1_def  (cost=0.00..25.88 rows=423 width=36)
         Filter: (id < 10)
(5 rows)
Reply | Threaded
Open this post in threaded view
|

Re: Problem with default partition pruning

Ibrar Ahmed-4
Hi  Yuzuko Hosoya,

Ignore my last message, I think this is also a legitimate scan on default partition.


On Mon, Mar 4, 2019 at 10:29 PM Ibrar Ahmed <[hidden email]> wrote:
Hi

Patch work fine to me, but I have one test case where default partition still scanned.

postgres=# explain select * from test1 where (id < 10) and true;
                            QUERY PLAN                             
-------------------------------------------------------------------
 Append  (cost=0.00..55.98 rows=846 width=36)
   ->  Seq Scan on test1_1  (cost=0.00..25.88 rows=423 width=36)
         Filter: (id < 10)
   ->  Seq Scan on test1_def  (cost=0.00..25.88 rows=423 width=36)
         Filter: (id < 10)
(5 rows)


--
Ibrar Ahmed
Reply | Threaded
Open this post in threaded view
|

Re: Problem with default partition pruning

yuzuko
In reply to this post by Imai, Yoshikazu
Imai-san,

Thanks for sharing your tests!

On Thu, Feb 28, 2019 at 5:27 PM Imai, Yoshikazu
<[hidden email]> wrote:

>
> Hosoya-san
>
> On Wed, Feb 27, 2019 at 6:51 AM, Yuzuko Hosoya wrote:
> > > From: Amit Langote [mailto:[hidden email]]
> > > Sent: Wednesday, February 27, 2019 11:22 AM
> > >
> > > Hosoya-san,
> > >
> > > On 2019/02/22 17:14, Yuzuko Hosoya wrote:
> > > > Hi,
> > > >
> > > > I found the bug of default partition pruning when executing a range
> > query.
> > > >
> > > > -----
> > > > postgres=# create table test1(id int, val text) partition by range
> > > > (id); postgres=# create table test1_1 partition of test1 for values
> > > > from (0) to (100); postgres=# create table test1_2 partition of
> > > > test1 for values from (150) to (200); postgres=# create table
> > > > test1_def partition of test1 default;
> > > >
> > > > postgres=# explain select * from test1 where id > 0 and id < 30;
> > > >                            QUERY PLAN
> > > > ----------------------------------------------------------------
> > > >  Append  (cost=0.00..11.83 rows=59 width=11)
> > > >    ->  Seq Scan on test1_1  (cost=0.00..5.00 rows=58 width=11)
> > > >          Filter: ((id > 0) AND (id < 30))
> > > >    ->  Seq Scan on test1_def  (cost=0.00..6.53 rows=1 width=12)
> > > >          Filter: ((id > 0) AND (id < 30))
> > > > (5 rows)
> > > >
> > > > There is no need to scan the default partition, but it's scanned.
> > > > -----
> > > >
> > > > In the current implement, whether the default partition is scanned
> > > > or not is determined according to each condition of given WHERE
> > > > clause at get_matching_range_bounds().  In this example,
> > > > scan_default is set true according to id > 0 because id >= 200
> > > > matches the default partition.  Similarly, according to id < 30,
> > scan_default is set true.
> > > > Then, these results are combined according to AND/OR at
> > perform_pruning_combine_step().
> > > > In this case, final result's scan_default is set true.
> > > >
> > > > The modifications I made are as follows:
> > > > - get_matching_range_bounds() determines only offsets of range bounds
> > > >   according to each condition
> > > > - These results are combined at perform_pruning_combine_step()
> > > > - Whether the default partition is scanned or not is determined at
> > > >   get_matching_partitions()
> > > >
> > > > Attached the patch.  Any feedback is greatly appreciated.
> > >
> > > Thank you for reporting.  Can you please add this to March CF in Bugs
> > > category so as not to lose
> > track
> > > of this?
> > >
> > > I will try to send review comments soon.
> > >
> > Thank you for your reply.  I added this to March CF.
>
> I tested with simple use case and I confirmed it works correctly like below.
>
> In case using between clause:
> postgres=# create table test1(id int, val text) partition by range (id);
> postgres=# create table test1_1 partition of test1 for values from (0) to (100);
> postgres=# create table test1_2 partition of test1 for values from (150) to (200);
> postgres=# create table test1_def partition of test1 default;
>
> [HEAD]
> postgres=# explain analyze select * from test1 where id between 0 and 50;
>                                                 QUERY PLAN
> -----------------------------------------------------------------------------------------------------------
>  Append  (cost=0.00..58.16 rows=12 width=36) (actual time=0.008..0.008 rows=0 loops=1)
>    ->  Seq Scan on test1_1  (cost=0.00..29.05 rows=6 width=36) (actual time=0.005..0.005 rows=0 loops=1)
>          Filter: ((id >= 0) AND (id <= 50))
>    ->  Seq Scan on test1_def  (cost=0.00..29.05 rows=6 width=36) (actual time=0.002..0.002 rows=0 loops=1)
>          Filter: ((id >= 0) AND (id <= 50))
>
>
> [patched]
> postgres=# explain analyze select * from test1 where id between 0 and 50;
>                                                QUERY PLAN
> ---------------------------------------------------------------------------------------------------------
>  Append  (cost=0.00..29.08 rows=6 width=36) (actual time=0.006..0.006 rows=0 loops=1)
>    ->  Seq Scan on test1_1  (cost=0.00..29.05 rows=6 width=36) (actual time=0.004..0.005 rows=0 loops=1)
>          Filter: ((id >= 0) AND (id <= 50))
>
>
>
> I considered about another use case. If default partition contains rows whose id = 300 and then we add another partition which have constraints like id >= 300 and id < 400, I thought we won't scan the rows anymore. But I noticed we simply can't add such a partition.
>
> postgres=# insert into test1 values (300);
> INSERT 0 1
> postgres=# create table test1_3 partition of test1 for values from (300) to (400);
> ERROR:  updated partition constraint for default partition "test1_def" would be violated by some row
>
>
> So I haven't come up with bad cases so far :)

I didn't test cases you mentioned.
Thanks to you, I could check correctness of the patch!

--
Best regards,
Yuzuko Hosoya
NTT Open Source Software Center

Reply | Threaded
Open this post in threaded view
|

Re: Problem with default partition pruning

yuzuko
In reply to this post by Ibrar Ahmed-4
Hi Ibrar,

On Tue, Mar 5, 2019 at 2:37 AM Ibrar Ahmed <[hidden email]> wrote:
>
> Hi  Yuzuko Hosoya,
>
> Ignore my last message, I think this is also a legitimate scan on default partition.
>
Oh, I got it.  Thanks a lot.

>
> On Mon, Mar 4, 2019 at 10:29 PM Ibrar Ahmed <[hidden email]> wrote:
>>
>> Hi
>>
>> Patch work fine to me, but I have one test case where default partition still scanned.
>>
>> postgres=# explain select * from test1 where (id < 10) and true;
>>                             QUERY PLAN
>> -------------------------------------------------------------------
>>  Append  (cost=0.00..55.98 rows=846 width=36)
>>    ->  Seq Scan on test1_1  (cost=0.00..25.88 rows=423 width=36)
>>          Filter: (id < 10)
>>    ->  Seq Scan on test1_def  (cost=0.00..25.88 rows=423 width=36)
>>          Filter: (id < 10)
>> (5 rows)
>
>
>
> --
> Ibrar Ahmed

--
Best regards,
Yuzuko Hosoya
NTT Open Source Software Center

Reply | Threaded
Open this post in threaded view
|

Re: Problem with default partition pruning

Thibaut Madelaine
In reply to this post by Imai, Yoshikazu

Le 28/02/2019 à 09:26, Imai, Yoshikazu a écrit :

> Hosoya-san
>
> On Wed, Feb 27, 2019 at 6:51 AM, Yuzuko Hosoya wrote:
>>> From: Amit Langote [mailto:[hidden email]]
>>> Sent: Wednesday, February 27, 2019 11:22 AM
>>>
>>> Hosoya-san,
>>>
>>> On 2019/02/22 17:14, Yuzuko Hosoya wrote:
>>>> Hi,
>>>>
>>>> I found the bug of default partition pruning when executing a range
>> query.
>>>> -----
>>>> postgres=# create table test1(id int, val text) partition by range
>>>> (id); postgres=# create table test1_1 partition of test1 for values
>>>> from (0) to (100); postgres=# create table test1_2 partition of
>>>> test1 for values from (150) to (200); postgres=# create table
>>>> test1_def partition of test1 default;
>>>>
>>>> postgres=# explain select * from test1 where id > 0 and id < 30;
>>>>                            QUERY PLAN
>>>> ----------------------------------------------------------------
>>>>  Append  (cost=0.00..11.83 rows=59 width=11)
>>>>    ->  Seq Scan on test1_1  (cost=0.00..5.00 rows=58 width=11)
>>>>          Filter: ((id > 0) AND (id < 30))
>>>>    ->  Seq Scan on test1_def  (cost=0.00..6.53 rows=1 width=12)
>>>>          Filter: ((id > 0) AND (id < 30))
>>>> (5 rows)
>>>>
>>>> There is no need to scan the default partition, but it's scanned.
>>>> -----
>>>>
>>>> In the current implement, whether the default partition is scanned
>>>> or not is determined according to each condition of given WHERE
>>>> clause at get_matching_range_bounds().  In this example,
>>>> scan_default is set true according to id > 0 because id >= 200
>>>> matches the default partition.  Similarly, according to id < 30,
>> scan_default is set true.
>>>> Then, these results are combined according to AND/OR at
>> perform_pruning_combine_step().
>>>> In this case, final result's scan_default is set true.
>>>>
>>>> The modifications I made are as follows:
>>>> - get_matching_range_bounds() determines only offsets of range bounds
>>>>   according to each condition
>>>> - These results are combined at perform_pruning_combine_step()
>>>> - Whether the default partition is scanned or not is determined at
>>>>   get_matching_partitions()
>>>>
>>>> Attached the patch.  Any feedback is greatly appreciated.
>>> Thank you for reporting.  Can you please add this to March CF in Bugs
>>> category so as not to lose
>> track
>>> of this?
>>>
>>> I will try to send review comments soon.
>>>
>> Thank you for your reply.  I added this to March CF.
> I tested with simple use case and I confirmed it works correctly like below.
>
> In case using between clause:
> postgres=# create table test1(id int, val text) partition by range (id);
> postgres=# create table test1_1 partition of test1 for values from (0) to (100);
> postgres=# create table test1_2 partition of test1 for values from (150) to (200);
> postgres=# create table test1_def partition of test1 default;
>
> [HEAD]
> postgres=# explain analyze select * from test1 where id between 0 and 50;
>                                                 QUERY PLAN                                                
> -----------------------------------------------------------------------------------------------------------
>  Append  (cost=0.00..58.16 rows=12 width=36) (actual time=0.008..0.008 rows=0 loops=1)
>    ->  Seq Scan on test1_1  (cost=0.00..29.05 rows=6 width=36) (actual time=0.005..0.005 rows=0 loops=1)
>          Filter: ((id >= 0) AND (id <= 50))
>    ->  Seq Scan on test1_def  (cost=0.00..29.05 rows=6 width=36) (actual time=0.002..0.002 rows=0 loops=1)
>          Filter: ((id >= 0) AND (id <= 50))
>
>
> [patched]
> postgres=# explain analyze select * from test1 where id between 0 and 50;
>                                                QUERY PLAN                                                
> ---------------------------------------------------------------------------------------------------------
>  Append  (cost=0.00..29.08 rows=6 width=36) (actual time=0.006..0.006 rows=0 loops=1)
>    ->  Seq Scan on test1_1  (cost=0.00..29.05 rows=6 width=36) (actual time=0.004..0.005 rows=0 loops=1)
>          Filter: ((id >= 0) AND (id <= 50))
>
>
>
> I considered about another use case. If default partition contains rows whose id = 300 and then we add another partition which have constraints like id >= 300 and id < 400, I thought we won't scan the rows anymore. But I noticed we simply can't add such a partition.
>
> postgres=# insert into test1 values (300);
> INSERT 0 1
> postgres=# create table test1_3 partition of test1 for values from (300) to (400);
> ERROR:  updated partition constraint for default partition "test1_def" would be violated by some row
>
>
> So I haven't come up with bad cases so far :)
>
> --
> Yoshikazu Imai

Hello Yoshikazu-San,

I tested your patch using some sub-partitions and found a possible problem.

I create a new partitioned partition test1_3 with 2 sub-partitions :

-------------------------

create table test1_3 partition of test1 for values from (200) to (400)
partition by range (id);
create table test1_3_1 partition of test1_3 for values from (200) to (250);
create table test1_3_2 partition of test1_3 for values from (250) to (350);

# explain select * from test1 where (id > 0 and id < 30);
                          QUERY PLAN                          
---------------------------------------------------------------
 Append  (cost=0.00..29.08 rows=6 width=36)
   ->  Seq Scan on test1_1  (cost=0.00..29.05 rows=6 width=36)
         Filter: ((id > 0) AND (id < 30))
(3 rows)

# explain select * from test1 where (id > 220 and id < 230);
                           QUERY PLAN                           
-----------------------------------------------------------------
 Append  (cost=0.00..29.08 rows=6 width=36)
   ->  Seq Scan on test1_3_1  (cost=0.00..29.05 rows=6 width=36)
         Filter: ((id > 220) AND (id < 230))
(3 rows)

# explain select * from test1
where (id > 0 and id < 30) or (id > 220 and id < 230);
                                QUERY PLAN                                
---------------------------------------------------------------------------
 Append  (cost=0.00..106.40 rows=39 width=36)
   ->  Seq Scan on test1_1  (cost=0.00..35.40 rows=13 width=36)
         Filter: (((id > 0) AND (id < 30)) OR ((id > 220) AND (id < 230)))
   ->  Seq Scan on test1_3_1  (cost=0.00..35.40 rows=13 width=36)
         Filter: (((id > 0) AND (id < 30)) OR ((id > 220) AND (id < 230)))
   ->  Seq Scan on test1_3_2  (cost=0.00..35.40 rows=13 width=36)
         Filter: (((id > 0) AND (id < 30)) OR ((id > 220) AND (id < 230)))
(7 rows)

-----------------

Partition pruning is functioning when only the sub-partition is
required. When both the partition and the sub-partition is required,
there is no pruning on the sub-partition.

Cordialement,

--
Thibaut Madelaine
Dalibo



Reply | Threaded
Open this post in threaded view
|

RE: Problem with default partition pruning

Yuzuko Hosoya
Hi Thibaut,

Thanks a lot for your test and comments.

>
> Le 28/02/2019 à 09:26, Imai, Yoshikazu a écrit :
> > Hosoya-san
> >
> > On Wed, Feb 27, 2019 at 6:51 AM, Yuzuko Hosoya wrote:
> >>> From: Amit Langote [mailto:[hidden email]]
> >>> Sent: Wednesday, February 27, 2019 11:22 AM
> >>>
> >>> Hosoya-san,
> >>>
> >>> On 2019/02/22 17:14, Yuzuko Hosoya wrote:
> >>>> Hi,
> >>>>
> >>>> I found the bug of default partition pruning when executing a range
> >> query.
> >>>> -----
> >>>> postgres=# create table test1(id int, val text) partition by range
> >>>> (id); postgres=# create table test1_1 partition of test1 for values
> >>>> from (0) to (100); postgres=# create table test1_2 partition of
> >>>> test1 for values from (150) to (200); postgres=# create table
> >>>> test1_def partition of test1 default;
> >>>>
> >>>> postgres=# explain select * from test1 where id > 0 and id < 30;
> >>>>                            QUERY PLAN
> >>>> ----------------------------------------------------------------
> >>>>  Append  (cost=0.00..11.83 rows=59 width=11)
> >>>>    ->  Seq Scan on test1_1  (cost=0.00..5.00 rows=58 width=11)
> >>>>          Filter: ((id > 0) AND (id < 30))
> >>>>    ->  Seq Scan on test1_def  (cost=0.00..6.53 rows=1 width=12)
> >>>>          Filter: ((id > 0) AND (id < 30))
> >>>> (5 rows)
> >>>>
> >>>> There is no need to scan the default partition, but it's scanned.
> >>>> -----
> >>>>
> >>>> In the current implement, whether the default partition is scanned
> >>>> or not is determined according to each condition of given WHERE
> >>>> clause at get_matching_range_bounds().  In this example,
> >>>> scan_default is set true according to id > 0 because id >= 200
> >>>> matches the default partition.  Similarly, according to id < 30,
> >> scan_default is set true.
> >>>> Then, these results are combined according to AND/OR at
> >> perform_pruning_combine_step().
> >>>> In this case, final result's scan_default is set true.
> >>>>
> >>>> The modifications I made are as follows:
> >>>> - get_matching_range_bounds() determines only offsets of range bounds
> >>>>   according to each condition
> >>>> - These results are combined at perform_pruning_combine_step()
> >>>> - Whether the default partition is scanned or not is determined at
> >>>>   get_matching_partitions()
> >>>>
> >>>> Attached the patch.  Any feedback is greatly appreciated.
> >>> Thank you for reporting.  Can you please add this to March CF in
> >>> Bugs category so as not to lose
> >> track
> >>> of this?
> >>>
> >>> I will try to send review comments soon.
> >>>
> >> Thank you for your reply.  I added this to March CF.
> > I tested with simple use case and I confirmed it works correctly like below.
> >
> > In case using between clause:
> > postgres=# create table test1(id int, val text) partition by range
> > (id); postgres=# create table test1_1 partition of test1 for values
> > from (0) to (100); postgres=# create table test1_2 partition of test1
> > for values from (150) to (200); postgres=# create table test1_def
> > partition of test1 default;
> >
> > [HEAD]
> > postgres=# explain analyze select * from test1 where id between 0 and 50;
> >                                                 QUERY PLAN
> > ----------------------------------------------------------------------
> > -------------------------------------
> >  Append  (cost=0.00..58.16 rows=12 width=36) (actual time=0.008..0.008 rows=0 loops=1)
> >    ->  Seq Scan on test1_1  (cost=0.00..29.05 rows=6 width=36) (actual time=0.005..0.005
> rows=0 loops=1)
> >          Filter: ((id >= 0) AND (id <= 50))
> >    ->  Seq Scan on test1_def  (cost=0.00..29.05 rows=6 width=36) (actual
> time=0.002..0.002 rows=0 loops=1)
> >          Filter: ((id >= 0) AND (id <= 50))
> >
> >
> > [patched]
> > postgres=# explain analyze select * from test1 where id between 0 and 50;
> >                                                QUERY PLAN
> > ----------------------------------------------------------------------
> > -----------------------------------
> >  Append  (cost=0.00..29.08 rows=6 width=36) (actual time=0.006..0.006 rows=0 loops=1)
> >    ->  Seq Scan on test1_1  (cost=0.00..29.05 rows=6 width=36) (actual time=0.004..0.005
> rows=0 loops=1)
> >          Filter: ((id >= 0) AND (id <= 50))
> >
> >
> >
> > I considered about another use case. If default partition contains rows whose id = 300
> and then we add another partition which have constraints like id >= 300 and id < 400, I thought
> we won't scan the rows anymore. But I noticed we simply can't add such a partition.
> >
> > postgres=# insert into test1 values (300); INSERT 0 1 postgres=#
> > create table test1_3 partition of test1 for values from (300) to
> > (400);
> > ERROR:  updated partition constraint for default partition "test1_def"
> > would be violated by some row
> >
> >
> > So I haven't come up with bad cases so far :)
> >
> > --
> > Yoshikazu Imai
>
> Hello Yoshikazu-San,
>
> I tested your patch using some sub-partitions and found a possible problem.
>
> I create a new partitioned partition test1_3 with 2 sub-partitions :
>
> -------------------------
>
> create table test1_3 partition of test1 for values from (200) to (400) partition by range
> (id); create table test1_3_1 partition of test1_3 for values from (200) to (250); create
> table test1_3_2 partition of test1_3 for values from (250) to (350);
>
> # explain select * from test1 where (id > 0 and id < 30);
>                           QUERY PLAN
> ---------------------------------------------------------------
>  Append  (cost=0.00..29.08 rows=6 width=36)
>    ->  Seq Scan on test1_1  (cost=0.00..29.05 rows=6 width=36)
>          Filter: ((id > 0) AND (id < 30))
> (3 rows)
>
> # explain select * from test1 where (id > 220 and id < 230);
>                            QUERY PLAN
> -----------------------------------------------------------------
>  Append  (cost=0.00..29.08 rows=6 width=36)
>    ->  Seq Scan on test1_3_1  (cost=0.00..29.05 rows=6 width=36)
>          Filter: ((id > 220) AND (id < 230))
> (3 rows)
>
> # explain select * from test1
> where (id > 0 and id < 30) or (id > 220 and id < 230);
>                                 QUERY PLAN
> ---------------------------------------------------------------------------
>  Append  (cost=0.00..106.40 rows=39 width=36)
>    ->  Seq Scan on test1_1  (cost=0.00..35.40 rows=13 width=36)
>          Filter: (((id > 0) AND (id < 30)) OR ((id > 220) AND (id < 230)))
>    ->  Seq Scan on test1_3_1  (cost=0.00..35.40 rows=13 width=36)
>          Filter: (((id > 0) AND (id < 30)) OR ((id > 220) AND (id < 230)))
>    ->  Seq Scan on test1_3_2  (cost=0.00..35.40 rows=13 width=36)
>          Filter: (((id > 0) AND (id < 30)) OR ((id > 220) AND (id < 230)))
> (7 rows)
>
> -----------------
>
> Partition pruning is functioning when only the sub-partition is required. When both the
> partition and the sub-partition is required, there is no pruning on the sub-partition.
>
Indeed, it's problematic.  I also did test and I found that
this problem was occurred when any partition didn't match
WHERE clauses.  So following query didn't work correctly.

# explain select * from test1_3 where (id > 0 and id < 30);            
                           QUERY PLAN          
-----------------------------------------------------------------
 Append  (cost=0.00..58.16 rows=12 width=36)
   ->  Seq Scan on test1_3_1  (cost=0.00..29.05 rows=6 width=36)
         Filter: ((id > 0) AND (id < 30))
   ->  Seq Scan on test1_3_2  (cost=0.00..29.05 rows=6 width=36)
         Filter: ((id > 0) AND (id < 30))
(5 rows)

I created a new patch to handle this problem, and confirmed
the query you mentioned works as expected

# explain select * from test1 where (id > 0 and id < 30) or (id > 220 and id < 230);
                                QUERY PLAN                                
---------------------------------------------------------------------------
 Append  (cost=0.00..70.93 rows=26 width=36)
   ->  Seq Scan on test1_1_1  (cost=0.00..35.40 rows=13 width=36)
         Filter: (((id > 0) AND (id < 30)) OR ((id > 220) AND (id < 230)))
   ->  Seq Scan on test1_3_1  (cost=0.00..35.40 rows=13 width=36)
         Filter: (((id > 0) AND (id < 30)) OR ((id > 220) AND (id < 230)))
(5 rows)

v2 patch attached.
Could you please check it again?

--
Best regards,
Yuzuko Hosoya

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

Re: Problem with default partition pruning

Kyotaro HORIGUCHI-2
Hello.

At Fri, 15 Mar 2019 15:05:41 +0900, "Yuzuko Hosoya" <[hidden email]> wrote in <001901d4daf5$1ef4f640$5cdee2c0$@lab.ntt.co.jp>
> v2 patch attached.
> Could you please check it again?

I have some comments on the patch itself.

The patch relies on the fact(?) that the lowest index is always
-1 in range partition and uses it as pseudo default
partition. I'm not sure it is really the fact and anyway it
donsn't seem the right thing to do. Could you explain how it
works, not what you did in this patch?


L96:
>                      /* There can only be zero or one matching partition. */
> -                    if (partindices[off + 1] >= 0)
> -                        result->bound_offsets = bms_make_singleton(off + 1);
> -                    else
> -                        result->scan_default =
> -                            partition_bound_has_default(boundinfo);
> +                    result->bound_offsets = bms_make_singleton(off + 1);

The comment had a meaning for the old code. Seems to need rewrite?

L183:
> +                /*                                                                                                
> +                 * All bounds are greater than the key, so we could only                                          
> +                 * expect to find the lookup key in the default partition.                                        
> +                 */

Long trailing spaces are attached to every line without
substantial modification.

L198:
> -                 * inclusive, no need add the adjacent partition.
> +                 * inclusive, no need add the adjacent partition.  If 'off' is
> +                 * -1 indicating that all bounds are greater, then we simply
> +                 * end up adding the first bound's offset, that is, 0.

 off doesn't seem to be -1 there.

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
In reply to this post by Yuzuko Hosoya
Hosoya-san,

On 2019/03/15 15:05, Yuzuko Hosoya wrote:

> Indeed, it's problematic.  I also did test and I found that
> this problem was occurred when any partition didn't match
> WHERE clauses.  So following query didn't work correctly.
>
> # explain select * from test1_3 where (id > 0 and id < 30);            
>                            QUERY PLAN          
> -----------------------------------------------------------------
>  Append  (cost=0.00..58.16 rows=12 width=36)
>    ->  Seq Scan on test1_3_1  (cost=0.00..29.05 rows=6 width=36)
>          Filter: ((id > 0) AND (id < 30))
>    ->  Seq Scan on test1_3_2  (cost=0.00..29.05 rows=6 width=36)
>          Filter: ((id > 0) AND (id < 30))
> (5 rows)
>
> I created a new patch to handle this problem, and confirmed
> the query you mentioned works as expected
>
> # explain select * from test1 where (id > 0 and id < 30) or (id > 220 and id < 230);
>                                 QUERY PLAN                                
> ---------------------------------------------------------------------------
>  Append  (cost=0.00..70.93 rows=26 width=36)
>    ->  Seq Scan on test1_1_1  (cost=0.00..35.40 rows=13 width=36)
>          Filter: (((id > 0) AND (id < 30)) OR ((id > 220) AND (id < 230)))
>    ->  Seq Scan on test1_3_1  (cost=0.00..35.40 rows=13 width=36)
>          Filter: (((id > 0) AND (id < 30)) OR ((id > 220) AND (id < 230)))
> (5 rows)
>
> v2 patch attached.
> Could you please check it again?
I think the updated patch breaks the promise that
get_matching_range_bounds won't set scan_default based on individual
pruning value comparisons.  How about the attached delta patch that
applies on top of your earlier v1 patch, which fixes the issue reported by
Thibaut?

Thanks,
Amit

v1-delta.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Problem with default partition pruning

Kyotaro HORIGUCHI-2
In reply to this post by Kyotaro HORIGUCHI-2
Hello.

At Fri, 15 Mar 2019 17:30:07 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <[hidden email]> wrote in <[hidden email]>
> The patch relies on the fact(?) that the lowest index is always
> -1 in range partition and uses it as pseudo default
> partition. I'm not sure it is really the fact and anyway it
> donsn't seem the right thing to do. Could you explain how it
> works, not what you did in this patch?

I understood how it works but still uneasy that only list
partitioning requires scan_default. Anyway please ignore this.

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 Amit Langote-2
Hi.

At Mon, 18 Mar 2019 18:44:07 +0900, Amit Langote <[hidden email]> wrote in <[hidden email]>
> > v2 patch attached.
> > Could you please check it again?
>
> I think the updated patch breaks the promise that
> get_matching_range_bounds won't set scan_default based on individual
> pruning value comparisons.  How about the attached delta patch that
> applies on top of your earlier v1 patch, which fixes the issue reported by
> Thibaut?

I read through the patch and understood how it works. And Amit's
proposal looks fine.

But that makes me think of scan_default as a wart.

The attached patch is a refactoring that removes scan_default
from PruneStepResult and the defaut partition is represented as
the same way as non-default partitions, without changing in
behavior. This improves the modularity of partprune code a bit.

The fix would be put on top of this easily.

Thoughts?

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

diff --git a/src/backend/partitioning/partbounds.c b/src/backend/partitioning/partbounds.c
index 5b897d50ee..828240119d 100644
--- a/src/backend/partitioning/partbounds.c
+++ b/src/backend/partitioning/partbounds.c
@@ -92,7 +92,6 @@ static int partition_range_bsearch(int partnatts, FmgrInfo *partsupfunc,
  Oid *partcollation,
  PartitionBoundInfo boundinfo,
  PartitionRangeBound *probe, bool *is_equal);
-static int get_partition_bound_num_indexes(PartitionBoundInfo b);
 static Expr *make_partition_op_expr(PartitionKey key, int keynum,
    uint16 strategy, Expr *arg1, Expr *arg2);
 static Oid get_partition_operator(PartitionKey key, int col,
@@ -266,6 +265,7 @@ create_hash_bounds(PartitionBoundSpec **boundspecs, int nparts,
 
  boundinfo->ndatums = ndatums;
  boundinfo->datums = (Datum **) palloc0(ndatums * sizeof(Datum *));
+ boundinfo->nindexes = greatest_modulus;
  boundinfo->indexes = (int *) palloc(greatest_modulus * sizeof(int));
  for (i = 0; i < greatest_modulus; i++)
  boundinfo->indexes[i] = -1;
@@ -399,7 +399,10 @@ create_list_bounds(PartitionBoundSpec **boundspecs, int nparts,
 
  boundinfo->ndatums = ndatums;
  boundinfo->datums = (Datum **) palloc0(ndatums * sizeof(Datum *));
- boundinfo->indexes = (int *) palloc(ndatums * sizeof(int));
+
+ /* the last element is reserved for the default partition */
+ boundinfo->nindexes = ndatums + 1;
+ boundinfo->indexes = (int *) palloc(boundinfo->nindexes *  sizeof(int));
 
  /*
  * Copy values.  Canonical indexes are values ranging from 0 to (nparts -
@@ -423,6 +426,9 @@ create_list_bounds(PartitionBoundSpec **boundspecs, int nparts,
  boundinfo->indexes[i] = (*mapping)[orig_index];
  }
 
+ /* set default partition index (-1) */
+ boundinfo->indexes[ndatums] = -1;
+
  /*
  * Set the canonical value for null_index, if any.
  *
@@ -596,7 +602,8 @@ create_range_bounds(PartitionBoundSpec **boundspecs, int nparts,
  * For range partitioning, an additional value of -1 is stored as the last
  * element.
  */
- boundinfo->indexes = (int *) palloc((ndatums + 1) * sizeof(int));
+ boundinfo->nindexes = ndatums + 1;
+ boundinfo->indexes = (int *) palloc(boundinfo->nindexes * sizeof(int));
 
  for (i = 0; i < ndatums; i++)
  {
@@ -676,6 +683,9 @@ partition_bounds_equal(int partnatts, int16 *parttyplen, bool *parttypbyval,
  if (b1->ndatums != b2->ndatums)
  return false;
 
+ if (b1->nindexes != b2->nindexes)
+ return false;
+
  if (b1->null_index != b2->null_index)
  return false;
 
@@ -704,7 +714,7 @@ partition_bounds_equal(int partnatts, int16 *parttyplen, bool *parttypbyval,
  * their indexes array will be same.  So, it suffices to compare
  * indexes array.
  */
- for (i = 0; i < greatest_modulus; i++)
+ for (i = 0; i < b1->nindexes; i++)
  if (b1->indexes[i] != b2->indexes[i])
  return false;
 
@@ -765,9 +775,9 @@ partition_bounds_equal(int partnatts, int16 *parttyplen, bool *parttypbyval,
  return false;
  }
 
- /* There are ndatums+1 indexes in case of range partitions */
- if (b1->strategy == PARTITION_STRATEGY_RANGE &&
- b1->indexes[i] != b2->indexes[i])
+ /* There may be ndatums+1 indexes in some cases */
+ Assert (i == b1->nindexes || i == b1->nindexes - 1);
+ if (i < b1->nindexes && b1->indexes[i] != b2->indexes[i])
  return false;
  }
  return true;
@@ -793,7 +803,7 @@ partition_bounds_copy(PartitionBoundInfo src,
  ndatums = dest->ndatums = src->ndatums;
  partnatts = key->partnatts;
 
- num_indexes = get_partition_bound_num_indexes(src);
+ num_indexes = dest->nindexes = src->nindexes;
 
  /* List partitioned tables have only a single partition key. */
  Assert(key->strategy != PARTITION_STRATEGY_LIST || partnatts == 1);
@@ -1710,46 +1720,6 @@ qsort_partition_rbound_cmp(const void *a, const void *b, void *arg)
  b1->lower, b2);
 }
 
-/*
- * get_partition_bound_num_indexes
- *
- * Returns the number of the entries in the partition bound indexes array.
- */
-static int
-get_partition_bound_num_indexes(PartitionBoundInfo bound)
-{
- int num_indexes;
-
- Assert(bound);
-
- switch (bound->strategy)
- {
- case PARTITION_STRATEGY_HASH:
-
- /*
- * The number of the entries in the indexes array is same as the
- * greatest modulus.
- */
- num_indexes = get_hash_partition_greatest_modulus(bound);
- break;
-
- case PARTITION_STRATEGY_LIST:
- num_indexes = bound->ndatums;
- break;
-
- case PARTITION_STRATEGY_RANGE:
- /* Range partitioned table has an extra index. */
- num_indexes = bound->ndatums + 1;
- break;
-
- default:
- elog(ERROR, "unexpected partition strategy: %d",
- (int) bound->strategy);
- }
-
- return num_indexes;
-}
-
 /*
  * get_partition_operator
  *
diff --git a/src/backend/partitioning/partprune.c b/src/backend/partitioning/partprune.c
index 95a60183a1..3cc9c9f5b8 100644
--- a/src/backend/partitioning/partprune.c
+++ b/src/backend/partitioning/partprune.c
@@ -105,7 +105,6 @@ typedef struct PruneStepResult
  */
  Bitmapset  *bound_offsets;
 
- bool scan_default; /* Scan the default partition? */
  bool scan_null; /* Scan the partition for NULL values? */
 } PruneStepResult;
 
@@ -671,23 +670,20 @@ get_matching_partitions(PartitionPruneContext *context, List *pruning_steps)
  Assert(final_result != NULL);
  i = -1;
  result = NULL;
- scan_default = final_result->scan_default;
+ scan_default = false;
  while ((i = bms_next_member(final_result->bound_offsets, i)) >= 0)
  {
  int partindex = context->boundinfo->indexes[i];
 
  /*
- * In range and hash partitioning cases, some index values may be -1,
- * indicating that no partition has been defined to accept a given
- * range of values (that is, the bound at given offset is the upper
- * bound of this unassigned range of values) or for a given remainder,
- * respectively.  As it's still part of the queried range of values,
- * add the default partition, if any.
+ * Some index slot may contain -1, indicating that no partition has
+ * been defined to accept a given range of values. As it's still part
+ * of the queried range of values, add the default partition, if any.
  */
  if (partindex >= 0)
  result = bms_add_member(result, partindex);
- else
- scan_default |= partition_bound_has_default(context->boundinfo);
+ else if (partition_bound_has_default(context->boundinfo))
+ scan_default = true;
  }
 
  /* Add the null and/or default partition if needed and if present. */
@@ -2202,7 +2198,7 @@ get_matching_hash_bounds(PartitionPruneContext *context,
  * There is neither a special hash null partition or the default hash
  * partition.
  */
- result->scan_null = result->scan_default = false;
+ result->scan_null = false;
 
  return result;
 }
@@ -2212,10 +2208,6 @@ get_matching_hash_bounds(PartitionPruneContext *context,
  * Determine the offsets of list bounds matching the specified value,
  * according to the semantics of the given operator strategy
  *
- * scan_default will be set in the returned struct, if the default partition
- * needs to be scanned, provided one exists at all.  scan_null will be set if
- * the special null-accepting partition needs to be scanned.
- *
  * 'opstrategy' if non-zero must be a btree strategy number.
  *
  * 'value' contains the value to use for pruning.
@@ -2244,7 +2236,7 @@ get_matching_list_bounds(PartitionPruneContext *context,
  Assert(context->strategy == PARTITION_STRATEGY_LIST);
  Assert(context->partnatts == 1);
 
- result->scan_null = result->scan_default = false;
+ result->scan_null = false;
 
  if (!bms_is_empty(nullkeys))
  {
@@ -2256,7 +2248,8 @@ get_matching_list_bounds(PartitionPruneContext *context,
  if (partition_bound_accepts_nulls(boundinfo))
  result->scan_null = true;
  else
- result->scan_default = partition_bound_has_default(boundinfo);
+ /* scan the default partition, if any */
+ result->bound_offsets = bms_make_singleton(boundinfo->ndatums);
  return result;
  }
 
@@ -2266,7 +2259,7 @@ get_matching_list_bounds(PartitionPruneContext *context,
  */
  if (boundinfo->ndatums == 0)
  {
- result->scan_default = partition_bound_has_default(boundinfo);
+ result->bound_offsets = bms_make_singleton(boundinfo->ndatums);
  return result;
  }
 
@@ -2280,10 +2273,9 @@ get_matching_list_bounds(PartitionPruneContext *context,
  */
  if (nvalues == 0)
  {
- Assert(boundinfo->ndatums > 0);
- result->bound_offsets = bms_add_range(NULL, 0,
-  boundinfo->ndatums - 1);
- result->scan_default = partition_bound_has_default(boundinfo);
+ Assert(boundinfo->nindexes > 0);
+ result->bound_offsets = bms_add_range(result->bound_offsets,
+  0, boundinfo->nindexes - 1);
  return result;
  }
 
@@ -2294,8 +2286,8 @@ get_matching_list_bounds(PartitionPruneContext *context,
  * First match to all bounds.  We'll remove any matching datums below.
  */
  Assert(boundinfo->ndatums > 0);
- result->bound_offsets = bms_add_range(NULL, 0,
-  boundinfo->ndatums - 1);
+ result->bound_offsets = bms_add_range(result->bound_offsets,
+  0, boundinfo->ndatums);
 
  off = partition_list_bsearch(partsupfunc, partcollation, boundinfo,
  value, &is_equal);
@@ -2308,23 +2300,9 @@ get_matching_list_bounds(PartitionPruneContext *context,
    off);
  }
 
- /* Always include the default partition if any. */
- result->scan_default = partition_bound_has_default(boundinfo);
-
  return result;
  }
 
- /*
- * With range queries, always include the default list partition, because
- * list partitions divide the key space in a discontinuous manner, not all
- * values in the given range will have a partition assigned.  This may not
- * technically be true for some data types (e.g. integer types), however,
- * we currently lack any sort of infrastructure to provide us with proofs
- * that would allow us to do anything smarter here.
- */
- if (opstrategy != BTEqualStrategyNumber)
- result->scan_default = partition_bound_has_default(boundinfo);
-
  switch (opstrategy)
  {
  case BTEqualStrategyNumber:
@@ -2338,7 +2316,9 @@ get_matching_list_bounds(PartitionPruneContext *context,
  result->bound_offsets = bms_make_singleton(off);
  }
  else
- result->scan_default = partition_bound_has_default(boundinfo);
+ /* scan the default partition, if any */
+ result->bound_offsets = bms_make_singleton(boundinfo->ndatums);
+
  return result;
 
  case BTGreaterEqualStrategyNumber:
@@ -2367,11 +2347,14 @@ get_matching_list_bounds(PartitionPruneContext *context,
  /*
  * off is greater than the numbers of datums we have partitions
  * for.  The only possible partition that could contain a match is
- * the default partition, but we must've set context->scan_default
- * above anyway if one exists.
+ * the default partition. Scan the default partition if one
+ * exists.
  */
  if (off > boundinfo->ndatums - 1)
+ {
+ result->bound_offsets = bms_make_singleton(boundinfo->ndatums);
  return result;
+ }
 
  minoff = off;
  break;
@@ -2390,11 +2373,14 @@ get_matching_list_bounds(PartitionPruneContext *context,
  /*
  * off is smaller than the datums of all non-default partitions.
  * The only possible partition that could contain a match is the
- * default partition, but we must've set context->scan_default
- * above anyway if one exists.
+ * default partition, but we scan the default partition if one
+ * exists.
  */
  if (off < 0)
+ {
+ result->bound_offsets = bms_make_singleton(boundinfo->ndatums);
  return result;
+ }
 
  maxoff = off;
  break;
@@ -2404,8 +2390,21 @@ get_matching_list_bounds(PartitionPruneContext *context,
  break;
  }
 
+ /*
+ * With range queries, always include the default list partition, because
+ * list partitions divide the key space in a discontinuous manner, not all
+ * values in the given range will have a partition assigned.  This may not
+ * technically be true for some data types (e.g. integer types), however,
+ * we currently lack any sort of infrastructure to provide us with proofs
+ * that would allow us to do anything smarter here.
+ */
+ Assert (opstrategy != BTEqualStrategyNumber);
+ result->bound_offsets = bms_add_member(result->bound_offsets,
+   boundinfo->ndatums);
+
  Assert(minoff >= 0 && maxoff >= 0);
- result->bound_offsets = bms_add_range(NULL, minoff, maxoff);
+ result->bound_offsets = bms_add_range(result->bound_offsets,
+  minoff, maxoff);
  return result;
 }
 
@@ -2418,9 +2417,8 @@ get_matching_list_bounds(PartitionPruneContext *context,
  * Each datum whose offset is in result is to be treated as the upper bound of
  * the partition that will contain the desired values.
  *
- * scan_default will be set in the returned struct, if the default partition
- * needs to be scanned, provided one exists at all.  Although note that we
- * intentionally don't set scan_default in this function if only because the
+ * bound_offsets may contain a bit for the indexes element that contains -1,
+ * which means the default partition if any.  That happens only if the
  * matching set of values, found by comparing the input values using the
  * specified opstrategy, contains unassigned portions of key space, which
  * we normally assume to belong to the default partition.  We don't infer
@@ -2461,7 +2459,7 @@ get_matching_range_bounds(PartitionPruneContext *context,
  Assert(context->strategy == PARTITION_STRATEGY_RANGE);
  Assert(nvalues <= partnatts);
 
- result->scan_null = result->scan_default = false;
+ result->scan_null = false;
 
  /*
  * If there are no datums to compare keys with, or if we got an IS NULL
@@ -2469,7 +2467,7 @@ get_matching_range_bounds(PartitionPruneContext *context,
  */
  if (boundinfo->ndatums == 0 || !bms_is_empty(nullkeys))
  {
- result->scan_default = partition_bound_has_default(boundinfo);
+ result->bound_offsets = bms_make_singleton(boundinfo->ndatums);
  return result;
  }
 
@@ -2489,9 +2487,12 @@ get_matching_range_bounds(PartitionPruneContext *context,
  if (partindices[maxoff] < 0)
  maxoff--;
 
- result->scan_default = partition_bound_has_default(boundinfo);
+ /* offset 0 is always corresnponds to invalid partition */
+ result->bound_offsets = bms_make_singleton(0);
+
  Assert(minoff >= 0 && maxoff >= 0);
- result->bound_offsets = bms_add_range(NULL, minoff, maxoff);
+ result->bound_offsets = bms_add_range(result->bound_offsets,
+  minoff, maxoff);
 
  return result;
  }
@@ -2501,7 +2502,7 @@ get_matching_range_bounds(PartitionPruneContext *context,
  * default partition, if any.
  */
  if (nvalues < partnatts)
- result->scan_default = partition_bound_has_default(boundinfo);
+ result->bound_offsets = bms_make_singleton(0);
 
  switch (opstrategy)
  {
@@ -2518,7 +2519,8 @@ get_matching_range_bounds(PartitionPruneContext *context,
  if (nvalues == partnatts)
  {
  /* There can only be zero or one matching partition. */
- result->bound_offsets = bms_make_singleton(off + 1);
+ result->bound_offsets =
+ bms_add_member(result->bound_offsets, off + 1);
  return result;
  }
  else
@@ -2607,7 +2609,8 @@ get_matching_range_bounds(PartitionPruneContext *context,
  }
 
  Assert(minoff >= 0 && maxoff >= 0);
- result->bound_offsets = bms_add_range(NULL, minoff, maxoff);
+ result->bound_offsets = bms_add_range(result->bound_offsets,
+  minoff, maxoff);
  }
  else
  {
@@ -2620,7 +2623,8 @@ get_matching_range_bounds(PartitionPruneContext *context,
  * -1 indicating that all bounds are greater, then we simply
  * end up adding the first bound's offset, that is, 0.
  */
- result->bound_offsets = bms_make_singleton(off + 1);
+ result->bound_offsets =
+ bms_add_member(result->bound_offsets, off + 1);
  }
 
  return result;
@@ -2821,8 +2825,8 @@ get_matching_range_bounds(PartitionPruneContext *context,
 
  Assert(minoff >= 0 && maxoff >= 0);
  if (minoff <= maxoff)
- result->bound_offsets = bms_add_range(NULL, minoff, maxoff);
-
+ result->bound_offsets = bms_add_range(result->bound_offsets,
+  minoff, maxoff);
  return result;
 }
 
@@ -3001,7 +3005,6 @@ perform_pruning_base_step(PartitionPruneContext *context,
 
  result = (PruneStepResult *) palloc(sizeof(PruneStepResult));
  result->bound_offsets = NULL;
- result->scan_default = false;
  result->scan_null = false;
 
  return result;
@@ -3102,17 +3105,9 @@ perform_pruning_combine_step(PartitionPruneContext *context,
  {
  PartitionBoundInfo boundinfo = context->boundinfo;
 
- /*
- * Add all valid offsets into the boundinfo->indexes array.  For range
- * partitioning, boundinfo->indexes contains (boundinfo->ndatums + 1)
- * valid entries.
- */
- if (context->strategy == PARTITION_STRATEGY_RANGE)
- result->bound_offsets = bms_add_range(NULL, 0, boundinfo->ndatums);
- else
- result->bound_offsets = bms_add_range(NULL, 0,
-  boundinfo->ndatums - 1);
- result->scan_default = partition_bound_has_default(boundinfo);
+ /* Add all valid offsets into the boundinfo->indexes array. */
+ result->bound_offsets = bms_add_range(NULL, 0, boundinfo->nindexes - 1);
+
  result->scan_null = partition_bound_accepts_nulls(boundinfo);
  return result;
  }
@@ -3143,8 +3138,6 @@ perform_pruning_combine_step(PartitionPruneContext *context,
  /* Update whether to scan null and default partitions. */
  if (!result->scan_null)
  result->scan_null = step_result->scan_null;
- if (!result->scan_default)
- result->scan_default = step_result->scan_default;
  }
  break;
 
@@ -3166,7 +3159,6 @@ perform_pruning_combine_step(PartitionPruneContext *context,
  result->bound_offsets =
  bms_copy(step_result->bound_offsets);
  result->scan_null = step_result->scan_null;
- result->scan_default = step_result->scan_default;
  firststep = false;
  }
  else
@@ -3179,8 +3171,6 @@ perform_pruning_combine_step(PartitionPruneContext *context,
  /* Update whether to scan null and default partitions. */
  if (result->scan_null)
  result->scan_null = step_result->scan_null;
- if (result->scan_default)
- result->scan_default = step_result->scan_default;
  }
  }
  break;
diff --git a/src/include/partitioning/partbounds.h b/src/include/partitioning/partbounds.h
index b1ae39ad63..18ac8cf0bb 100644
--- a/src/include/partitioning/partbounds.h
+++ b/src/include/partitioning/partbounds.h
@@ -65,6 +65,7 @@ typedef struct PartitionBoundInfoData
  PartitionRangeDatumKind **kind; /* The kind of each range bound datum;
  * NULL for hash and list partitioned
  * tables */
+ int nindexes; /* Length of the indexes following array */
  int   *indexes; /* Partition indexes */
  int null_index; /* Index of the null-accepting partition; -1
  * if there isn't one */
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 Amit-san,

From: Amit Langote [mailto:[hidden email]]
Sent: Monday, March 18, 2019 6:44 PM
 

> Hosoya-san,
>
> On 2019/03/15 15:05, Yuzuko Hosoya wrote:
> > Indeed, it's problematic.  I also did test and I found that this
> > problem was occurred when any partition didn't match WHERE clauses.
> > So following query didn't work correctly.
> >
> > # explain select * from test1_3 where (id > 0 and id < 30);
> >                            QUERY PLAN
> > -----------------------------------------------------------------
> >  Append  (cost=0.00..58.16 rows=12 width=36)
> >    ->  Seq Scan on test1_3_1  (cost=0.00..29.05 rows=6 width=36)
> >          Filter: ((id > 0) AND (id < 30))
> >    ->  Seq Scan on test1_3_2  (cost=0.00..29.05 rows=6 width=36)
> >          Filter: ((id > 0) AND (id < 30))
> > (5 rows)
> >
> > I created a new patch to handle this problem, and confirmed the query
> > you mentioned works as expected
> >
> > # explain select * from test1 where (id > 0 and id < 30) or (id > 220 and id < 230);
> >                                 QUERY PLAN
> > ----------------------------------------------------------------------
> > -----  Append  (cost=0.00..70.93 rows=26 width=36)
> >    ->  Seq Scan on test1_1_1  (cost=0.00..35.40 rows=13 width=36)
> >          Filter: (((id > 0) AND (id < 30)) OR ((id > 220) AND (id < 230)))
> >    ->  Seq Scan on test1_3_1  (cost=0.00..35.40 rows=13 width=36)
> >          Filter: (((id > 0) AND (id < 30)) OR ((id > 220) AND (id <
> > 230)))
> > (5 rows)
> >
> > v2 patch attached.
> > Could you please check it again?
>
> I think the updated patch breaks the promise that get_matching_range_bounds won't set scan_default
> based on individual pruning value comparisons.  How about the attached delta patch that applies on
> top of your earlier v1 patch, which fixes the issue reported by Thibaut?
>
Indeed.  I agreed with your proposal.
Also, I confirmed your patch works correctly.

Best regards,
Yuzuko Hosoya



Reply | Threaded
Open this post in threaded view
|

Re: Problem with default partition pruning

Thibaut Madelaine

Le 19/03/2019 à 08:01, Yuzuko Hosoya a écrit :

> Hi Amit-san,
>
> From: Amit Langote [mailto:[hidden email]]
> Sent: Monday, March 18, 2019 6:44 PM
>  
>> Hosoya-san,
>>
>> On 2019/03/15 15:05, Yuzuko Hosoya wrote:
>>> Indeed, it's problematic.  I also did test and I found that this
>>> problem was occurred when any partition didn't match WHERE clauses.
>>> So following query didn't work correctly.
>>>
>>> # explain select * from test1_3 where (id > 0 and id < 30);
>>>                            QUERY PLAN
>>> -----------------------------------------------------------------
>>>  Append  (cost=0.00..58.16 rows=12 width=36)
>>>    ->  Seq Scan on test1_3_1  (cost=0.00..29.05 rows=6 width=36)
>>>          Filter: ((id > 0) AND (id < 30))
>>>    ->  Seq Scan on test1_3_2  (cost=0.00..29.05 rows=6 width=36)
>>>          Filter: ((id > 0) AND (id < 30))
>>> (5 rows)
>>>
>>> I created a new patch to handle this problem, and confirmed the query
>>> you mentioned works as expected
>>>
>>> # explain select * from test1 where (id > 0 and id < 30) or (id > 220 and id < 230);
>>>                                 QUERY PLAN
>>> ----------------------------------------------------------------------
>>> -----  Append  (cost=0.00..70.93 rows=26 width=36)
>>>    ->  Seq Scan on test1_1_1  (cost=0.00..35.40 rows=13 width=36)
>>>          Filter: (((id > 0) AND (id < 30)) OR ((id > 220) AND (id < 230)))
>>>    ->  Seq Scan on test1_3_1  (cost=0.00..35.40 rows=13 width=36)
>>>          Filter: (((id > 0) AND (id < 30)) OR ((id > 220) AND (id <
>>> 230)))
>>> (5 rows)
>>>
>>> v2 patch attached.
>>> Could you please check it again?
>> I think the updated patch breaks the promise that get_matching_range_bounds won't set scan_default
>> based on individual pruning value comparisons.  How about the attached delta patch that applies on
>> top of your earlier v1 patch, which fixes the issue reported by Thibaut?
>>
> Indeed.  I agreed with your proposal.
> Also, I confirmed your patch works correctly.
>
> Best regards,
> Yuzuko Hosoya

I kept on testing with sub-partitioning.
I found a case, using 2 default partitions, where a default partition is
not pruned:

--------------

create table test2(id int, val text) partition by range (id);
create table test2_20_plus_def partition of test2 default;
create table test2_0_20 partition of test2 for values from (0) to (20)
  partition by range (id);
create table test2_0_10 partition of test2_0_20 for values from (0) to (10);
create table test2_10_20_def partition of test2_0_20 default;

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

--------------

I have the same output using Amit's v1-delta.patch or Hosoya's
v2_default_partition_pruning.patch.



Reply | Threaded
Open this post in threaded view
|

Re: Problem with default partition pruning

Amit Langote-2
Hi Thibaut,

On 2019/03/19 23:58, Thibaut Madelaine wrote:
> I kept on testing with sub-partitioning.
Thanks.

> I found a case, using 2 default partitions, where a default partition is
> not pruned:
>
> --------------
>
> create table test2(id int, val text) partition by range (id);
> create table test2_20_plus_def partition of test2 default;
> create table test2_0_20 partition of test2 for values from (0) to (20)
>   partition by range (id);
> create table test2_0_10 partition of test2_0_20 for values from (0) to (10);
> create table test2_10_20_def partition of test2_0_20 default;
>
> # explain (costs off) select * from test2 where id=5 or id=25;
>                QUERY PLAN               
> -----------------------------------------
>  Append
>    ->  Seq Scan on test2_0_10
>          Filter: ((id = 5) OR (id = 25))
>    ->  Seq Scan on test2_10_20_def
>          Filter: ((id = 5) OR (id = 25))
>    ->  Seq Scan on test2_20_plus_def
>          Filter: ((id = 5) OR (id = 25))
> (7 rows)
>
> --------------
>
> I have the same output using Amit's v1-delta.patch or Hosoya's
> v2_default_partition_pruning.patch.
I think I've figured what may be wrong.

Partition pruning step generation code should ignore any arguments of an
OR clause that won't be true for a sub-partitioned partition, given its
partition constraint.

In this case, id = 25 contradicts test2_0_20's partition constraint (which
is, a IS NOT NULL AND a >= 0 AND a < 20), so the OR clause should really
be simplified to id = 5, ignoring the id = 25 argument.  Note that we
remove id = 25 only for the considerations of pruning and not from the
actual clause that's passed to the final plan, although it wouldn't be a
bad idea to try to do that.

Attached revised delta patch, which includes the fix described above.

Thanks,
Amit

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

Re: Problem with default partition pruning

Thibaut Madelaine

Le 20/03/2019 à 10:06, Amit Langote a écrit :

> Hi Thibaut,
>
> On 2019/03/19 23:58, Thibaut Madelaine wrote:
>> I kept on testing with sub-partitioning.
> Thanks.
>
>> I found a case, using 2 default partitions, where a default partition is
>> not pruned:
>>
>> --------------
>>
>> create table test2(id int, val text) partition by range (id);
>> create table test2_20_plus_def partition of test2 default;
>> create table test2_0_20 partition of test2 for values from (0) to (20)
>>   partition by range (id);
>> create table test2_0_10 partition of test2_0_20 for values from (0) to (10);
>> create table test2_10_20_def partition of test2_0_20 default;
>>
>> # explain (costs off) select * from test2 where id=5 or id=25;
>>                QUERY PLAN               
>> -----------------------------------------
>>  Append
>>    ->  Seq Scan on test2_0_10
>>          Filter: ((id = 5) OR (id = 25))
>>    ->  Seq Scan on test2_10_20_def
>>          Filter: ((id = 5) OR (id = 25))
>>    ->  Seq Scan on test2_20_plus_def
>>          Filter: ((id = 5) OR (id = 25))
>> (7 rows)
>>
>> --------------
>>
>> I have the same output using Amit's v1-delta.patch or Hosoya's
>> v2_default_partition_pruning.patch.
> I think I've figured what may be wrong.
>
> Partition pruning step generation code should ignore any arguments of an
> OR clause that won't be true for a sub-partitioned partition, given its
> partition constraint.
>
> In this case, id = 25 contradicts test2_0_20's partition constraint (which
> is, a IS NOT NULL AND a >= 0 AND a < 20), so the OR clause should really
> be simplified to id = 5, ignoring the id = 25 argument.  Note that we
> remove id = 25 only for the considerations of pruning and not from the
> actual clause that's passed to the final plan, although it wouldn't be a
> bad idea to try to do that.
>
> Attached revised delta patch, which includes the fix described above.
>
> Thanks,
> Amit
Amit, I tested many cases with nested range sub-partitions... and I did
not find any problem with your last patch  :-)

I tried mixing with hash partitions with no problems.

From the patch, there seems to be less checks than before. I cannot
think of a case that can have performance impacts.

Hosoya-san, if you agree with Amit's proposal, do you think you can send
a patch unifying your default_partition_pruning.patch and Amit's second
v1-delta.patch?

Cordialement,

Thibaut





Reply | Threaded
Open this post in threaded view
|

RE: Problem with default partition pruning

Yuzuko Hosoya
Hi,

Thanks a lot for additional tests and the new patch.


> Le 20/03/2019 à 10:06, Amit Langote a écrit :
> > Hi Thibaut,
> >
> > On 2019/03/19 23:58, Thibaut Madelaine wrote:
> >> I kept on testing with sub-partitioning.
> > Thanks.
> >
> >> I found a case, using 2 default partitions, where a default partition
> >> is not pruned:
> >>
> >> --------------
> >>
> >> create table test2(id int, val text) partition by range (id); create
> >> table test2_20_plus_def partition of test2 default; create table
> >> test2_0_20 partition of test2 for values from (0) to (20)
> >>   partition by range (id);
> >> create table test2_0_10 partition of test2_0_20 for values from (0)
> >> to (10); create table test2_10_20_def partition of test2_0_20
> >> default;
> >>
> >> # explain (costs off) select * from test2 where id=5 or id=25;
> >>                QUERY PLAN
> >> -----------------------------------------
> >>  Append
> >>    ->  Seq Scan on test2_0_10
> >>          Filter: ((id = 5) OR (id = 25))
> >>    ->  Seq Scan on test2_10_20_def
> >>          Filter: ((id = 5) OR (id = 25))
> >>    ->  Seq Scan on test2_20_plus_def
> >>          Filter: ((id = 5) OR (id = 25))
> >> (7 rows)
> >>
> >> --------------
> >>
> >> I have the same output using Amit's v1-delta.patch or Hosoya's
> >> v2_default_partition_pruning.patch.
> > I think I've figured what may be wrong.
> >
> > Partition pruning step generation code should ignore any arguments of
> > an OR clause that won't be true for a sub-partitioned partition, given
> > its partition constraint.
> >
> > In this case, id = 25 contradicts test2_0_20's partition constraint
> > (which is, a IS NOT NULL AND a >= 0 AND a < 20), so the OR clause
> > should really be simplified to id = 5, ignoring the id = 25 argument.
> > Note that we remove id = 25 only for the considerations of pruning and
> > not from the actual clause that's passed to the final plan, although
> > it wouldn't be a bad idea to try to do that.
> >
> > Attached revised delta patch, which includes the fix described above.
> >
> > Thanks,
> > Amit
> Amit, I tested many cases with nested range sub-partitions... and I did not find any problem with your
> last patch  :-)
>
> I tried mixing with hash partitions with no problems.
>
> From the patch, there seems to be less checks than before. I cannot think of a case that can have
> performance impacts.
>
> Hosoya-san, if you agree with Amit's proposal, do you think you can send a patch unifying your
> default_partition_pruning.patch and Amit's second v1-delta.patch?
>
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)


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?


Best regards,
Yuzuko Hosoya

v1-delta-2.patch (4K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Problem with default partition pruning

Amit Langote-2
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


12345