Hi,
On 2026-03-06 11:25:45 +0800, Chao Li wrote:
> 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
> ```
> In this example, rel_pages is actually a count. In theory it could just be an
> int (or some other count type).
> Another example:
> ```
> static void
> system_time_samplescangetsamplesize(PlannerInfo *root,
>
> RelOptInfo *baserel,
> List
> *paramexprs,
>
> BlockNumber *pages,
> double
> *tuples)
> ```
> Here the parameter pages is also a count indicating the number of pages.
>
> So I want to confirm whether my understanding is correct that these are
> misuses of BlockNumber. If so, would it be worth a cleanup patch to fix such
> cases?
I don't love that we use BlockNumber this way either, but it is fairly
widespread and of long standing. There is a lot of code in postgres, if you
just go and scour it to find stuff you individually don't like, you will be
busy for a long time (writing the "fixes") and we will be busy for a long time
(dealing with the influx of patches). Just because some rarely changing
aspect of the code does something mildly ugly doesn't mean you have to
immediately send patches about it. Sending and processing patches takes time
and energy.
Greetings,
Andres Freund
