Date: Thu, 6 Feb 2020 11:57:07 +0100
From: Dmitry Dolgov <9erthalion6@gmail.com>
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: florisvannee@Optiver.com, pg@bowt.ie, jesper.pedersen@redhat.com,
	david.rowley@2ndquadrant.com, tomas.vondra@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: <20200206105707.fuidmdsi6oapieoz@localhost>
References: <20200204200205.ip5c2mzy3i2okqey@localhost>
 <5be025f30d8e45cda167c58b5d5e72d2@opammb0561.comp.optiver.com>
 <20200205163730.eqvoxh6eb3do52r5@localhost>
 <20200206.102450.1514793516830965023.horikyota.ntt@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200206.102450.1514793516830965023.horikyota.ntt@gmail.com>
Precedence: bulk

> 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
> > > On Tue, Feb 04, 2020 at 08:34:09PM +0000, Floris Van Nee wrote:
> > >
> > > > this point me and Jesper inclined to go with the second option. But maybe
> > > > I'm missing something, are there any other suggestions?
> > >
> > > Unfortunately I figured this would need a more invasive fix. I tend to agree that it'd be better to not skip in situations like this. I think it'd make most sense to make any plan for these 'prepare/fetch' queries would not use skip, but rather a materialize node, right?
> >
> > Yes, sort of, without a skip scan it would be just an index only scan
> > with unique on top. Actually it's not immediately clean how to achieve
> > this, since at the moment, when planner is deciding to consider index
> > skip scan, there is no information about neither direction nor whether
> > we're dealing with a cursor. Maybe we can somehow signal to the decision
> > logic that the root was a DeclareCursorStmt by e.g. introducing a new
> > field to the query structure (or abusing an existing one, since
> > DeclareCursorStmt is being processed by standard_ProcessUtility, just
> > for a test I've tried to use utilityStmt of a nested statement hoping
> > that it's unused and it didn't break tests yet).
>
> Umm.  I think it's a wrong direction.  While defining a cursor,
> default scrollability is decided based on the query allows backward
> scan or not.  That is, the definition of backward-scan'ability is not
> just whether it can scan from the end toward the beginning, but
> whether it can go back and forth freely or not.  In that definition,
> the *current* skip scan does not supporting backward scan.  If we want
> to allow descending order-by in a query, we should support scrollable
> cursor, too.
>
> 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?