Re: [HACKERS] Patch: Write Amplification Reduction Method (WARM)

Looking at your 0002 patch now.  It no longer applies, but the conflicts
are trivial to fix.  Please rebase and resubmit.

 Maybe I will spend some time trying to
convert it to Perl using PostgresNode.

I think having the "recheck" index methods create an ExecutorState looks
out of place.  How difficult is it to pass the estate from the calling
code?

IMO heap_get_root_tuple_one should be called just heap_get_root_tuple().
That function and its plural sibling heap_get_root_tuples() should
indicate in their own comments what the expectations are regarding the
root_offsets output argument, rather than deferring to the comments in
the "internal" function, since they differ on that point; for the rest
of the invariants I think it makes sense to say "Also see the comment
for heap_get_root_tuples_internal".  I wonder if heap_get_root_tuple
should just return the ctid instead of assigning the value to a
passed-in pointer, i.e.
OffsetNumber
heap_get_root_tuple(Page page, OffsetNumber target_offnum)
{
        OffsetNumber    off;
        heap_get_root_tuples_internal(page, target_offnum, &off);
        return off;
}

The simple_heap_update + CatalogUpdateIndexes pattern is getting
obnoxious.  How about creating something like catalog_heap_update which
does both things at once, and stop bothering each callsite with the WARM
stuff?  In fact, given that CatalogUpdateIndexes is used in other
places, maybe we should leave its API alone and create another function,
so that we don't have to change the many places that only do
simple_heap_insert.  (Places like OperatorCreate which do either insert
or update could just move the index update call into each branch.)

.
I wonder if heap_hot_search_buffer() and heap_hot_search() should return
a tri-valued enum instead of boolean; that idea looks reasonable in
theory but callers have to do more work afterwards, so maybe not.