> 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