Re: foreign key locks, 2nd attempt - Mailing list pgsql-hackers

On Mon, Jan 16, 2012 at 04:52:36PM -0300, Alvaro Herrera wrote:
> Excerpts from Heikki Linnakangas's message of lun ene 16 16:17:42 -0300 2012:
> > On 15.01.2012 06:49, Alvaro Herrera wrote:
> > > --- 164,178 ----
> > >   #define HEAP_HASVARWIDTH        0x0002    /* has variable-width attribute(s) */
> > >   #define HEAP_HASEXTERNAL        0x0004    /* has external stored attribute(s) */
> > >   #define HEAP_HASOID                0x0008    /* has an object-id field */
> > > ! #define HEAP_XMAX_KEYSHR_LOCK    0x0010    /* xmax is a key-shared locker */
> > >   #define HEAP_COMBOCID            0x0020    /* t_cid is a combo cid */
> > >   #define HEAP_XMAX_EXCL_LOCK        0x0040    /* xmax is exclusive locker */
> > > ! #define HEAP_XMAX_IS_NOT_UPDATE    0x0080    /* xmax, if valid, is only a locker.
> > > !                                          * Note this is not set unless
> > > !                                          * XMAX_IS_MULTI is also set. */
> > > !
> > > ! #define HEAP_LOCK_BITS    (HEAP_XMAX_EXCL_LOCK | HEAP_XMAX_IS_NOT_UPDATE | \
> > > !                          HEAP_XMAX_KEYSHR_LOCK)
> > >   #define HEAP_XMIN_COMMITTED        0x0100    /* t_xmin committed */
> > >   #define HEAP_XMIN_INVALID        0x0200    /* t_xmin invalid/aborted */
> > >   #define HEAP_XMAX_COMMITTED        0x0400    /* t_xmax committed */
> > 
> > HEAP_XMAX_IS_NOT_UPDATE is a pretty opaque name for that. From the name, 
> > I'd say that a DELETE would set that, but the comment says it has to do 
> > with locking. After going through all the combinations in my mind, I 
> > think I finally understood it: HEAP_XMAX_IS_NOT_UPDATE is set if xmax is 
> > a multi-xact, that represent only locking xids, no updates. How about 
> > calling it "HEAP_XMAX_LOCK_ONLY", and setting it whenever whether or not 
> > xmax is a multi-xid?
> 
> Hm, sounds like a good idea.  Will do.

+1

> > Why are you renaming HeapTupleHeaderGetXmax() into 
> > HeapTupleHeaderGetRawXmax()? Any current callers of 
> > HeapTupleHeaderGetXmax() should already check that HEAP_XMAX_IS_MULTI is 
> > not set.
> 
> I had this vague impression that it'd be better to break existing
> callers so that they ensure they decide between
> HeapTupleHeaderGetRawXmax and HeapTupleHeaderGetUpdateXid.  Noah
> suggested changing the macro name, too.  It's up to each caller to
> decide what's the sematics they want.  Most want the latter; and callers
> outside core are more likely to want that one.  If we kept the old name,
> they would get the wrong value.

My previous suggestion was to have both macros:

#define HeapTupleHeaderGetXmax(tup) \
( \AssertMacro(!((tup)->t_infomask & HEAP_XMAX_IS_MULTI)), \HeapTupleHeaderGetRawXmax(tup) \
)

#define HeapTupleHeaderGetRawXmax(tup) \
( \(tup)->t_choice.t_heap.t_xmax \
)

No strong preference, though.