Re: Hash index bucket split bug

From: vignesh C <vignesh21(at)gmail(dot)com>
To: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Cc: Peter Geoghegan <pg(at)bowt(dot)ie>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Robert Haas <robertmhaas(at)gmail(dot)com>
Subject: Re: Hash index bucket split bug
Date: 2026-08-10 12:02:43
Message-ID: CALDaNm0dk1Po7F1+csYqejWy=HT6YYz_Ao5WhQoEnNVrwLZBfw@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, 10 Aug 2026 at 15:49, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Thu, Jul 9, 2026 at 10:02 PM Peter Geoghegan <pg(at)bowt(dot)ie> wrote:
> >
> > I ran this against CI, and saw failures on 32-bit meson tied to tuple
> > alignment. When MAXALIGN is 4, the assumptions about page layout
> > underlying the test case break.
> >
> > Attached is V3, which directly addresses the alignment issue, and
> > fully passes CI.
> >
>
> Thanks, the fixup-patch looks good to me. However, I was thinking
> whether it is a good idea to refactor the common code into separate
> functions like attached? This is atop your patches. BTW, I took help
> from Claude to do this refactoring. I am fine with committing
> /sql/hash_split.sql test along with the code-fix.

One suggestion: how about adding an assertion at the end of the scan
so that we catch this condition immediately?
With the fix, reaching 'end_of_scan' in the forward direction with
'hashso_buc_populated && !hashso_buc_split' should no longer be
possible in '_hash_next()'. We could add an assertion along these
lines:
if (end_of_scan)
{
Assert(!so->hashso_buc_populated ||
(ScanDirectionIsForward(dir) ?
so->hashso_buc_split :
!so->hashso_buc_split));

_hash_dropscanbuf(rel, so);
...
}

This would help ensure that the scan state is consistent when we reach
the end of the scan and would make any regression in this area fail
with an assertion rather than silently producing an incorrect result.
If there is a more appropriate location or a simpler invariant that
captures the same condition, I'm happy with that approach as well.

Regards,
Vignesh

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Karina Litskevich 2026-08-10 12:21:39 Re: Implicit conversion from int64 to int32 when calling hash_get_num_entries
Previous Message Kirill Reshke 2026-08-10 11:41:23 Re: REVOKE's CASCADE protection doesn't work with INHERITed table owners