Hi, On 2023-01-25 10:02:28 -0500, Tom Lane wrote: > David Rowley <dgrowle...@gmail.com> writes: > > Does anyone know of any reason why we shouldn't ditch the nomovement > > code in heapgettup/heapgettup_pagemode?
+1 Because I dug it up yesterday. There used to be callers of heap* with NoMovement. But they were unused themselves: http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=adbfab119b308a7e0e6b1305de9be222cfd5c85b > * 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. I wonder if we'd be better off removing NoMovementScanDirection, and using count == (uint64)-1 for what NoMovementScanDirection is currently used for as an ExecutorRun parameter. Seems less confusing to me - right now we have two parameters with non-obbvious meanings and interactions. > 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.) +1 Certainly seems confusing to me. Greetings, Andres Freund
