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?
