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

From: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
To: Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>
Cc: Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, amul sul <sulamul(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>
Subject: Re: partition pruning doesn't work with IS NULL clause in multikey range partition case
Date: 2018-07-13 21:11:31
Message-ID: 20180713211131.jycspr6zjoat73v5@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2018-Jul-13, Ashutosh Bapat wrote:

> On Fri, Jul 13, 2018 at 1:15 PM, Amit Langote
> <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)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

Attachment Content-Type Size
generate_prunestep_for_isnull-v5.patch text/plain 6.4 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Heikki Linnakangas 2018-07-13 21:17:30 Re: [WIP PATCH] Index scan offset optimisation using visibility map
Previous Message Andres Freund 2018-07-13 21:08:45 Re: [PATCH] LLVM tuple deforming improvements