Re: Fix overflow of nbatch

From: Tomas Vondra <tomas(at)vondra(dot)me>
To: David Rowley <dgrowleyml(at)gmail(dot)com>
Cc: Vaibhav Jain <jainva(at)google(dot)com>, pgsql-hackers(at)postgresql(dot)org, Madhukar <madhukarprasad(at)google(dot)com>, Sangeetha Seshadri <sangsesh(at)google(dot)com>
Subject: Re: Fix overflow of nbatch
Date: 2025-09-23 01:01:54
Message-ID: e463272d-358b-4df3-a4b5-51a6f182cc46@vondra.me
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 9/23/25 02:02, David Rowley wrote:
> On Tue, 23 Sept 2025 at 09:31, Tomas Vondra <tomas(at)vondra(dot)me> wrote:
>> On 9/22/25 22:45, David Rowley wrote:
>>> I think a1b4f289b mistakenly thought that there'd be size_t arithmetic
>>> in the following two lines because the final result is a size_t:
>>>
>>> size_t current_space = hash_table_bytes + (2 * nbatch * BLCKSZ);
>>> size_t new_space = hash_table_bytes * 2 + (nbatch * BLCKSZ);
>>>
>>
>> Yeah, I failed to notice this part of the formula can overflow.
>
> Ok cool. We're just in the freeze for 18.0 at the moment. Once that's
> over, should I take care of this, or do you want to?
>

Feel free to fix, but I can take care of it once 18 is out the door.
It's my bug, after all.

BTW ExecHashIncreaseBatchSize needs the same fix, I think.

I wonder how likely the overflow is. AFAICS we'd need nbatch=256k (with
8KB blocks), which is a lot. But with the balancing logic, it'd also
mean each batch is about ~2GB. So the whole "hash table" would be about
500GB. Possible, but unlikely.

However, looking at the code now, I think the code should adjust the
hash_table_bytes value, not just space_allowed. It's meant to be the
same thing. Will check tomorrow.

thanks

--
Tomas Vondra

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David Rowley 2025-09-23 01:20:13 Re: Fix overflow of nbatch
Previous Message Chao Li 2025-09-23 00:51:29 Re: Trivial fix for comment of function table_tuple_lock