Reply-To: jesper.pedersen@redhat.com
Subject: Re: Index Skip Scan
To: Dmitry Dolgov <9erthalion6@gmail.com>
Cc: a.kuzmenkov@postgrespro.ru, pg@bowt.ie,
 Thomas Munro <thomas.munro@enterprisedb.com>,
 Bhushan Uparkar <bhushan.uparkar@gmail.com>,
 PostgreSQL-development <pgsql-hackers@postgresql.org>,
 Alexander Korotkov <a.korotkov@postgrespro.ru>
References: <707b6f68-16fa-7aa7-96e5-eeb4865e6a30@redhat.com>
 <CAPpHfducUiDw7hVT=Vui8rxTmawCkVc0H--v=jdbztMUGhzuRg@mail.gmail.com>
 <89F84E17-D684-4717-B66F-0F05CB00168E@gmail.com>
 <CAEepm=1ikXHVgSNruQ1HydDZm3OLnLNb5qT2c8S=Mh-VsFQXcQ@mail.gmail.com>
 <CAH2-WznxAN940e=2D8Auz06d=_ZK6F5LVH0HUMbCXGAa=Tirpw@mail.gmail.com>
 <CAEepm=0S1JUUwKCK4jhmtTWm9VUTsCe3+b5tSvddpMpkzas7Hw@mail.gmail.com>
 <CAH2-Wzkk0+vSAiaRSkX84KR0SofKkY1yGHEcLoRBXT-JjM37bw@mail.gmail.com>
 <CA+q6zcV-wgnSMu-YOEmyXVckncQyriiKtUZXm=Gx2s+FV=5MDw@mail.gmail.com>
 <c87d41ae-7c18-a3a1-96f2-f366a43412dc@redhat.com>
 <522092c7-3cb7-a1ec-410b-dfa97ae722f3@postgrespro.ru>
 <11b7c9d1-ae00-4385-97b2-fcd45f0a1a71@redhat.com>
 <3cfd317e-2a8b-8681-0833-8f2ead3af4d4@postgrespro.ru>
 <CA+q6zcUW=GXiw24iiLee3ZhA4JY4wc1hmGE8+20v-W47sOQjNA@mail.gmail.com>
From: Jesper Pedersen <jesper.pedersen@redhat.com>
Organization: Red Hat
Message-ID: <cc30955d-2ee9-4c36-e5d7-fc5b552fbbb6@redhat.com>
Date: Thu, 27 Sep 2018 09:59:50 -0400
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.0
MIME-Version: 1.0
In-Reply-To: 
 <CA+q6zcUW=GXiw24iiLee3ZhA4JY4wc1hmGE8+20v-W47sOQjNA@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Precedence: bulk

Hi Dmitry,

On 9/15/18 3:52 PM, Dmitry Dolgov wrote:
>> On Thu, 13 Sep 2018 at 21:36, Alexander Kuzmenkov <a.kuzmenkov@postgrespro.ru> wrote:
>> El 13/09/18 a las 18:39, Jesper Pedersen escribió:
>>> I think we can improve this,
>>> and the skip scan can be strictly faster than index scan regardless of
>>> the data. As a first approximation, imagine that we somehow skipped
>>> equal tuples inside _bt_next instead of sending them to the parent
>>> Unique node. This would already be marginally faster than Unique + Index
>>> scan. A more practical implementation would be to remember our position
>>> in tree (that is, BTStack returned by _bt_search) and use it to skip
>>> pages in bulk. This looks straightforward to implement for a tree that
>>> does not change, but I'm not sure how to make it work with concurrent
>>> modifications. Still, this looks a worthwhile direction to me, because
>>> if we have a strictly faster skip scan, we can just use it always and
>>> not worry about our unreliable statistics. What do you think?
>>>
>>
>> This is something to look at -- maybe there is a way to use
>> btpo_next/btpo_prev instead/too in order to speed things up. Atm we just
>> have the scan key in BTScanOpaqueData. I'll take a look after my
>> upcoming vacation; feel free to contribute those changes in the meantime
>> of course.
> 
> But having this logic inside _bt_next means that it will make a non-skip index
> only scan a bit slower, am I right?

Correct.

> Probably it would be easier and more
> straightforward to go with the idea of dynamic fallback then. The first naive
> implementation that I came up with is to wrap an index scan node into a unique,
> and remember estimated number of groups into IndexOnlyScanState, so that we can
> check if we performed much more skips than expected. With this approach index
> skip scan will work a bit slower than in the original patch in case if
> ndistinct is correct (because a unique node will recheck rows we returned), and
> fallback to unique + index only scan in case if planner has underestimated
> ndistinct.
> 

I think we need a comment on this in the patch, as 10 * 
node->ioss_PlanRows looks a bit random.

Thanks for your contribution !

Best regards,
  Jesper