Re: BRIN: Prevent the heapblk overflow during index summarization on very large tables resulting in an infinite loop
David Rowley <dgrowleyml@gmail.com>
From: David Rowley <dgrowleyml@gmail.com>
To: sunil s <sunilfeb26@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org
Date: 2025-10-19T08:52:23Z
Lists: pgsql-hackers
Commits
Same data as JSON:
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
Fix BRIN 32-bit counter wrap issue with huge tables
- c28f8ca29445 13.23 landed
- eea24eb0ac2f 14.20 landed
- 810aaf7f2706 15.15 landed
- ef915bf9367c 16.11 landed
- c4f5a59ab16b 17.7 landed
- 715983a81ac8 18.1 landed
- 9fd29d7ff476 19 (unreleased) landed
-
Fix integer overflow bug in GiST buffering build calculations.
- 4bc6fb57f774 9.2.0 cited
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