Re: heapgettup() with NoMovementScanDirection unused in core?

On Sat, 28 Jan 2023 at 12:15, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> /*
>  * Determine the net effect of two direction specifications.
>  * This relies on having ForwardScanDirection = +1, BackwardScanDirection = -1,
>  * and will probably not do what you want if applied to any other values.
>  */
> #define CombineScanDirections(a, b)  ((a) * (b))
>
> The main thing this'd buy us is being able to grep for uses of the
> trick.  If it's written as just multiplication, good luck being
> able to find what's depending on that, should you ever need to.

Yeah, I think the multiplication macro is a good way of doing it.
Having the definition of it close to the ScanDirection enum's
definition is likely a very good idea so that anyone adjusting the
enum values is more likely to notice that it'll cause an issue. A
small note on the enum declaration about the -1, +1 values being
exploited in various places might be a good idea too. I see v6-0006 in
[1] further exploits this, so that's further reason to document that.

My personal preference would have been to call it
ScanDirectionCombine, so the naming is more aligned to the 4 other
macro names that start with ScanDirection in sdir.h, but I'm not going
to fuss over it.

David

[1] https://postgr.es/m/CAAKRu_ZyiXwWS1WXSZneoy+sjoH_+F5UhO-1uFhyi-u0d6z+fA@mail.gmail.com


David