On 31 October 2018 at 12:24, Julien Rouhaud <rjuju...@gmail.com> wrote: > On Mon, Oct 29, 2018 at 1:44 AM David Rowley > <david.row...@2ndquadrant.com> wrote: >> >> On 28 October 2018 at 03:49, Julien Rouhaud <rjuju...@gmail.com> wrote: >> > I just had a look at your patch. I see that you implemented only a >> > subset of the possible optimizations (only the case for range >> > partitionoing without subpartitions). This has been previously >> > discussed, but we should be able to do similar optimization for list >> > partitioning if there's no interleaved values, and also for some cases >> > of multi-level partitioning. >> >> I had thought about these cases but originally had thought they would >> be more complex to implement than I could justify. On review, I've >> found some pretty cheap ways to handle both sub-partitions and for >> LIST partitioned tables. Currently, with LIST partitioned tables I've >> coded it to only allow the optimisation if there's no DEFAULT >> partition and all partitions are defined with exactly 1 Datum. This >> guarantees that there are no interleaved values, but it'll just fail >> to optimise cases like FOR VALUES IN(1,2) + FOR VALUES In(3,4). The >> reason that I didn't go to the trouble of the additional checks was >> that I don't really want to add any per-partition overhead to this. > > I see, but the overhead you mention is because you're doing that check > during the planning in build_partition_pathkeys(). As advised by > Robert quite some time ago > (https://www.postgresql.org/message-id/CA+TgmobOWgT1=zyjx-q=7s8akxnodix46qg0_-yx7k369p6...@mail.gmail.com), > we can store that information when the PartitionDesc is built, so > that would it wouldn't be problematic. Since checking for overlapping > values is straightforward with the BoundInfoData infrastructure, it'd > be a pity to miss this optimization in such cases, which I believe > would not be rare.
Thanks for looking at this again. I retrospectively read that thread after Amit mentioned about your patch. I just disagree with Robert about caching this flag. The reason is, if the flag is false due to some problematic partitions, if we go and prune those, then we needlessly fail to optimise that case. I propose we come back and do the remaining optimisations with interleaved LIST partitions and partitioned tables with DEFAULT partitions later, once we have a new "live_parts" field in RelOptInfo. That way we can just check the live parts to ensure they're compatible with the optimization. If we get what's done already in then we're already a bit step forward. [...] >> v2 of the patch is attached. I've not had time yet to give it a >> throughout post write review, but on first look it seems okay. > > > I've registered as a reviewer. I still didn't have a deep look at > the patch yet, but thanks a lot for working on it! Thanks for signing up to review. I need to send another revision of the patch to add a missing call to truncate_useless_pathkeys(). Will try to do that today. -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
