> On Mar 6, 2026, at 15:56, Rafia Sabih <[email protected]> wrote:
>
>
>
> On Thu, 5 Mar 2026 at 19:26, Chao Li <[email protected]> wrote:
> Hi,
>
> While reviewing [1], I noticed several cases where BlockNumber seems to be
> misused.
>
> Although BlockNumber is currently underlying defined as uint32, it has a
> special meaning. For example:
> ```
> #define InvalidBlockNumber ((BlockNumber) 0xFFFFFFFF)
> #define MaxBlockNumber ((BlockNumber) 0xFFFFFFFE)
> ```
> So my understanding is that BlockNumber should only be used to identify a
> block.
> However, I saw several places where variables of type BlockNumber are
> actually used as counts. For example:
> ```
> typedef struct LVRelState
> {
>
> BlockNumber blkno; <== correct usage
>
> BlockNumber rel_pages; /* total number of pages */ <== mis-use
> ```
> Actually, InvalidBlockNumber and MaxBlockNumber are special values, not the
> BlockNumber itself, it is as you said underlying uint32.
> AFAIk these types for typedef are done so that we understand them in a
> particular context and not just use them as any other uint32. Increases the
> code readability.
> There are other such examples too like Bucket in Hash.
Hi Rafia,
Thanks for sharing your opinion. However, I am not fully convinced.
When we use a type, we usually don’t need to care about its underlying actual
type. If one day BlockNumber were redefined as a structure type, then all
usages where it is treated as a counter would break, right? While such a change
may be unlikely, it is not impossible. For example, there is an ongoing
discussion [1] proposing to change Datum into a structure, while Datum is
currently defined as uint64_t.
To me, this kind of misuse also hurts readability. As I mentioned earlier, I
noticed the issue while reviewing a patch: I was confused by the definition of
a struct field until I read the implementation and realized that it was
actually being used as a counter.
[1] https://postgr.es/m/[email protected]
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
