Hi,
On 2022-04-11 12:19:51 -0400, Tom Lane wrote:
> I wrote:
> > Reproduced here. It's surprising that nobody noticed this before,
> > because AFAICS the bug is pretty old: it dates to somebody foolishly
> > deciding that heap_fetch didn't need its keep_buf argument, which
> > evidently happened in v12 (didn't track down the exact commit yet).
>
> The blame falls on Andres' commit 5db6df0c0. There is no commentary
> there justifying this change, and I judge the amount of thought that
> went into it by noting that that commit removed the argument without
> removing the header comment explaining it.
I've only just had a first coffee, so I'm a bit foggy still. There were a few
different attempts at making tuple locking fit into some form abstraction, not
making it easy to remember...
Hm. It looks like the problem only can happen when the tuple is filtered out
by the snapshot. We don't need the xmin in line 509 from the tuple, we have it
in SnapshotDirty. The problem is we need t_ctid too.
Ah. I dimly recall that for a while the patchset had a ctid in SnapshotData
too, for SnapshotDirty purposes. That might be the origin of the problem.
One way to address it in a way not requiring an API break would be to pass
SnapshotAny to heap_fetch and then do an explicit visibility check "ourselves"
in heapam_lock_tuple().
Greetings,
Andres Freund