public inbox for [email protected]  
help / color / mirror / Atom feed
From: Heikki Linnakangas <[email protected]>
To: Andres Freund <[email protected]>
To: [email protected] <[email protected]>
Cc: Alexander Kuzmenkov <[email protected]>
Subject: Assertion failure in hash_kill_items()
Date: Tue, 17 Mar 2026 19:15:10 +0200
Message-ID: <[email protected]> (raw)

I bumped into an assertion failure, while playing with variants of the 
test case that Alexander Kuzmenkov wrote to exercise hash index page 
cleanup [1]. This is master-only, related to the recent changes in how 
buffers are marked dirty.

CREATE TABLE hash_cleanup_heap(keycol INT);
CREATE INDEX hash_cleanup_index on hash_cleanup_heap USING HASH (keycol);
BEGIN;
INSERT INTO hash_cleanup_heap SELECT 1 FROM generate_series(1, 500) as i;
ROLLBACK;
SET enable_seqscan = off;
SET enable_bitmapscan = off;
SELECT count(*) FROM hash_cleanup_heap WHERE keycol = 1;

TRAP: failed Assert("BufferIsValid(buffer)"), File: 
"../src/backend/storage/buffer/bufmgr.c", Line: 509, PID: 1275145
postgres: heikki postgres [local] SELECT(ExceptionalCondition+0x84) 
[0xaaaaecab8650]
postgres: heikki postgres [local] SELECT(+0x20498b4) [0xaaaaec4698b4]
postgres: heikki postgres [local] SELECT(+0x205db78) [0xaaaaec47db78]
postgres: heikki postgres [local] SELECT(BufferBeginSetHintBits+0x44) 
[0xaaaaec47df58]
postgres: heikki postgres [local] SELECT(_hash_kill_items+0xa8c) 
[0xaaaaeb6a51c8]
postgres: heikki postgres [local] SELECT(_hash_next+0x2a8) [0xaaaaeb69d780]
postgres: heikki postgres [local] SELECT(hashgettuple+0x5a4) 
[0xaaaaeb682920]
postgres: heikki postgres [local] SELECT(index_getnext_tid+0x4b4) 
[0xaaaaeb73322c]
postgres: heikki postgres [local] SELECT(index_getnext_slot+0x90) 
[0xaaaaeb733ebc]
postgres: heikki postgres [local] SELECT(+0x19507d8) [0xaaaaebd707d8]
postgres: heikki postgres [local] SELECT(+0x189a47c) [0xaaaaebcba47c]
postgres: heikki postgres [local] SELECT(+0x189a588) [0xaaaaebcba588]
postgres: heikki postgres [local] SELECT(ExecScan+0x18c) [0xaaaaebcbab24]
postgres: heikki postgres [local] SELECT(+0x1953a00) [0xaaaaebd73a00]
postgres: heikki postgres [local] SELECT(+0x188bf9c) [0xaaaaebcabf9c]
postgres: heikki postgres [local] SELECT(+0x18cb754) [0xaaaaebceb754]
postgres: heikki postgres [local] SELECT(+0x18ccd70) [0xaaaaebcecd70]
postgres: heikki postgres [local] SELECT(+0x18dae40) [0xaaaaebcfae40]
postgres: heikki postgres [local] SELECT(+0x18d98d8) [0xaaaaebcf98d8]
postgres: heikki postgres [local] SELECT(+0x188bf9c) [0xaaaaebcabf9c]
postgres: heikki postgres [local] SELECT(+0x1858814) [0xaaaaebc78814]
postgres: heikki postgres [local] SELECT(+0x1863318) [0xaaaaebc83318]
postgres: heikki postgres [local] SELECT(standard_ExecutorRun+0x594) 
[0xaaaaebc7a034]

The first attached patch fixes it. It's pretty straightforward: the 
function was using so->currPos.buf, but that's only valid if the page 
was already pinned on entry to the function. It should be using the 
local 'buf' variable instead.

The second patch simplifies the condition in the 'unlock_page' part. 
This isn't new, and isn't needed to fix the bug, it just caught my eye 
while looking at this. I don't understand why the condition was the way 
it was, checking just 'havePin' seems sufficient and more correct to me. 
Am I missing something?

[1] 
https://www.postgresql.org/message-id/CALzhyqxrc1ZHYmf5V8NE%2ByMboqVg7xZrQM7K2c7VS0p1v8z42w%40mail.g...

- Heikki


Attachments:

  [text/x-patch] 0001-fix-crash-on-killing-items-on-hash-overflow-pages.patch (1.1K, 2-0001-fix-crash-on-killing-items-on-hash-overflow-pages.patch)
  download | inline diff:
From de433e4092e33e49a5f01c71d5ab6b34320167cd Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <[email protected]>
Date: Tue, 17 Mar 2026 19:06:04 +0200
Subject: [PATCH 1/2] fix crash on killing items on hash overflow pages

---
 src/backend/access/hash/hashutil.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/backend/access/hash/hashutil.c b/src/backend/access/hash/hashutil.c
index 3e16119d027..081adbc88a6 100644
--- a/src/backend/access/hash/hashutil.c
+++ b/src/backend/access/hash/hashutil.c
@@ -600,7 +600,7 @@ _hash_kill_items(IndexScanDesc scan)
 					 * update the page while just holding a share lock. If we
 					 * are not allowed, there's no point continuing.
 					 */
-					if (!BufferBeginSetHintBits(so->currPos.buf))
+					if (!BufferBeginSetHintBits(buf))
 						goto unlock_page;
 				}
 
@@ -621,7 +621,7 @@ _hash_kill_items(IndexScanDesc scan)
 	if (killedsomething)
 	{
 		opaque->hasho_flag |= LH_PAGE_HAS_DEAD_TUPLES;
-		BufferFinishSetHintBits(so->currPos.buf, true, true);
+		BufferFinishSetHintBits(buf, true, true);
 	}
 
 unlock_page:
-- 
2.47.3



  [text/x-patch] 0002-simplify-the-condition-in-unlock_page.patch (778B, 3-0002-simplify-the-condition-in-unlock_page.patch)
  download | inline diff:
From 8e7e3f6e63c92db8a7d8139ec60c0fdee98830ac Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <[email protected]>
Date: Tue, 17 Mar 2026 19:08:21 +0200
Subject: [PATCH 2/2] simplify the condition in 'unlock_page'

---
 src/backend/access/hash/hashutil.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/backend/access/hash/hashutil.c b/src/backend/access/hash/hashutil.c
index 081adbc88a6..1d1b05f876a 100644
--- a/src/backend/access/hash/hashutil.c
+++ b/src/backend/access/hash/hashutil.c
@@ -625,8 +625,7 @@ _hash_kill_items(IndexScanDesc scan)
 	}
 
 unlock_page:
-	if (so->hashso_bucket_buf == so->currPos.buf ||
-		havePin)
+	if (havePin)
 		LockBuffer(so->currPos.buf, BUFFER_LOCK_UNLOCK);
 	else
 		_hash_relbuf(rel, buf);
-- 
2.47.3



view thread (4+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: Assertion failure in hash_kill_items()
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox