On 2/24/23 16:14, Alvaro Herrera wrote:
> On 2023-Feb-24, Tomas Vondra wrote:
>
>> I guess the easiest fix would be to do the arithmetic in 64 bits. That'd
>> eliminate the overflow.
>
> Yeah, that might be easy to set up. We then don't have to worry about
> it until BlockNumber is enlarged to 64 bits ... but by that time surely
> we can just grow it again to a 128 bit loop variable.
>
>> Alternatively, we could do something like
>>
>> prevHeapBlk = 0;
>> for (heapBlk = 0; (heapBlk < nblocks) && (prevHeapBlk <= heapBlk);
>> heapBlk += pagesPerRange)
>> {
>> ...
>> prevHeapBlk = heapBlk;
>> }
>
> I think a formulation of this kind has the benefit that it works after
> BlockNumber is enlarged to 64 bits, and doesn't have to be changed ever
> again (assuming it is correct).
>
Did anyone even propose doing that? I suspect this is unlikely to be the
only place that'd might be broken by that.
> ... if pagesPerRange is not a whole divisor of MaxBlockNumber, I think
> this will neglect the last range in the table.
>
Why would it? Let's say BlockNumber is uint8, i.e. 255 max. And there
are 10 pages per range. That's 25 "full" ranges, and the last range
being just 5 pages. So we get into
prevHeapBlk = 240
heapBlk = 250
and we read the last 5 pages. And then we update
prevHeapBlk = 250
heapBlk = (250 + 10) % 255 = 5
and we don't do that loop. Or did I get this wrong, somehow?
regards
--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company