Re: Problem with default partition pruning

On 2019/04/10 12:53, Kyotaro HORIGUCHI wrote:
> At Wed, 10 Apr 2019 11:17:53 +0900, Amit Langote <Langote_Amit_f8@lab.ntt.co.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.

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.

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].

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.

Thanks,
Amit

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