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
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 |