Re: Mis-use of type BlockNumber?

From: Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: pgsql-hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Mis-use of type BlockNumber?
Date: 2026-03-06 14:10:42
Message-ID: EF2453A1-DB27-4D4D-8D8B-3D3659A40FDD@gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On Mar 6, 2026, at 21:16, Andres Freund <andres(at)anarazel(dot)de> 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/

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Heikki Linnakangas 2026-03-06 14:12:57 Re: Better shared data structure management and resizable shared data structures
Previous Message Sami Imseih 2026-03-06 13:29:08 Re: [BUG + PATCH] DSA pagemap out-of-bounds in make_new_segment odd-sized path