Re: heapgettup() with NoMovementScanDirection unused in core?

David Rowley <dgrowleyml@gmail.com> writes:
> Does anyone know of any reason why we shouldn't ditch the nomovement
> code in heapgettup/heapgettup_pagemode?

AFAICS, the remaining actual use-case for NoMovementScanDirection
is that defined by ExecutorRun:

 *        If direction is NoMovementScanDirection then nothing is done
 *        except to start up/shut down the destination.  Otherwise,
 *        we retrieve up to 'count' tuples in the specified direction.
 *
 *        Note: count = 0 is interpreted as no portal limit, i.e., run to
 *        completion.

We must have the NoMovementScanDirection option because count = 0
does not mean "do nothing", and I noted at least two call sites
that require it.

The heapgettup definition is thus not only unreachable, but confusingly
inconsistent with this meaning.

I wonder if we couldn't also get rid of this confusingly-inconsistent
alternative usage in the planner:

 * 'indexscandir' is one of:
 *        ForwardScanDirection: forward scan of an ordered index
 *        BackwardScanDirection: backward scan of an ordered index
 *        NoMovementScanDirection: scan of an unordered index, or don't care
 * (The executor doesn't care whether it gets ForwardScanDirection or
 * NoMovementScanDirection for an indexscan, but the planner wants to
 * distinguish ordered from unordered indexes for building pathkeys.)

While that comment's claim is plausible, I think it's been wrong for
years.  AFAICS indxpath.c makes pathkeys before it ever does this:

                                      index_is_ordered ?
                                      ForwardScanDirection :
                                      NoMovementScanDirection,

and nothing depends on that later either.  So I think we could
simplify this to something like "indexscandir is either
ForwardScanDirection or BackwardScanDirection.  (Unordered
index types need not support BackwardScanDirection.)"

            regards, tom lane