From: Sergei Glukhov <s.glukhov@postgrespro.ru>
To: David Rowley <dgrowleyml@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: Problem, partition pruning for prepared statement with IS NULL clause.
Date: Tue, 10 Oct 2023 12:31:32 +0400
Message-ID: <2f09ce72-315e-2a33-589a-8519ada8df61@postgrespro.ru> (raw)
In-Reply-To: <CAApHDvrc8BjkN8U3NUVzvat3EtC7bzLwFdUw_OFf3Sv-QCp-Vg@mail.gmail.com>
References: <d05b26fa-af54-27e1-f693-6c31590802fa@postgrespro.ru>
<CAApHDvrc8BjkN8U3NUVzvat3EtC7bzLwFdUw_OFf3Sv-QCp-Vg@mail.gmail.com>
Hi David,
On 10/9/23 03:26, David Rowley wrote:
> On Sat, 7 Oct 2023 at 03:11, Sergei Glukhov <s.glukhov@postgrespro.ru> wrote:
>> I noticed that combination of prepared statement with generic plan and
>> 'IS NULL' clause could lead partition pruning to crash.
>> Test case:
>> ------
>> set plan_cache_mode to force_generic_plan;
>> prepare stmt AS select * from hp where a is null and b = $1;
>> explain execute stmt('xxx');
> Thanks for the detailed report and proposed patch.
>
> I think your proposed fix isn't quite correct. I think the problem
> lies in InitPartitionPruneContext() where we assume that the list
> positions of step->exprs are in sync with the keyno. If you look at
> perform_pruning_base_step() the code there makes a special effort to
> skip over any keyno when a bit is set in opstep->nullkeys.
>
> It seems that your patch is adjusting the keyno that's given to the
> PruneCxtStateIdx() and it looks like (for your test case) it'll end up
> passing keyno==0 when it should be passing keyno==1. keyno is the
> index of the partition key, so you can't pass 0 when it's for key
> index 1.
>
> I wonder if it's worth expanding the tests further to cover more of
> the pruning cases to cover run-time pruning too.
Thanks for the explanation. I thought by some reason that 'exprstates '
array doesn't
contain elements related to 'IS NULL' clause. Now I see that they are
there and
just empty and untouched.
I verified the patch and it fixes the problem.
Regarding test case,
besides the current test case and the test for dynamic partition
pruning, say,
select a, (select b from hp where a is null and b = a.b) AS b from hp a
where a = 1 and b = 'xxx';
I would like to suggest to slightly refactor 'Test Partition pruning for
HASH partitioning' test
from 'partition_prune.sql' and add one more key field. The reason is
that two-element
key is not enough for thorough testing since it tests mostly corner
cases. Let me know
if it's worth doing.
Example:
------
create table hp (a int, b text, c int, d int)
partition by hash (a part_test_int4_ops, b part_test_text_ops, c
part_test_int4_ops);
create table hp0 partition of hp for values with (modulus 4, remainder 0);
create table hp3 partition of hp for values with (modulus 4, remainder 3);
create table hp1 partition of hp for values with (modulus 4, remainder 1);
create table hp2 partition of hp for values with (modulus 4, remainder 2);
insert into hp values (null, null, null, 0);
insert into hp values (1, null, 1, 1);
insert into hp values (1, 'xxx', 1, 2);
insert into hp values (null, 'xxx', null, 3);
insert into hp values (2, 'xxx', 2, 4);
insert into hp values (1, 'abcde', 1, 5);
------
Another crash in the different place even with the fix:
------
explain select * from hp where a = 1 and b is null and c = 1;
------
Regards,
Gluh
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: s.glukhov@postgrespro.ru, dgrowleyml@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: Problem, partition pruning for prepared statement with IS NULL clause.
In-Reply-To: <2f09ce72-315e-2a33-589a-8519ada8df61@postgrespro.ru>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox