Re: partition pruning doesn't work with IS NULL clause in multikey range partition case

On 2018/07/12 14:32, Amit Langote wrote:
> Thanks Ashutosh for reporting and Dilip for the analysis and the patch.
>
> On 2018/07/11 21:39, Dilip Kumar wrote:
>> On Wed, Jul 11, 2018 at 5:36 PM, amul sul <sulamul(at)gmail(dot)com> wrote:
>>> On Wed, Jul 11, 2018 at 5:10 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
>>
>>>>
>>> I am not sure that I have understand the following comments
>>> 11 + * Generate one prune step for the information derived from IS NULL,
>>> 12 + * if any. To prune hash partitions, we must have found IS NULL
>>> 13 + * clauses for all partition keys.
>>> 14 */
>>>
>>> I am not sure that I have understood this -- no such restriction
>>> required to prune the hash partitions, if I am not missing anything.
>>
>> Maybe it's not very clear but this is the original comments I have
>> retained. Just moved it out of the (!generate_opsteps) condition.
>>
>> Just the explain this comment consider below example,
>>
>> create table hp (a int, b text) partition by hash (a int, b text);
>> 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);
>>
>> postgres=# insert into hp values (1, null);
>> INSERT 0 1
>> postgres=# insert into hp values (2, null);
>> INSERT 0 1
>> postgres=# select tableoid::regclass, * from hp;
>> tableoid | a | b
>> ----------+---+---
>> hp1 | 1 |
>> hp2 | 2 |
>> (2 rows)
>>
>> Now, if we query based on "b is null" then we can not decide which
>> partition should be pruned whereas in case
>> of other schemes, it will go to default partition so we can prune all
>> other partitions.
>
> That's right. By generating a pruning step with only nullkeys set, we are
> effectively discarding OpExprs that may have been found for some partition
> keys. That's fine for list/range partitioning, because nulls can only be
> found in a designated partition, so it's okay to prune all other
> partitions and for that it's enough to generate the pruning step like
> that. For hash partitioning, nulls could be contained in any partition so
> it's not okay to discard OpExpr's like that. We can generate pruning
> steps with combination of null and non-null keys in the hash partitioning
> case if there are any OpExprs.
>
> I think your fix is correct. I slightly modified it along with updating
> nearby comments and added regression tests.

I updated regression tests to reduce lines. There is no point in
repeating tests like v2 patch did.

Thanks,
Amit