> On Mar 6, 2026, at 21:16, Andres Freund <[email protected]> wrote:
> 
> 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.
> 

That’s a fair point and that’s why I didn’t go ahead to work on the patch.

But I think we should avoid to introduce such usages in new code. In other 
words, while reviewing patches, we should raise comments for such mis-usages. 
Is my understanding correct?

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/






Reply via email to