Re: [BUGFIX] amcanbackward is not checked before building backward index paths

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru>
Cc: PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [BUGFIX] amcanbackward is not checked before building backward index paths
Date: 2018-05-15 22:41:16
Message-ID: 27644.1526424076@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)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.)

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.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David Rowley 2018-05-15 22:51:20 Re: Should total_pages be calculated after partition pruning and constraint exclusion?
Previous Message Tom Lane 2018-05-15 22:07:42 Re: Flexible permissions for REFRESH MATERIALIZED VIEW