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(at)sss(dot)pgh(dot)pa(dot)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