At Wed, 5 Feb 2020 17:37:30 +0100, Dmitry Dolgov <9erthali...@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. regards. -- Kyotaro Horiguchi NTT Open Source Software Center
