On Wed, May 16, 2018 at 1:41 AM, Tom Lane <t...@sss.pgh.pa.us> wrote:
> Nikita Glukhov <n.glu...@postgrespro.ru> writes: > > Experimenting with the new pluggable storage API, I found that > amcanbackward > > flag is not checked in build_index_paths() before > > build_index_pathkeys(... BackwardScanDirection) call when we are building > > paths for ORDER BY. And this flag is even not copied into IndexOptInfo > struct. > > Obviously, this can lead to misuse of Backward Index [Only] Scan plans. > > > Attached patch with the corresponding fix. > > I think this patch is based on a misunderstanding of what amcanbackward > means. We *require* indexes that claim to support amcanorder to support > scanning in either direction. What amcanbackward is about is whether > the index can support reversing direction mid-scan, as would be required > to support FETCH FORWARD followed by FETCH BACKWARD in a cursor. That's > actually independent of whether the index can implement a defined > ordering; see for example the hash AM, which sets amcanbackward but not > amcanorder. > > This is documented, though perhaps not sufficiently clearly, in > indexam.sgml: > > The amgettuple function has a direction argument, which can be either > ForwardScanDirection (the normal case) or BackwardScanDirection. If > the first call after amrescan specifies BackwardScanDirection, then > the set of matching index entries is to be scanned back-to-front > rather than in the normal front-to-back direction, so amgettuple must > return the last matching tuple in the index, rather than the first one > as it normally would. (This will only occur for access methods that > set amcanorder to true.) After the first call, amgettuple must be > prepared to advance the scan in either direction from the most > recently returned entry. (But if amcanbackward is false, all > subsequent calls will have the same direction as the first one.) > Thank you for clarifying this point. We've missed that. Perhaps there is a case for adding an additional flag to allow specifying > "I can't support ORDER BY DESC", but I'm not in a big hurry to do so. > I think there would be more changes than this needed to handle such a > restriction, anyway. > OK, got it. We'll probably propose a patch implementing that to the next commitfest. ------ Alexander Korotkov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company
