Re: pgsql: contrib/amcheck needs RecentGlobalXmin to be PGDLLIMPORT'ified.

On 2017-03-09 23:50:27 -0500, Tom Lane wrote:
> Andres Freund <andres(at)anarazel(dot)de> writes:
> > On 2017-03-10 03:55:50 +0000, Tom Lane wrote:
> >> contrib/amcheck needs RecentGlobalXmin to be PGDLLIMPORT'ified.
>
> > Heh, I just wanted to push a patch removing the assertion because it
> > doesn't add that much. There's imo no reason not to mark the variable
> > PGDLLIMPORT, so I'm good with this too.
>
> Oh, I hadn't looked closely enough to notice that the only reference
> there was
>
> Assert(TransactionIdIsValid(RecentGlobalXmin));
>
> I agree: that is just about utterly useless.

Well, it mirrors an existing Assert, that'd be hit when doing normal
index lookups. But I agree that a bug around this is exceedingly
unlikely at this point, so there's no coverage value in it.

> Let's revert my patch
> and remove that Assert. I'm not eager to encourage people to reference
> the xmin globals if we don't have to.

We have a bunch of index access methods (nbtree, spgist) referencing
RecentGlobalXmin for, imo, reasonable reasons. So it doesn't seem
unreasonable to keep it available for extensions, given the amount of
work has gone into making indexes from extensions a usable thing.

Greetings,

Andres Freund