Re: Visibility bug in tuple lock

> Thanks! That's not quite correct though. This is very subtle, but the
> 'tuple' argument to heap_lock_updated_tuple() points to the buffer
> holding the original tuple. So doing
> HeapTupleHeaderGetUpdateXid(tuple->t_data) reads the current xmax of the
> tuple, which can now be different than what it was earlier, i.e.
> different from the xwait that we waited for. It's important that the
> 'ctid' and 'priorXmax' that we use to follow the chain are read together.

Oops, you are right, I was too quick, it has already been quite some
time since i was dealing with this problem initially.

I see you moved the check ItemPointerEquals(&tuple->t_self, ctid) out
to heap_lock_tuple()
but isn't this redundant in the check `if (follow_updates && updated
&& !ItemPointerEquals(&tuple->t_self, &t_ctid))` if `updated` is
already true in this condition?

Also in `heap_lock_updated_tuple_rec()` is the check for
`TransactionIdIsValid(priorXmax)` now redundant too?

/ * Locking the initial tuple where those
  * values came from is the caller's responsibility. */

I think this is still ambiguous, you can still interpret that as the
initial tuple needs to be locked before the function is called. Maybe
it is better to change it to something like:
"This function will not lock the initial tuple."

Unrelated question, I was wondering why these functions take a "const
ItemPointerData *" as an argument as opposed to just pass the ctid by
value?

Jasper