On Wed, Apr 8, 2020 at 3:41 AM Dmitry Dolgov <9erthali...@gmail.com> wrote:
> > On Mon, Apr 06, 2020 at 06:31:08PM +0000, Floris Van Nee wrote: > > > > > Hm, I wasn't aware about this one, thanks for bringing this up. Btw, > Floris, I > > > would appreciate if in the future you can make it more visible that > changes you > > > suggest contain some fixes. E.g. it wasn't clear for me from your > previous email > > > that that's the case, and it doesn't make sense to pull into different > direction > > > when we're trying to achieve the same goal :) > > > > I wasn't aware that this particular case could be triggered before I saw > Dilip's email, otherwise I'd have mentioned it here of course. It's just > that because my patch handles filter conditions in general, it works for > this case too. > > Oh, then fortunately I've got a wrong impression, sorry and thanks for > clarification :) > > > > > > In the patch I posted a week ago these cases are all handled > > > > > correctly, as it introduces this extra logic in the Executor. > > > > > > > > Okay, So I think we can merge those fixes in Dmitry's patch set. > > > > > > I'll definitely take a look at suggested changes in filtering part. > > > > It may be possible to just merge the filtering part into your patch, but > I'm not entirely sure. Basically you have to pull the information about > skipping one level up, out of the node, into the generic IndexNext code. > > I was actually thinking more about just preventing skip scan in this > situation, which is if I'm not mistaken could be solved by inspecting > qual conditions to figure out if they're covered in the index - > something like in attachments (this implementation is actually too > restrictive in this sense and will not allow e.g. expressions, that's > why I haven't bumped patch set version for it - soon I'll post an > extended version). > > Other than that to summarize current open points for future readers > (this thread somehow became quite big): > > * Making UniqueKeys usage more generic to allow using skip scan for more > use cases (hopefully it was covered by the v33, but I still need a > confirmation from David, like blinking twice or something). > > * Suspicious performance difference between different type of workload, > mentioned by Tomas (unfortunately I had no chance yet to investigate). > > * Thinking about supporting conditions, that are not covered by the index, > to make skipping more flexible (one of the potential next steps in the > future, as suggested by Floris). > Looks this is the latest patch, which commit it is based on? Thanks -- Best Regards Andy Fan
