Re: Problem with default partition pruning

At Wed, 10 Apr 2019 14:55:48 +0900, Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> wrote in <d2c38e4e-ade4-74de-f686-af37e4a5f1c1(at)lab(dot)ntt(dot)co(dot)jp>
> On 2019/04/10 12:53, Kyotaro HORIGUCHI wrote:
> > At Wed, 10 Apr 2019 11:17:53 +0900, Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> >> Yeah, I think we should move the "if (partconstr)" block to the "if
> >> (is_orclause(clause))" block as I originally proposed in:
> >>
> >> https://www.postgresql.org/message-id/9bb31dfe-b0d0-53f3-3ea6-e64b811424cf%40lab.ntt.co.jp
> >>
> >> It's kind of a hack to get over the limitation that
> >> get_matching_partitions() can't prune default partitions for certain OR
> >> clauses and I think we shouldn't let that hack grow into what seems like
> >> almost duplicating relation_excluded_by_constraints() but without the
> >> constraint_exclusion GUC guard.
> >
> > That leaves an issue of contradicting clauses that is not an arm
> > of OR-expr. Isn't that what Hosoya-san is trying to fix?
>
> Yes, that's right. But as I said, maybe we should try not to duplicate
> the functionality of relation_excluded_by_constraints() in partprune.c.

Currently we classify partition constraint as a constraint. So it
should be handled not in partition pruning, but constraint
exclusion code. That's sound reasonable.

> To summarize, aside from the problem described by the subject of this
> thread (patch for that is v4_default_partition_pruning.patch posted by
> Hosoya-san on 2019/04/04), we have identified couple of other issues:
>
> 1. One that Thibaut reported on 2019/03/04
>
> > I kept on testing with sub-partitioning.Thanks.
> > I found a case, using 2 default partitions, where a default partition is
> > not pruned:
> >
> > --------------
> >
> > create table test2(id int, val text) partition by range (id);
> > create table test2_20_plus_def partition of test2 default;
> > create table test2_0_20 partition of test2 for values from (0) to (20)
> > partition by range (id);
> > create table test2_0_10 partition of test2_0_20 for values from (0) to (10);
> > create table test2_10_20_def partition of test2_0_20 default;
> >
> > # explain (costs off) select * from test2 where id=5 or id=25;
> > QUERY PLAN
> > -----------------------------------------
> > Append
> > -> Seq Scan on test2_0_10
> > Filter: ((id = 5) OR (id = 25))
> > -> Seq Scan on test2_10_20_def
> > Filter: ((id = 5) OR (id = 25))
> > -> Seq Scan on test2_20_plus_def
> > Filter: ((id = 5) OR (id = 25))
> > (7 rows)
>
> For this, we can move the "if (partconstr)" block in the same if
> (is_orclause(clause)) block, as proposed in the v1-delta.patch that I
> proposed on 2019/03/20. Note that that patch restricts the scope of
> applying predicate_refuted_by() only to the problem that's currently
> tricky to solve by partition pruning alone -- pruning default partitions
> for OR clauses like in the above example.

This is a failure of partition pruning, which should be resolved
in the partprune code.

> 2. Hosoya-san reported on 2019/03/22 that a contradictory WHERE clause
> applied to a *partition* doesn't return an empty plan:
>
> > I understood Amit's proposal. But I think the issue Thibaut reported
> > would occur regardless of whether clauses have OR clauses or not as
> > follows.
> >
> > I tested a query which should output "One-Time Filter: false".
> >
> > # explain select * from test2_0_20 where id = 25;
> > QUERY PLAN
> > -----------------------------------------------------------------------
> > Append (cost=0.00..25.91 rows=6 width=36)
> > -> Seq Scan on test2_10_20_def (cost=0.00..25.88 rows=6 width=36)
> > Filter: (id = 25)
>
> So, she proposed to apply predicate_refuted_by to the whole
> baserestrictinfo (at least in the latest patch), which is same as always
> performing constraint exclusion to sub-partitioned partitions. I
> initially thought it might be a good idea, but only later realized that
> now there will be two places doing the same constraint exclusion proof --
> gen_partprune_steps_internal(), and set_rel_size() calling
> relation_excluded_by_constraints(). The latter depends on
> constraint_exclusion GUC whose default being 'partition' would mean we'd
> not get an empty plan with it. Even if you turn it to 'on', a bug of
> get_relation_constraints() will prevent the partition constraint from
> being loaded and performing constraint exclusion with it; I reported it in
> [1].

Hmm. One perplexing thing here is the fact that partition
constraint is not a table constraint but a partitioning
classification in users' view.

> I think that we may be better off solving the latter problem as follows:
>
> 1. Modify relation_excluded_by_constraints() to *always* try to exclude
> "baserel" partitions using their partition constraint (disregarding
> constraint_exclusion = off/partition).
>
> 2. Modify prune_append_rel_partitions(), which runs much earlier these
> days compared to set_rel_size(), to call relation_excluded_by_constraint()
> modified as described in step 1. If it returns true, don't perform
> partition pruning, set the appendrel parent as dummy right away. It's not
> done today, but appendrel parent can also be set to dummy based on the
> result of pruning, that is, when get_matching_partitions() returns no
> matching partitions.
>
> 3. Modify set_base_rel_sizes() to ignore already-dummy rels, so that we
> don't perform constraint exclusion again via set_rel_size().
>
> I have to say this other problem involving partition constraints is quite
> complicated (aforementioned past bug messing up the situation further), so
> it would be nice if a committer can review and commit the solutions for
> the originally reported pruning issues.

Tend to agree. Anyway the other problem needs to involve parent
of the specified relation, which is not a thing that doesn't seem
to be able to be done the ordinary way.

> Thanks,
> Amit
>
> [1]
> https://www.postgresql.org/message-id/9813f079-f16b-61c8-9ab7-4363cab28d80@lab.ntt.co.jp

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center