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