Re: nbtree: assertion failure in _bt_killitems() for posting tuple

On Thu, Mar 19, 2020 at 9:34 AM Anastasia Lubennikova
<a(dot)lubennikova(at)postgrespro(dot)ru> wrote:
> During tests, we catched an assertion failure in _bt_killitems() for
> posting tuple in unique index:
>
> /* kitem must have matching offnum when heap TIDs match */
> Assert(kitem->indexOffset == offnum);
>
> https://github.com/postgres/postgres/blob/master/src/backend/access/nbtree/nbtutils.c#L1809
>
> I struggle to understand the meaning of this assertion.
> Don't we allow the chance that posting tuple moved right on the page as
> the comment says?

I think you're right. However, it still seems like we should check
that "kitem->indexOffset" is consistent among all of the BTScanPosItem
entries that we have for each TID that we believe to be from the same
posting list tuple.

(Thinks some more...)

Even if the offnum changes when the buffer lock is released, due to
somebody inserting on to the same page, I guess that we still expect
to observe all of the heap TIDs together in the posting list. Though
maybe not. Maybe it's possible for a deduplication pass to occur when
the buffer lock is dropped, in which case we should arguably behave in
the same way when we see the same heap TIDs (i.e. delete the entire
posting list without regard for whether or not the TIDs happened to
appear in a posting list initially). I'm not sure, though.

It will make no difference most of the time, since the
kill_prior_tuple stuff is generally not applied when the page is
changed at all -- the LSN is checked by the logic added by commit
2ed5b87f. That's why I asked about unlogged indexes (we don't do the
LSN thing there). But I still think that we need to take a firm
position on it.

--
Peter Geoghegan