"Pavan Deolasee" <pavan.deolasee@gmail.com> writes:
> I liked those changes. I am wondering if we could have avoided
> duplicating the chain following code in heap_hot_search_buffer and
> index_getnext.
I wasn't totally thrilled with duplicating that code, but the callers of
heap_hot_search are only interested in finding a single visible tuple,
whereas index_getnext has to find them all on successive visits.
Exposing a capability to do that in heap_hot_search seemed to make its
API ugly enough that duplicated code was better. (I really didn't like
the original setup where it was supposed to find the next tuple after
the one passed in, thereby forcing the caller to handle the first
chain member...)
BTW, I'm in process of taking out the separate HEAPTUPLE_DEAD_CHAIN
return value from HeapTupleSatisfiesVacuum. After looking through
all the callers I concluded that this complicated life more than it
helped. The places that actually want to make the distinction can
check HeapTupleIsHotUpdated easily enough for themselves, and in
several of the places that don't want to make the distinction it's
notationally cumbersome to deal with it. For instance, in the patch
as I posted it last night, both heap_hot_search and index_getnext
probably fail to detect all-dead HOT chains, because some of the
chain members are likely to be HEAPTUPLE_DEAD_CHAIN rather than plain
DEAD, and they're not coping.
regards, tom lane