Assertion failure in hash_kill_items()

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.gmail.com

- Heikki