Hi, On 2019-03-21 11:15:57 -0700, Andres Freund wrote: > Pending work: > - Wondering if table_insert/delete/update should rather be > table_tuple_insert etc. Would be a bit more consistent with the > callback names, but a bigger departure from existing code.
I've left this as is. > - I'm not yet happy with TableTupleDeleted computation in heapam.c, I > want to revise that further I changed that. Found a bunch of untested paths, I've pushed tests for those already. > - formatting Done that. > - commit message Done that. > - a few comments need a bit of polishing (ExecCheckTIDVisible, > heapam_tuple_lock) Done that. > - Rename TableTupleMayBeModified to TableTupleOk, but also probably a > s/TableTuple/TableMod/ It's now TM_*. /* * Result codes for table_{update,delete,lock}_tuple, and for visibility * routines inside table AMs. */ typedef enum TM_Result { /* * Signals that the action succeeded (i.e. update/delete performed, lock * was acquired) */ TM_Ok, /* The affected tuple wasn't visible to the relevant snapshot */ TM_Invisible, /* The affected tuple was already modified by the calling backend */ TM_SelfModified, /* * The affected tuple was updated by another transaction. This includes * the case where tuple was moved to another partition. */ TM_Updated, /* The affected tuple was deleted by another transaction */ TM_Deleted, /* * The affected tuple is currently being modified by another session. This * will only be returned if (update/delete/lock)_tuple are instructed not * to wait. */ TM_BeingModified, /* lock couldn't be acquired, action skipped. Only used by lock_tuple */ TM_WouldBlock } TM_Result; > - I'll probably move TUPLE_LOCK_FLAG_LOCK_* into tableam.h Done. > - two more passes through the patch One of them completed. Which is good, because there was a subtle bug in heapam_tuple_lock (*tid was adjusted to be the followup tuple after the heap_fetch(), before going to heap_lock_tuple - but that's wrong, it should only be adjusted when heap_fetch() ing the next version.). Greetings, Andres Freund