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: Michael Paquier <michael@paquier.xyz>
Cc: sunil s <sunilfeb26@gmail.com>, pgsql-hackers@lists.postgresql.org
Date: 2025-10-21T05:32:01Z
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 Tue, 21 Oct 2025 at 15:55, Michael Paquier <michael@paquier.xyz> wrote: > Using signed or unsigned is not going to matter much at the end. We > would be far from the count even if the number is signed. I'd leave it as uint64. There's no reason to mixup the signedness between these two variables. > + * Since heapBlk is incremented by opaque->bo_pagesPerRange, it can exceed > + * the maximum 32-bit limit (2^32) on very large tables, potentially causing > + * the loop to become infinite. > + * > + * To prevent this overflow, the counter must use a 64-bit type, ensuring it > + * can handle cases where nblocks approaches 2^32. > > 2^32 is mentioned twice. A simpler suggestion: > Since heapBlk is incremented by opaque->bo_pagesPerRange, it could > exceed the maximum 32-bit limit (2^32) on very large tables and > wraparound. The counter must be 64 bits wide for this reason. I wasn't a fan of that change either. I suggested "We make use of uint64 for heapBlk as a BlockNumber could wrap for tables with close to 2^32 pages.", but that's not what happened. > Like totalpages, there is an argument about consistency based on the > result type of bringetbitmap(). It's minor, still. I don't think totalpages being int64 is an argument to make the heapBlk int64. > It would be simpler to switch "pageno" to be 64-bit wide as well, > rather than casting it back to BlockNumber. I suggested that too, but ... I'm happy to finish this one off. I was leaving it for Tomas to comment, but I think he'll be busy with pgconf.eu for the next few days. David