Re: Problem with default partition pruning - Mailing list pgsql-hackers
From | Kyotaro Horiguchi |
---|---|
Subject | Re: Problem with default partition pruning |
Date | |
Msg-id | 20190809.120920.64991939.horikyota.ntt@gmail.com Whole thread Raw |
In response to | Re: Problem with default partition pruning (Amit Langote <amitlangote09@gmail.com>) |
Responses |
Re: Problem with default partition pruning
|
List | pgsql-hackers |
Sorry for being late. I found it is committed before I caught up this thread again.. At Thu, 8 Aug 2019 14:50:54 +0900, Amit Langote <amitlangote09@gmail.com> wrote in <CA+HiwqEmJizJ4DmuPWCL-WrHGO-hFVd08TS7HnCkSF4CyZr8tg@mail.gmail.com> > Hi Alvaro, > > On Thu, Aug 8, 2019 at 5:27 AM Alvaro Herrera <alvherre@2ndquadrant.com> wrote: > > On 2019-Aug-07, Simon Riggs wrote: > > > I saw your recent commit and it scares me in various places, noted below. > > > > > > "Commit: Apply constraint exclusion more generally in partitioning" > > > > > > "This applies particularly to the default partition..." > > > > > > My understanding of the thread was the complaint was about removing the > > > default partition. I would prefer to see code executed just for that case, > > > so that people who do not define a default partition are unaffected. > > > > Well, as the commit message noted, it applies to other cases also, not > > just the default partition. The default partition just happens to be > > the most visible case. > > Just to be clear, I don't think there was any patch posted on this > thread that was to address *non-default* partitions failing to be > pruned by "partition pruning". If that had been the problem, we'd be > fixing the bugs of the partition pruning code, not apply constraint > exclusion more generally to paper over such bugs. I may be misreading > what you wrote here though. > > The way I interpret the "generally" in the "apply constraint exclusion > more generally" is thus: we can't prune the default partition without > the constraint exclusion clutches for evidently a broader sets of > clauses than the previous design assumed. The previous design assumed > that only OR clauses whose arguments contradicted the parent's > partition constraint are problematic, but evidently any clause set > that contradicts the partition constraint is problematic. Again, the > problem is that it's impossible to prune the "default" partition with > such clauses, not the *non-default* ones -- values extracted from > contradictory clauses would not match any of the bounds so all > non-default partitions would be pruned that way. > > By the way, looking closer at the patch committed today, I realized I > had misunderstood what you proposed as the *4th* possible place to > move the constraint exclusion check to. I had misread the proposal > and thought you meant to move it outside the outermost loop of > gen_partprune_steps_internal(), but that's not where the check is now. > I think it's better to call predicate_refuted_by() only once by > passing the whole list of clauses instead of for each clause > separately. The result would be the same but the former would be more > efficient, because it avoids repeatedly paying the cost of setting up > predtest.c data structures when predicate_refuted_by() is called. > Sorry that I'm only saying this now. +1 as I mentioned in [1]. > Also it wouldn't be incorrect to do the check only if the parent has a > default partition. That will also address the Simon's concern this > might slow down the cases where this effort is useless. > > I've attached a patch that does that. When working on it, I realized > that the way RelOptInfo.partition_qual is processed is a bit > duplicative, so I created a separate patch to make that a bit more > consistent. 0001 seems reasonable. By the way, the patch doesn't touch get_relation_constraints(), but I suppose it can use the modified partition constraint qual already stored in rel->partition_qual in set_relation_partition_info. And we could move constifying to set_rlation_partition_info? Also, I'd like to see comments that the partition_quals is already varnode-fixed. And 0002, yeah, just +1 from me. [1] https://www.postgresql.org/message-id/20190409.173725.31175835.horiguchi.kyotaro@lab.ntt.co.jp regards. -- Kyotaro Horiguchi NTT Open Source Software Center
pgsql-hackers by date: