Re: Corrupted btree index on HEAD because of covering indexes

> I would like to go and implement this check now, and if all goes well
> I may propose that you commit it to the master branch for v11. I don't
> see this as a new feature. I see it as essential testing
> infrastructure. What do you think about adding this new check soon?
Agree. Hope, nobody found a way how to use amcheck module in production 
to serve user requests. But, it should be implemented before BETA stage, 
in opposite case we will get a lot of objections.

> 
>> Nevertheless, seems, I found. In _bt_mark_page_halfdead() we use truncated
>> high key IndexTuple as a storage of blocknumber of top parent to remove. And
>> sets BTreeTupleSetNAtts(&trunctuple, 0) - it's stored in ip_posid.
>>
>> But some later, in _bt_unlink_halfdead_page() we check ItemPointer high key
>> with ItemPointerIsValid macro - and it returns false, because offset is
>> actually InvalidOffsetNumber - i.e. 0 which was set by BTreeTupleSetNAtts.
>> And some wrong decisions are follows, I didn't look at that.
> 
> I'm not at all surprised. I strongly suspected that it was some simple
> issue with the representation, since the INCLUDE patch didn't actually
> change the page deletion algorithm in any way.
+1

> 
>> Trivial and naive fix is attached, but for me it looks a bit annoing that we
>> store pointer (leafhikey) somewhere inside unlocked page.
> 
> Well, it has worked that way for a long time. No reason to change it now.
Agree, but at least this place needs a comment - why it's safe.

> I also think that we could have better conventional regression test
> coverage here.
Will try to invent not so large test.

-- 
Teodor Sigaev                      E-mail: teodor@sigaev.ru
                                       WWW: http://www.sigaev.ru/