Hash index bucket split bug

Started by Peter Geoghegan20 days ago4 messageshackers
Jump to latest

There's a bug in the logic for hash index scans: we fail to fully
account for concurrent bucket splits. This can lead to incorrect
results, as an affected scan may fail to return all matching rows.
This is due to an oversight in commit 7c75ef57, so all stable branches
are affected.

The underlying problem is that _hash_next fails to step from one
bucket of an in-progress to the other. In other words, it lacks the
required transition logic that _hash_readnext already has (and
_hash_readprev along with it). Note that the transition ordinarily
happens anyway, inside _hash_readpage's no-matches loop, which is why
this went unnoticed for so long: _hash_next only fails to make the
transition when the last page it reads in the first bucket contains at
least one matching tuple. In practice that means a tuple inserted into
the bucket being populated after the split began.

The attached bug fix patch v1-0001-* simply adds the missing handling
to _hash_next (copied from _hash_readnext and _hash_readprev).

I stumbled upon this bug when I asked Claude code to review a patch of
mine that adds support for the new amgetbatch interface to the hash
index AM (allowing hash indexes to perform I/O prefetching of heap
blocks). I had Claude rewrite its original reproducer into a
regression test and an isolation test, both of which appear in the
second patch.

Both tests are useful in their own way. The regression test runs
quickly and so might be worth committing alongside the fix -- but it
depends on leaving behind an incomplete split. The isolation test
demonstrates that the bug doesn't hinge upon an incomplete split. The
incomplete split case is particularly nasty, though, because the
potential for wrong answers persists until an inserter completes the
split.

--
Peter Geoghegan

Attachments:

v1-0001-Fix-hash-index-scans-concurrent-with-bucket-split.patchapplication/octet-stream; name=v1-0001-Fix-hash-index-scans-concurrent-with-bucket-split.patchDownload+54-1
v1-0002-Add-tests-for-hash-index-scans-concurrent-with-bu.patchapplication/octet-stream; name=v1-0002-Add-tests-for-hash-index-scans-concurrent-with-bu.patchDownload+371-2
#2Amit Kapila
amit.kapila16@gmail.com
In reply to: Peter Geoghegan (#1)
Re: Hash index bucket split bug

On Thu, Jul 9, 2026 at 2:10 AM Peter Geoghegan <pg@bowt.ie> wrote:

Both tests are useful in their own way. The regression test runs
quickly and so might be worth committing alongside the fix -- but it
depends on leaving behind an incomplete split. The isolation test
demonstrates that the bug doesn't hinge upon an incomplete split. The
incomplete split case is particularly nasty, though, because the
potential for wrong answers persists until an inserter completes the
split.

I am getting following regression diff even after applying the patch:
-step s1_split: <... completed>
step s2_scan:
SELECT count(*) FROM hash_split_test
WHERE v = (SELECT k FROM hash_split_key);
@@ -68,3 +67,4 @@
11
(1 row)

I will review the code later.

--
With Regards,
Amit Kapila.

In reply to: Amit Kapila (#2)
Re: Hash index bucket split bug

On Thu, Jul 9, 2026 at 6:45 AM Amit Kapila <amit.kapila16@gmail.com> wrote:

I am getting following regression diff even after applying the patch:
-step s1_split: <... completed>
step s2_scan:
SELECT count(*) FROM hash_split_test
WHERE v = (SELECT k FROM hash_split_key);
@@ -68,3 +67,4 @@
11
(1 row)

This is just a minor test flakiness issue with the isolation test.
Apparently, we need to force a deterministic report position by adding
a no-op step to the splitter's session.

Attached v2 has a more stable version of the isolation test (no other
changes compared to V1). Does this version work for you?

The isolation test is just for illustrative purposes. I don't think
that it's committable.

--
Peter Geoghegan

Attachments:

v2-0001-Fix-hash-index-scans-concurrent-with-bucket-split.patchapplication/octet-stream; name=v2-0001-Fix-hash-index-scans-concurrent-with-bucket-split.patchDownload+54-1
v2-0002-Add-tests-for-hash-index-scans-concurrent-with-bu.patchapplication/octet-stream; name=v2-0002-Add-tests-for-hash-index-scans-concurrent-with-bu.patchDownload+376-2
In reply to: Peter Geoghegan (#3)
Re: Hash index bucket split bug

On Thu, Jul 9, 2026 at 11:24 AM Peter Geoghegan <pg@bowt.ie> wrote:

Attached v2 has a more stable version of the isolation test (no other
changes compared to V1). Does this version work for you?

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.

--
Peter Geoghegan

Attachments:

v3-0001-Fix-hash-index-scans-concurrent-with-bucket-split.patchapplication/x-patch; name=v3-0001-Fix-hash-index-scans-concurrent-with-bucket-split.patchDownload+54-1
v3-0002-Add-tests-for-hash-index-scans-concurrent-with-bu.patchapplication/x-patch; name=v3-0002-Add-tests-for-hash-index-scans-concurrent-with-bu.patchDownload+394-2