I don't think this set of functions provides good enough level of abstraction for storage AM. This functions encapsulate only low-level work of insert/update/delete tuples into heap itself. However, it still assumed that indexes are managed outside of storage AM. I don't think this is right, assuming that one of most demanded storage API usage would be different MVCC implementation (for instance, UNDO log based as discussed upthread). Different MVCC implementation is likely going to manage indexes in a different way. For example, storage AM utilizing UNDO would implement in-place update even when indexed columns are modified. Therefore this piece of code in ExecUpdate() wouldn't be relevant anymore.
/* * insert index entries for tuple * * Note: heap_update returns the tid (location) of the new tuple in * the t_self field. * * If it's a HOT update, we mustn't insert new index entries. */ if ((resultRelInfo->ri_NumIndices > 0) && !storage_tuple_is_heaponly(resultRelationDesc, tuple)) recheckIndexes = ExecInsertIndexTuples(slot, &(slot->tts_tid), estate, false, NULL, NIL);
I'm firmly convinced that this logic should be encapsulated into storage AM altogether with inserting new index tuples on storage insert. Also, HOT should be completely encapsulated into heapam. It's quite evident for me that storage utilizing UNDO wouldn't have anything like our current HOT. Therefore, I think there shouldn't be hot_search_buffer() API function. tuple_fetch() may cover hot_search_buffer(). That might require some signature change of tuple_fetch() (probably, extra arguments).
For me, it's crucial point that pluggable storages should be able to have different MVCC implementation, and correspondingly have full control over its interactions with indexes.
Thus, it would be good if we would get consensus on that point. I'd like other discussion participants to comment whether they agree/disagree and why.
