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

On 2018-Jul-13, Ashutosh Bapat wrote:

> On Fri, Jul 13, 2018 at 1:15 PM, Amit Langote
> <Langote_Amit_f8@lab.ntt.co.jp> wrote:
> >>
> >> I don't think this is true. When equality conditions and IS NULL clauses cover
> >> all partition keys of a hash partitioned table and do not have contradictory
> >> clauses, we should be able to find the partition which will remain unpruned.
> >
> > I was trying to say the same thing, but maybe the comment doesn't like it.
> >  How about this:
> >
> > +     * For hash partitioning, if we found IS NULL clauses for some keys and
> > +     * OpExpr's for others, gen_prune_steps_from_opexps() will generate the
> > +     * necessary pruning steps if all partition keys are taken care of.
> > +     * If we found only IS NULL clauses, then we can generate the pruning
> > +     * step here but only if all partition keys are covered.
> >
> 
> It's still confusing a bit. I think "taken care of" doesn't convey the
> same meaning as "covered". I don't think the second sentence is
> necessary, it talks about one special case of the first sentence. But
> this is better than the prior version.

Hmm, let me reword this comment completely.  How about the attached?

I also changed the order of the tests, putting the 'generate_opsteps'
one in the middle instead of at the end (so the not-null one doesn't
test that boolean again).  AFAICS it's logically the same, and it makes
more sense to me that way.

I also changed the tests so that they apply to the existing mc2p table,
which is identical to the one Amit was adding.

> > How about if we explicitly spell out the strategies like this:
> >
> > +    if (!bms_is_empty(nullkeys) &&
> > +        (part_scheme->strategy == PARTITION_STRATEGY_LIST ||
> > +         part_scheme->strategy == PARTITION_STRATEGY_RANGE ||
> > +         (part_scheme->strategy == PARTITION_STRATEGY_HASH &&
> > +          bms_num_members(nullkeys) == part_scheme->partnatts)))
> 
> I still do not understand why do we need (part_scheme->strategy ==
> PARTITION_STRATEGY_HASH && bms_num_members(nullkeys) ==
> part_scheme->partnatts) there. I thought that this will be handled by
> code, which deals with null keys and op_exprs together, somewhere
> else.

I'm not sure I understand this question.  This strategy applies to hash
partitioning only if we have null tests for all keys, so there are no
op_exprs.  If there are any, there's no point in checking them.

Now this case is a fun one:
select * from mc2p where a in (1, 2) and b is null;
Note that no partitions are pruned in that case; yet if you do this:
select * from mc2p where a in (1) and b is null;
then it does prune.  I think the reason for this is that the
PARTCLAUSE_MATCH_STEPS is ... pretty special.

> > Actually, there is only one case where the pruning step generated by that
> > block of code is useful -- to prune a list partition that accepts only
> > nulls.  List partitioning only allows one partition, key so this works,
> > but let's say only accidentally.  I modified the condition as follows:
> >
> > +    else if (!generate_opsteps && !bms_is_empty(notnullkeys) &&
> > +             bms_num_members(notnullkeys) == part_scheme->partnatts)
> >
> 
> Hmm. Ok. May be it's better to explicitly say that it's only useful in
> case of list partitioned table.

Yeah, maybe.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services