comments around heap_lock_tuple confus{ing,ed} around deleted tuples

Hi,

While looking at resolving [1] I re-read heap_lock_tuple() and
subsidiary routines and got thoroughly confused for a while.

One reason was that function names and comments talk about updated, when
they also actually deal with deletes. heap_lock_updated_tuple()
specifically is called on tuples that have not been updated, but have
been deleted.

/*
* heap_lock_updated_tuple
* Follow update chain when locking an updated tuple, acquiring locks (row
* marks) on the updated versions.
*
* The initial tuple is assumed to be already locked.

So

a) The function name is wrong, we're not necessarily dealing with an
updated tuple.
b) The initial tuple is actually not generally locked when the function
is called. See the call below the
/* if there are updates, follow the update chain */
comment.

Or is that supposed to mean that the initial tuple has already been
locked with the heavyweight lock? But that can't be true either,
because afaics the heap_lock_updated_tuple() call for
LockTupleKeyShare doesn't even do that?

It's also fairly weird that heap_lock_updated_tuple() returns
/* nothing to lock */
return HeapTupleMayBeUpdated;
when the tuple has been deleted (and thus
ItemPointerEquals(&tuple->t_self, ctid)). That'll not get returned by
heap_lock_tuple() itself, but seems thoroughly confusing.

There's some argument to be made for not changing this because "it seems
to work", but the wrong comments and function names are not unlikely to
cause future bugs...

Greetings,

Andres Freund

[1] http://archives.postgresql.org/message-id/CAAJ_b95PkwojoYfz0bzXU8OokcTVGzN6vYGCNVUukeUDrnF3dw%40mail.gmail.com