Hi Alvaro, On Thu, Aug 8, 2019 at 5:27 AM Alvaro Herrera <alvhe...@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. 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. Thanks, Amit
0001-Remove-duplicative-code-processing-RelOptInfo.partit.patch
Description: Binary data
0002-Improve-constraint-exclusion-usage-in-partprune.c-a-.patch
Description: Binary data
