Re: heapgettup() with NoMovementScanDirection unused in core?

Hi,

I have written the patch to remove the unreachable code in
heapgettup_pagemode]().

On Wed, Jan 25, 2023 at 10:02 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> 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.)"
>

I also did what I *think* Tom is suggesting here -- make index scan's
scan direction always forward or backward...

Maybe the set should be two patches...dunno.

- Melanie