Thread

Re: Replace magic numbers with strategy numbers for B-tree indexes

Daniil Davydov <3danissimo@gmail.com> — 2025-07-03T07:50:59Z

Hi,

On Wed, Jul 2, 2025 at 6:24 PM Peter Eisentraut <peter@eisentraut.org> wrote:
>
> On 30.06.25 05:21, Daniil Davydov wrote:
> > Hi,
> > I noticed that some asserts and cycles use magic numbers 1 and 0
> > instead of BTLessStrategyNumber and InvalidStrategy.
> > At the same time, the BTMaxStrategyNumber macro is used there.
> > I suggest using appropriate macros for 1 and 0 values.
>
> This code, both the original and your changes, make a lot of assumptions
> about the btree strategy numbers, such as that BTLessStrategyNumber is
> the smallest valid one, that InvalidStrategy is smaller than all of
> them, and that all numbers between the smallest and BTMaxStrategyNumber
> are assigned.
>
> However, some of the code actually does require that, because it fills
> in array fields for consecutive strategy numbers.  So hiding that fact
> by changing 1 to BTLessStrategyNumber introduces more mystery.
>

Thanks for looking into it!

OK, I can agree that the assumption that InvalidStrategy has the
smallest value is a bit too rough.

But BTLessStrategyNumber and BTMaxStrategyNumber literally say that
these are the min/max numbers.
Thus, assertions like "strategynum >= BTLessStrategyNumber" makes much
more sense than "strategynum >= 1"
(especially when the comment says something like "Check that only
allowed strategy numbers exist") and it is easier to maintain.

The same goes for cycles like [BTLessStrategyNumber;
BTMaxStrategyNumber] and [1; BTMaxStrategyNumber].
All arrays working with strategy numbers are initializing with
BTMaxStrategyNumber elements, so we cannot get any error here.
And if we init an array with length = BTMaxStrategyNumber, we must
assume that all numbers are assigned.
Otherwise, I don't understand why we should have "holes" in the numbering?

I still think that we should get rid of magic numbers.
As a compromise, I'm not replacing 0 with Invalid in the second
version of the patch.

What do you think?

--
Best regards,
Daniil Davydov