On Fri, Mar 10, 2017 at 11:47 AM, Andres Freund <andres@anarazel.de> wrote:
>> 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.
I was in favor of just removing the assertion myself, given the
PGDLLIMPORT issue, but FWIW I am generally in favor of documenting
assertions like this. I suppose that this assertion is less likely to
ever break than most other assertions, but presumably no code ever
gets committed without somebody being pretty confident that any
assertions it happens to have will never fail to hold. It doesn't seem
productive to worry about whether or not any trivial assertions are
pulling their weight. They're justified as documentation. If the
assertion ever does fail, preventing someone from pushing buggy code,
then so much the better.
--
Peter Geoghegan