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

Andres Freund wrote:

> 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.

No objection to renaming the function. I am certain that when I first
wrote it, it was going to be used for updated tuples; I never considered
deletes. After it was repurposed, I never thought about renaming it.

> 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.

Hmm, OK, I don't remember this. But no, it's not about the heavyweight
lock -- it's about the xmax-level tuple lock.

> 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.

Yeah, what MayBeUpdated is supposed to mean in this case is "there is no
error, we were able to do the thing we were asked to do", rather than
exactly "yes, you may update the tuple". I guess you could argue that
reusing HTSU result values for it was wrong. It was certainly
convenient.

--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services