On 2019-03-18 16:24:40 -0700, Andres Freund wrote:
> Hi,
>
> On 2019-03-13 08:29:47 -0400, Robert Haas wrote:
> > On Tue, Mar 12, 2019 at 8:39 PM Andres Freund <andres@anarazel.de> wrote:
> > > > I like that option.
> > >
> > > In that vein, does anybody have an opinion about the naming of
> > > a) HeapUpdateFailureData, which will be used for different AMs
> > > b) HTSU_Result itself, which'll be the return parameter for
> > > update/delete via tableam
> > > c) Naming of HTSU_Result members (like HeapTupleBeingUpdated)
> > >
> > > I can see us doing several things:
> > > 1) Live with the old names, explain the naming as historical baggage
> > > 2) Replace names, but add typedefs / #defines for backward compatibility
> > > 3) Rename without backward compatibility
> >
> > I think I have a mild preference for renaming HeapUpdateFailureData to
> > TableUpdateFailureData, but I'm less excited about renaming
> > HTSU_Result or its members. I don't care if you want to do
> > s/HeapTuple/TableTuple/g and s/HTSU_Result/TTSU_Result/g though.
>
> Anybody else got an opion on those? I personally don't have meaningful
> feelings on this. I'm mildly inclined to the renamings suggested
> above.
What I now have is:
/*
* 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) */
TableTupleMayBeModified,
/* The affected tuple wasn't visible to the relevant snapshot */
TableTupleInvisible,
/* The affected tuple was already modified by the calling backend */
TableTupleSelfModified,
/* The affected tuple was updated by another transaction */
TableTupleUpdated,
/* The affected tuple was deleted by another transaction */
TableTupleDeleted,
/*
* 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.
*/
TableTupleBeingModified,
/* lock couldn't be acquired, action skipped. Only used by lock_tuple */
TableTupleWouldBlock
} TM_Result;
I got rid of the SU part of the prefix, because SatisfiesUpdate isn't
actually exposed outside of the AMs.
I'm kinda wondering about replacing the TableTuple prefix with TableMod,
seems less confusing to me. I'm also wondering about replacing
*MayBeModified with *OK.
Right now I have
typedef enum TM_Result HTSU_Result;
#define HeapTupleMayBeUpdated TableTupleMayBeModified
#define HeapTupleInvisible TableTupleInvisible
#define HeapTupleSelfUpdated TableTupleSelfModified
#define HeapTupleUpdated TableTupleUpdated
#define HeapTupleDeleted TableTupleDeleted
#define HeapTupleBeingUpdated TableTupleBeingModified
#define HeapTupleWouldBlock TableTupleWouldBlock
in heapam.h (whereas the above is in tableam.h), for backward
compat. But I'm not sure it's worth it.
Greetings,
Andres Freund