Mis-use of type BlockNumber?

On Thu, 5 Mar 2026 at 19:26, Chao Li <li.evan.chao@gmail.com> 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.

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?

[1] /messages/by-id/81DAFACF-7D55-4A84-ACB0-0425D1669DB4@gmail.com

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

--
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH

On Mar 6, 2026, at 15:56, Rafia Sabih <rafia.pghackers@gmail.com> wrote:

On Thu, 5 Mar 2026 at 19:26, Chao Li <li.evan.chao@gmail.com> 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]/messages/by-id/8246d7ff-f4b7-4363-913e-827dadfeb145@eisentraut.org 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]: /messages/by-id/8246d7ff-f4b7-4363-913e-827dadfeb145@eisentraut.org

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

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

On Mar 6, 2026, at 21:16, Andres Freund <andres@anarazel.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/

On Fri, Mar 6, 2026 at 9:11 AM Chao Li <li.evan.chao@gmail.com> wrote:

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?

I also don't like how BlockNumber is used this way. However, if you
introduce a new counter and use uint32 instead when all surrounding
counters are BlockNumber, it sticks out as different and is confusing.
If you go and change all instances of BlockNumber being used this way
because you added one new counter, that's a lot of churn. If we decide
to change BlockNumber to a struct, we'll have a good reason and be
willing to pay the price of the churn at that point. I'm not a
stickler for consistency and in fact think it's okay to have some
things inconsistent if it means introducing new patterns that are
better. But having a single counter in a struct of the same unit as a
bunch of existing counters be a different datatype doesn't seem worth
it to me.

- Melanie

Hi,

On 2026-03-06 10:22:10 -0500, Melanie Plageman wrote:

On Fri, Mar 6, 2026 at 9:11 AM Chao Li <li.evan.chao@gmail.com> wrote:

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?

I also don't like how BlockNumber is used this way. However, if you
introduce a new counter and use uint32 instead when all surrounding
counters are BlockNumber, it sticks out as different and is confusing.

FWIW, I don't think uint32 would be a good choice. I think we're eventually
going to have to allow larger relations and a lot of counters in uint32 would
make that a good bit harder than right now, where it's BlockNumber. So you'd
have to introduce a new BlockCounter type. At which point you ... can just use
BlockNumber, or uint64 (and waste space for now).

Greetings,

Andres Freund

On Mar 7, 2026, at 03:31, Andres Freund <andres@anarazel.de> wrote:

Hi,

On 2026-03-06 10:22:10 -0500, Melanie Plageman wrote:

On Fri, Mar 6, 2026 at 9:11 AM Chao Li <li.evan.chao@gmail.com> wrote:

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?

I also don't like how BlockNumber is used this way. However, if you
introduce a new counter and use uint32 instead when all surrounding
counters are BlockNumber, it sticks out as different and is confusing.

FWIW, I don't think uint32 would be a good choice. I think we're eventually
going to have to allow larger relations and a lot of counters in uint32 would
make that a good bit harder than right now, where it's BlockNumber. So you'd
have to introduce a new BlockCounter type. At which point you ... can just use
BlockNumber, or uint64 (and waste space for now).

Thanks for sharing your thoughts.

IMHO, the range of a counter does not have to match the range of BlockNumber, that really depends on the context. Even if I were to do such a refactoring, I was never thinking of replacing BlockNumber with uint32. To me, the right choice should be an ordinary numeric type selected according to the semantics of the value and the required range.

Similarly, there are many places that count the number of tuples, but PG does not have a dedicated type for that, e.g. something like TupleCount. That suggests, at least to me, that the question here is simply which ordinary numeric type best matches the meaning of the value, rather than reusing BlockNumber by default.

All in all, I think there is at least agreement that using BlockNumber for a block counter is not semantically ideal, and that doing a wholesale refactoring is probably not worthwhile. Where we seem to differ is that I think we should avoid introducing new cases like this, and perhaps fix existing ones opportunistically when a patch is already touching the relevant code. It sounds like the preference is to leave it as-is for now.

I have noted this, and perhaps raise it again if there is a better opportunity in the future.

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