Re: concerns around pg_lsn

On Thu, Aug 01, 2019 at 12:39:26PM +0530, Jeevan Ladhe wrote:
> Here is a patch that takes care of addressing the flag issue including
> pg_lsn_in_internal() and others.

Your original patch for pg_lsn_in_internal() was right IMO, and the
new one is not.  In the numeric and float code paths, we have this
kind of pattern:
if (have_error)
{
    *have_error = true;
    return;
}
else
    elog(ERROR, "Boom. Show is over.");

But the pg_lsn.c portion does not have that.  have_error cannot be
NULL or the caller may fall into the trap of setting it to NULL and
miss some errors at parsing-time.  So I think that keeping the
assertion on (have_error != NULL) is necessary.
--
Michael