Re: [COMMITTERS] pgsql: Account for catalog snapshot in PGXACT->xmin updates.

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Tue, Nov 15, 2016 at 3:55 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Account for catalog snapshot in PGXACT->xmin updates.

> It seems to me that this makes it possible for
> ThereAreNoPriorRegisteredSnapshots() to fail spuriously. The only
> core code that calls that function is in copy.c, and apparently we
> never reach that point with the catalog snapshot set. But that seems
> fragile.

Hm. If there were some documentation explaining what
ThereAreNoPriorRegisteredSnapshots is supposed to do, it would be possible
for us to have a considered argument as to whether this patch broke it or
not. As things stand, it is not obvious whether it ought to be ignoring
the catalog snapshot or not; one could construct plausible arguments
either way. It's not even very obvious why both 0 and 1 registered
snapshots should be considered okay; that looks a lot more like a hack
than reasoned-out behavior. So I'm satisfied if COPY FREEZE does what
it's supposed to do, which it still appears to do.

IOW, I'm not interested in touching this unless someone first provides
adequate documentation for the pre-existing kluge here. There is no
API spec at all on the function itself, and the comment in front of the
one call site is too muddle-headed to provide any insight.

regards, tom lane