Date: Sat, 8 Feb 2020 14:11:59 +0100
From: Tomas Vondra <tomas.vondra@2ndquadrant.com>
To: Dmitry Dolgov <9erthalion6@gmail.com>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>, florisvannee@optiver.com,
	pg@bowt.ie, jesper.pedersen@redhat.com,
	david.rowley@2ndquadrant.com, alvherre@2ndquadrant.com,
	thomas.munro@gmail.com, jtc331@gmail.com, rafia.pghackers@gmail.com,
	jeff.janes@gmail.com, bhushan.uparkar@gmail.com,
	pgsql-hackers@postgresql.org, a.korotkov@postgrespro.ru
Subject: Re: Index Skip Scan
Message-ID: <20200208131159.5sjcbcjnwdj4q7fa@development>
References: <20200205163730.eqvoxh6eb3do52r5@localhost>
 <20200206.102450.1514793516830965023.horikyota.ntt@gmail.com>
 <20200206105707.fuidmdsi6oapieoz@localhost>
 <20200206.212220.642002310227620136.horikyota.ntt@gmail.com>
 <20200207162543.zlj2tvcwy7jku6vu@localhost>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Disposition: inline
In-Reply-To: <20200207162543.zlj2tvcwy7jku6vu@localhost>
Precedence: bulk

On Fri, Feb 07, 2020 at 05:25:43PM +0100, Dmitry Dolgov wrote:
>> On Thu, Feb 06, 2020 at 09:22:20PM +0900, Kyotaro Horiguchi wrote:
>> At Thu, 6 Feb 2020 11:57:07 +0100, Dmitry Dolgov <9erthalion6@gmail.com> wrote in
>> > > On Thu, Feb 06, 2020 at 10:24:50AM +0900, Kyotaro Horiguchi wrote:
>> > > At Wed, 5 Feb 2020 17:37:30 +0100, Dmitry Dolgov <9erthalion6@gmail.com> wrote in
>> > > We could add an additional parameter "in_cursor" to
>> > > ExecSupportBackwardScan and let skip scan return false if in_cursor is
>> > > true, but I'm not sure it's acceptable.
>> >
>> > I also was thinking about whether it's possible to use
>> > ExecSupportBackwardScan here, but skip scan is just a mode of an
>> > index/indexonly scan. Which means that ExecSupportBackwardScan also need
>> > to know somehow if this mode is being used, and then, since this
>> > function is called after it's already decided to use skip scan in the
>> > resulting plan, somehow correct the plan (exclude skipping and try to
>> > find next best path?) - do I understand your suggestion correct?
>>
>> I didn't thought so hardly, but a bit of confirmation told me that
>> IndexSupportsBackwardScan returns fixed flag for AM.  It seems that
>> things are not that simple.
>
>Yes, I've mentioned that already in one of the previous emails :) The
>simplest way I see to achieve what we want is to do something like in
>attached modified version with a new hasDeclaredCursor field. It's not a
>final version though, but posted just for discussion, so feel free to
>suggest any improvements or alternatives.

IMO the proper fix for this case (moving forward, reading backwards) is
simply making it work by properly checking deleted tuples etc. Not sure
why that would be so much complex (haven't tried implementing it)?

I think making this depend on things like declared cursor etc. is going
to be tricky, may easily be more complex than checking deleted tuples,
and the behavior may be quite surprising.

regards

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services