Thread: Yet another issue with step generation in partition pruning

Commit 13838740f fixed some issues with step generation in partition
pruning, but as I mentioned in [1], I noticed that there is yet
another issue: get_steps_using_prefix() assumes that clauses in the
passed-in prefix list are sorted in ascending order of their partition
key numbers, but the caller (i.e., gen_prune_steps_from_opexps())
doesn’t ensure that in the case of range partitioning, leading to an
assertion failure.  Here is an example causing such a failure, which
would happen with/without that commit:

create table rp_prefix_test2 (a int, b int, c int) partition by range (a, b, c);
create table rp_prefix_test2_p1 partition of rp_prefix_test2 for
values from (1, 1, 0) to (1, 1, 10);
create table rp_prefix_test2_p2 partition of rp_prefix_test2 for
values from (2, 2, 0) to (2, 2, 10);
select * from rp_prefix_test2 where a <= 1 and b <= 1 and b = 1 and c <= 0;

I don't think we write queries like this, but for this query, the
caller would create the prefix list for the last partition key “c”
{b=1, a<=1, b<=1} (the clauses are not sorted properly!), then calling
get_steps_using_prefix(), which leads to an assertion failure.
Attached is a patch for fixing this issue.

Best regards,
Etsuro Fujita

[1] https://www.postgresql.org/message-id/CAPmGK15%3Dc8Q-Ac3ogzZp_d6VsfRYSL2tD8zLwy_WYdrMXQhiCQ%40mail.gmail.com

Fujita-san,

Thanks a lot for your time on fixing these multi-column range
partition pruning issues.  I'm sorry that I failed to notice the
previous two reports on -bugs for which you committed a fix last week.

On Tue, Aug 4, 2020 at 9:46 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> Commit 13838740f fixed some issues with step generation in partition
> pruning, but as I mentioned in [1], I noticed that there is yet
> another issue: get_steps_using_prefix() assumes that clauses in the
> passed-in prefix list are sorted in ascending order of their partition
> key numbers, but the caller (i.e., gen_prune_steps_from_opexps())
> doesn’t ensure that in the case of range partitioning, leading to an
> assertion failure.  Here is an example causing such a failure, which
> would happen with/without that commit:
>
> create table rp_prefix_test2 (a int, b int, c int) partition by range (a, b, c);
> create table rp_prefix_test2_p1 partition of rp_prefix_test2 for
> values from (1, 1, 0) to (1, 1, 10);
> create table rp_prefix_test2_p2 partition of rp_prefix_test2 for
> values from (2, 2, 0) to (2, 2, 10);
> select * from rp_prefix_test2 where a <= 1 and b <= 1 and b = 1 and c <= 0;
>
> I don't think we write queries like this, but for this query, the
> caller would create the prefix list for the last partition key “c”
> {b=1, a<=1, b<=1} (the clauses are not sorted properly!), then calling
> get_steps_using_prefix(), which leads to an assertion failure.

That analysis is spot on.

> Attached is a patch for fixing this issue.

I have looked at the patch and played around with it using the
regression tests you've added recently. I was not able to find any
results that looked surprising.

Thanks again.

--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com

Amit-san,

On Wed, Aug 5, 2020 at 5:13 PM Amit Langote <amitlangote09@gmail.com> wrote:
> Thanks a lot for your time on fixing these multi-column range
> partition pruning issues.  I'm sorry that I failed to notice the
> previous two reports on -bugs for which you committed a fix last week.

No problem.

> On Tue, Aug 4, 2020 at 9:46 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> > Attached is a patch for fixing this issue.
>
> I have looked at the patch and played around with it using the
> regression tests you've added recently. I was not able to find any
> results that looked surprising.

That's good to hear!  Thanks for reviewing!  Will push the patch tomorrow.

Best regards,
Etsuro Fujita

On Thu, Aug 6, 2020 at 12:20 AM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> Will push the patch tomorrow.

Done.  (I didn't have time for this, because I was terribly busy with
other stuff.)

Best regards,
Etsuro Fujita

On Fri, Aug 7, 2020 at 2:55 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> On Thu, Aug 6, 2020 at 12:20 AM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> > Will push the patch tomorrow.
>
> Done.  (I didn't have time for this, because I was terribly busy with
> other stuff.)

I mean I didn't have time for this *yesterday*.

Best regards,
Etsuro Fujita