At Wed, 10 Apr 2019 14:55:48 +0900, Amit Langote <langote_amit...@lab.ntt.co.jp> wrote in <d2c38e4e-ade4-74de-f686-af37e4a5f...@lab.ntt.co.jp> > On 2019/04/10 12:53, Kyotaro HORIGUCHI wrote: > > At Wed, 10 Apr 2019 11:17:53 +0900, Amit Langote > > <langote_amit...@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.
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-4363cab28...@lab.ntt.co.jp regards. -- Kyotaro Horiguchi NTT Open Source Software Center