On Sun, 19 Oct 2025 at 19:03, sunil s <sunilfeb26@gmail.com> wrote:
> Previously, heapBlk was defined as an unsigned 32-bit integer. When incremented
> by pagesPerRange on very large tables, it could wrap around, causing the condition
> heapBlk < nblocks to remain true indefinitely — resulting in an infinite loop.
> This was explained very nicely by Tomas Vondra[1] and below two solutions were
>
> suggested.
> i) Change to int64
> ii) Tracking the prevHeapBlk
This is similar to what table_block_parallelscan_nextpage() has to do
to avoid wrapping around when working with tables containing near 2^32
pages. I'd suggest using uint64 rather than int64 and also adding a
comment to mention why that type is being used rather than
BlockNumber. Something like: "We make use of uint64 for heapBlk as a
BlockNumber could wrap for tables with close to 2^32 pages."
You could move the declaration of heapBlk into the for loop line so
that the comment about using uint64 is closer to the declaration.
I suspect you will also want to switch to using uint64 for the
"pageno" variable at the end of the loop. My compiler isn't warning
about the "pageno = heapBlk;", but maybe other compilers will.
Not for this patch, but I wonder why we switch memory contexts so
often in that final tbm_add_page() loop. I think it would be better to
switch once before the loop and back again after it. What's there
seems a bit wasteful for any pagesPerRange > 1.
David