Re: BRIN: Prevent the heapblk overflow during index summarization on very large tables resulting in an infinite loop

From: David Rowley <dgrowleyml(at)gmail(dot)com>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: sunil s <sunilfeb26(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: BRIN: Prevent the heapblk overflow during index summarization on very large tables resulting in an infinite loop
Date: 2025-10-21 05:32:01
Message-ID: CAApHDvqdvCdZiZ=K99o+kirYh-byxyPU9vPfFwtbRtMVTxzKDw@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, 21 Oct 2025 at 15:55, Michael Paquier <michael(at)paquier(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2025-10-21 05:48:37 Re: Extended Statistics set/restore/clear functions.
Previous Message Peter Smith 2025-10-21 05:27:41 Re: POC: enable logical decoding when wal_level = 'replica' without a server restart