Date: Mon, 11 May 2020 13:26:55 +0200
From: Dmitry Dolgov <9erthalion6@gmail.com>
To: Dilip Kumar <dilipbalaut@gmail.com>
Cc: Floris Van Nee <florisvannee@optiver.com>,
	David Rowley <dgrowleyml@gmail.com>,
	Kyotaro Horiguchi <horikyota.ntt@gmail.com>,
	Tomas Vondra <tomas.vondra@2ndquadrant.com>,
	Peter Geoghegan <pg@bowt.ie>,
	Jesper Pedersen <jesper.pedersen@redhat.com>,
	Alvaro Herrera <alvherre@2ndquadrant.com>,
	Thomas Munro <thomas.munro@gmail.com>,
	James Coleman <jtc331@gmail.com>,
	Rafia Sabih <rafia.pghackers@gmail.com>,
	Jeff Janes <jeff.janes@gmail.com>,
	"bhushan.uparkar@gmail.com" <bhushan.uparkar@gmail.com>,
	PostgreSQL-development <pgsql-hackers@postgresql.org>,
	Alexander Korotkov <a.korotkov@postgrespro.ru>
Subject: Re: Index Skip Scan
Message-ID: <20200511112655.ijz2sjltzdpif3cn@localhost>
References: <20200405161029.fpqqll7xbgfqy5na@localhost>
 <CAFiTN-sRnYp7MkctzYakyjWRPQQHFzEAwKV8vwf+sUgA+8JtQw@mail.gmail.com>
 <80ef5f0744f542b18e508fb589a6425a@opammb0562.comp.optiver.com>
 <CAFiTN-tp8S3BTwYXT33tnY=53fNCsS4ZZUjbEFtcJym1acoLeA@mail.gmail.com>
 <20200406163941.rh3okji2rhbnptyt@localhost>
 <8abe43ac82714c2f9044fb8bac0ebfd0@opammb0562.comp.optiver.com>
 <20200407194220.edf4nbi7l4cpxk4v@localhost>
 <CAFiTN-sRRGZ_6za6qhN+e98wCfcLrn4bE35RqxycteKbDutorA@mail.gmail.com>
 <20200510174935.teqyazdw6jj6vh47@localhost>
 <CAFiTN-t1dR7zq=PN8HiiouE3MfBrLrVfq3NXa37=dWsf_MtgAw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: 
 <CAFiTN-t1dR7zq=PN8HiiouE3MfBrLrVfq3NXa37=dWsf_MtgAw@mail.gmail.com>
Precedence: bulk

> On Mon, May 11, 2020 at 04:04:00PM +0530, Dilip Kumar wrote:
>
> > > +static inline bool
> > > +_bt_scankey_within_page(IndexScanDesc scan, BTScanInsert key,
> > > + Buffer buf, ScanDirection dir)
> > > +{
> > > + OffsetNumber low, high;
> > > + Page page = BufferGetPage(buf);
> > > + BTPageOpaque opaque = (BTPageOpaque) PageGetSpecialPointer(page);
> > > +
> > > + low = P_FIRSTDATAKEY(opaque);
> > > + high = PageGetMaxOffsetNumber(page);
> > > +
> > > + if (unlikely(high < low))
> > > + return false;
> > > +
> > > + return (_bt_compare(scan->indexRelation, key, page, low) > 0 &&
> > > + _bt_compare(scan->indexRelation, key, page, high) < 1);
> > > +}
> > >
> > > I think the high key condition should be changed to
> > > _bt_compare(scan->indexRelation, key, page, high) < 0 ?  Because if
> > > prefix qual is equal to the high key then also there is no point in
> > > searching on the current page so we can directly skip.
> >
> > From nbtree/README and comments to functions like _bt_split I've got an
> > impression that the high key could be equal to the last item on the leaf
> > page, so there is a point in searching. Is that incorrect?
>
> But IIUC,  here we want to decide whether we will get the next key in
> the current page or not?

In general this function does what it says, it checks wether or not the
provided scankey could be found within the page. All the logic about
finding a proper next key to fetch is implemented on the call site, and
within this function we want to test whatever was passed in. Does it
answer the question?