Re: TransactionIdGetCommitTsData and its dereferenced pointers

Michael Paquier wrote:

> TransactionIdGetCommitTsData(at)commit_ts(dot)c does the following:
> if (ts)
> *ts = entry.time;
> [...]
> return *ts != 0;
> This is a bad idea, because if TransactionIdGetCommitTsData is called
> with ts == NULL this would simply crash. It seems to me that it makes
> little sense to have ts == NULL either way because this function is
> intended to return a timestamp in any case, hence I think that we
> should simply Assert(ts == NULL) and remove those checks.

I pushed this. But without the assert, because it's pointless: if the
passed pointer were indeed null, we would still crash later in the
function, getting the same effect as the assert.

Thanks,

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